On Fri, Jun 3, 2022 at 11:28 PM Numan Siddique <[email protected]> wrote:
> On Wed, Jun 1, 2022 at 8:28 AM Ales Musil <[email protected]> wrote: > > > > The localnet is excluded from MAC learning for scale > > reason. However there might be a valid workflow > > when yo uwant to enable the learning and benefit > > for that for HW offload. Add option called > > 'localnet_learn_fdb' to LSP, which will enable/disable > > the learning. Setting it as disabled by default. > > > > Reported-at: https://bugzilla.redhat.com/2070529 > > Signed-off-by: Ales Musil <[email protected]> > > Thanks for adding the support. > > A few minor comments below. > > Numan > > > --- > > northd/northd.c | 10 ++++++++-- > > ovn-nb.xml | 7 +++++++ > > ovn-sb.xml | 1 + > > tests/ovn-northd.at | 37 +++++++++++++++++++++++++++++++++++++ > > 4 files changed, 53 insertions(+), 2 deletions(-) > > > > diff --git a/northd/northd.c b/northd/northd.c > > index 16ea7a6aa..b3077714e 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -5408,8 +5408,14 @@ build_lswitch_learn_fdb_op( > > struct ovn_port *op, struct hmap *lflows, > > struct ds *actions, struct ds *match) > > { > > - if (op->nbsp && !op->n_ps_addrs && !strcmp(op->nbsp->type, "") && > > - op->has_unknown) { > > + if (!op->nbsp) { > > + return; > > + } > > + > > + bool localnet_learn_fdb = smap_get_bool(&op->nbsp->options, > > + "localnet_learn_fdb", > false); > > I'd suggest to add a helper function for the above i.e to check if > learn fdb is enabled or not for localnet ports > and modify the below 'if' as > > if (!op->n_ps_addrs && op->has_unknown && (!strcmp(op->nbsp->type, "") || > (lsp_is_localnet(op->nbsp) && > localnet_lsp_has_learn_<or_a_better_name_function>(op->nbsp->options))) > { > /// > } > > This would avoid unnecessary smap_get_bool() call for other lsp types. > > I think it would be good if you can add a new test in ovn.at or > enhance existing OVN FDB (MAC learning) test cases in ovn.at > to cover this scenario. > > You can create a vif in the localnet bridge and inject some packets > from there so that the packet enters the br-int via the patch port > and the test can check if the mac learning is working as expected or not. > > Thanks > Numan > > > + if (!op->n_ps_addrs && op->has_unknown && (!strcmp(op->nbsp->type, > "") || > > + (localnet_learn_fdb && lsp_is_localnet(op->nbsp)))) { > > ds_clear(match); > > ds_clear(actions); > > ds_put_format(match, "inport == %s", op->json_key); > > diff --git a/ovn-nb.xml b/ovn-nb.xml > > index 3e3e142b3..9df6b1aab 100644 > > --- a/ovn-nb.xml > > +++ b/ovn-nb.xml > > @@ -976,6 +976,13 @@ > > headers. Supported values: 802.11q (default), 802.11ad. > > </column> > > > > + <column name="options" key="localnet_learn_fdb" > > + type='{"type": "boolean"}'> > > + Optional. Allows localnet port to learn MACs and store them > in FDB > > + table if set to <code>true</code>. The default value is > > + <code>false</code>. > > + </column> > > + > > </group> > > > > <group title="Options for l2gateway ports"> > > diff --git a/ovn-sb.xml b/ovn-sb.xml > > index 4c35dda36..3d92ba88f 100644 > > --- a/ovn-sb.xml > > +++ b/ovn-sb.xml > > @@ -4602,6 +4602,7 @@ tcp.flags = RST; > > <table name="FDB" title="Port to MAC bindings"> > > <p> > > This table is primarily used to learn the MACs observed on a VIF > > + (or a localnet port with 'localnet_learn_fdb' enabled) > > which belongs to a <code>Logical_Switch_Port</code> record in > > <code>OVN_Northbound</code> whose port security is disabled > > and 'unknown' address set. If port security is disabled on a > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index 5bd0935e7..89eeb0894 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -7418,3 +7418,40 @@ AT_CHECK([cat sw0flows | grep -e port_sec | sort > | sed 's/table=./table=?/' ], [ > > > > AT_CLEANUP > > ]) > > + > > +OVN_FOR_EACH_NORTHD([ > > +AT_SETUP([Localnet MAC learning option]) > > +ovn_start > > + > > +AT_CHECK([ovn-nbctl ls-add ls0]) > > + > > +AT_CHECK([ovn-nbctl lsp-add ls0 ln_port]) > > +AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown]) > > +AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet]) > > +AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=phys]) > > +AT_CHECK([ovn-nbctl --wait=sb sync]) > > + > > +# Check MAC learning flows with 'localnet_learn_fdb' default (false) > > +AT_CHECK([ovn-sbctl dump-flows ls0 | grep -e > 'ls_in_\(put\|lookup\)_fdb' | sort | sed 's/table=./table=?/'], [0], [dnl > > + table=? (ls_in_lookup_fdb ), priority=0 , match=(1), > action=(next;) > > + table=? (ls_in_put_fdb ), priority=0 , match=(1), > action=(next;) > > +]) > > + > > +# Enable 'localnet_learn_fdb' and check the flows > > +AT_CHECK([ovn-nbctl --wait=sb lsp-set-options ln_port > localnet_learn_fdb=true]) > > +AT_CHECK([ovn-sbctl dump-flows ls0 | grep -e > 'ls_in_\(put\|lookup\)_fdb' | sort | sed 's/table=./table=?/'], [0], [dnl > > + table=? (ls_in_lookup_fdb ), priority=0 , match=(1), > action=(next;) > > + table=? (ls_in_lookup_fdb ), priority=100 , match=(inport == > "ln_port"), action=(reg0[[11]] = lookup_fdb(inport, eth.src); next;) > > + table=? (ls_in_put_fdb ), priority=0 , match=(1), > action=(next;) > > + table=? (ls_in_put_fdb ), priority=100 , match=(inport == > "ln_port" && reg0[[11]] == 0), action=(put_fdb(inport, eth.src); next;) > > +]) > > + > > +# Disable 'localnet_learn_fdb' and check the flows > > +AT_CHECK([ovn-nbctl --wait=sb lsp-set-options ln_port > localnet_learn_fdb=false]) > > +AT_CHECK([ovn-sbctl dump-flows ls0 | grep -e > 'ls_in_\(put\|lookup\)_fdb' | sort | sed 's/table=./table=?/'], [0], [dnl > > + table=? (ls_in_lookup_fdb ), priority=0 , match=(1), > action=(next;) > > + table=? (ls_in_put_fdb ), priority=0 , match=(1), > action=(next;) > > +]) > > + > > +AT_CLEANUP > > +]) > > -- > > 2.35.3 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > Hi Numan, thank you for the review. I have posted v2. Thanks, Ales -- Ales Musil Senior Software Engineer - RHV Network Red Hat EMEA <https://www.redhat.com> [email protected] IM: amusil <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
