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
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
