On Tue, Dec 7, 2021 at 11:43 AM Vladislav Odintsov <[email protected]> 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 <[email protected]> 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
> >> [email protected]
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
