Thanks Numan,
I’ll wait for your comments on #3.
Regards,
Vladislav Odintsov
> On 8 Dec 2021, at 19:13, Numan Siddique <num...@ovn.org> wrote:
>
> On Tue, Dec 7, 2021 at 11:43 AM Vladislav Odintsov <odiv...@gmail.com> wrote:
>>
>> Talking about patches 1 and 2 - they've got totally no negative impact,
>> it's an optimization for HW VTEP scenario - I'd like them to be included
>> as a part of 21.12.
>>
>> For patch #3 there is absolutely no affect for users who use either only
>> stateless ACLs or only stateful.
>>
>> For users, who do mix of allow-stateless and allow-related rules it's a
>> _possible_ affect, only if priority for allow-related rules is higher,
>> than for allow-stateless AND these rules have overlapping meaning. It's
>> worth to mention, that if somebody has such rules installed now, they
>> don't work as the could be treated.
>>
>> Let me know your thoughts.
>
> Ok.
>
> I applied the first 2 patches to the main branch with the below
> changes in patch 2.
> ddlog compilation was broken.
> ---
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 2efc4bb1f7..5db6ff03db 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -5483,7 +5483,7 @@ build_lswitch_input_port_sec_op(
>
> if (!strcmp(op->nbsp->type, "vtep")) {
> ds_put_format(actions, "next(pipeline=ingress, table=%d);",
> - S_SWITCH_IN_L2_LKUP);
> + ovn_stage_get_table(S_SWITCH_IN_L2_LKUP));
> } else {
> ds_put_cstr(actions, "next;");
> }
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index 530bb1e9d9..2fe73959c6 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -3469,19 +3469,18 @@ for (&SwitchPort(.lsp = lsp, .sw = sw,
> .json_name = json_name, .ps_eth_addresses
> i"inport == ${json_name} && eth.src ==
> {${ps_eth_addresses.join(\" \")}}"
> } in
>
> - var actions = {
> - var queue = match (pbinding.options.get(i"qdisc_queue_id")) {
> - None -> i"next;",
> - Some{id} -> i"set_queue(${id}); "
> - };
> - var ramp = if (lsp.__type == i"vtep") {
> - i"next(pipeline=ingress,
> table=${s_SWITCH_IN_L2_LKUP()});"
> - } else {
> - i"next;"
> - } in
> - };
> - i"${queue}${ramp}"
> - } in
> + var actions = {
> + var queue = match (pbinding.options.get(i"qdisc_queue_id")) {
> + None -> i"",
> + Some{id} -> i"set_queue(${id});"
> + };
> + var ramp = if (lsp.__type == i"vtep") {
> + i"next(pipeline=ingress,
> table=${s_SWITCH_IN_L2_LKUP().table_id});"
> + } else {
> + i"next;"
> + };
> + i"${queue}${ramp}"
> + } in
> Flow(.logical_datapath = sw._uuid,
> .stage = s_SWITCH_IN_PORT_SEC_L2(),
> .priority = 50,
>
>
> -------------
> I still need to take a look at patch 3.
>
> Thanks
> Numan
>
>>
>> regards,
>>
>> Vladislav Odintsov
>>
>> On 07.12.2021 19:14, Numan Siddique wrote:
>>> On Tue, Dec 7, 2021 at 3:58 AM Vladislav Odintsov <odiv...@gmail.com> wrote:
>>>> On 01.12.2021 15:56, Vladislav Odintsov wrote:
>>>>> Currently if user has a stateless and statetul ACLs (allow-stateless and
>>>>> allow-related) in one port group or in one logical switch simultaneously,
>>>>> the stateless rules whould take precedence.
>>>>> This patch series adds support for mixing all the ACLs types with the
>>>>> respect to their priority.
>>>>> This change requires next:
>>>>>
>>>>> Also, as an optimisation, traffic from HW VTEP switch in ingress datapath
>>>>> is passed from ls_in_l2_sec directly to ls_in_l2_lkup, as it doesn't need
>>>>> any processing in ingress pipeline except determining outport in
>>>>> ls_in_l2_lkup table.
>>>>>
>>>>> Vladislav Odintsov (3):
>>>>> Revert "northd: support HW VTEP with stateful datapath"
>>>>> northd: send ingress packets from HW VTEP directly to L2_LKUP table
>>>>> northd: support mix of stateless ACL with lower priority than stateful
>>>>>
>>>>> northd/northd.c | 113 ++++++++++++++++++++++------------------
>>>>> northd/ovn-northd.8.xml | 35 ++++---------
>>>>> northd/ovn_northd.dl | 47 +++++------------
>>>>> tests/ovn-northd.at | 50 ++++++++++--------
>>>>> 4 files changed, 114 insertions(+), 131 deletions(-)
>>>>>
>>>> Hi Numan,
>>>>
>>>> is is possible to plan this series to be included in 21.12?
>>> Hi Vladislav,
>>>
>>> I was thinking of considering them after branching. Do you need these
>>> patches for 21.12 ?
>>> I'm trying to understand the risk factor ? Are these patches risky at
>>> this time or will not affect other users who don't use this scenario ?
>>>
>>> If it is risk free, +1 from me for 21.12.
>>>
>>> Thanks
>>> Numan
>>>
>>>> _______________________________________________
>>>> dev mailing list
>>>> d...@openvswitch.org
>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
