> On Thu, May 20, 2021 at 5:05 PM Mark Michelson <[email protected]> wrote: > > > > Acked-by: Mark Michelson > > > > I have a small nit below, but since it's strictly with a comment, it's > > easy for whoever merges this to correct the comment when they do it. > > There's no reason to spin up a new version of this.
ack, thx Mark and Numan. Regards, Lorenzo > > > > On 5/4/21 1:59 PM, Lorenzo Bianconi wrote: > > > Since the localnet port is available on each hv, do not forward traffic > > > to the localnet port if it is present in order to avoid switch fdb > > > misconfiguration. > > > Related bz: https://bugzilla.redhat.com/show_bug.cgi?id=1942877 > > > > > > Signed-off-by: Lorenzo Bianconi <[email protected]> > > > --- > > > controller/physical.c | 23 +++++++++++++++++++++++ > > > include/ovn/logical-fields.h | 4 ++++ > > > tests/ovn.at | 17 +++++++++++++++++ > > > 3 files changed, 44 insertions(+) > > > > > > diff --git a/controller/physical.c b/controller/physical.c > > > index 96c959d18..258842634 100644 > > > --- a/controller/physical.c > > > +++ b/controller/physical.c > > > @@ -1193,6 +1193,11 @@ consider_port_binding(struct ovsdb_idl_index > > > *sbrec_port_binding_by_name, > > > > > > load_logical_ingress_metadata(binding, &zone_ids, ofpacts_p); > > > > > > + if (!strcmp(binding->type, "localport")) { > > > + /* mark the packet as incoming from a localport */ > > > + put_load(1, MFF_LOG_FLAGS, MLF_LOCALPORT_BIT, 1, ofpacts_p); > > > + } > > > + > > > /* Resubmit to first logical ingress pipeline table. */ > > > put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p); > > > ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, > > > @@ -1251,6 +1256,24 @@ consider_port_binding(struct ovsdb_idl_index > > > *sbrec_port_binding_by_name, > > > ofport, flow_table); > > > } > > > > > > + /* Table 34, priority 160. > > > > Nit: Table 34 is incorrect here. OFTABLE_CHECK_LOOPBACK is table 39 now. > > I think the issue here is that when the tables shifted, nobody updated > > the comments in this file, and so it's wrong here and several other > > places in physical.c > > > > I corrected the comment and applied the patch to the main branch. > > Numan > > > > + * ======================= > > > + * > > > + * Do not forward local traffic from a localport to a localnet > > > port. > > > + */ > > > + if (!strcmp(binding->type, "localnet")) { > > > + /* do not forward traffic from localport to localnet port */ > > > + match_init_catchall(&match); > > > + ofpbuf_clear(ofpacts_p); > > > + match_set_metadata(&match, htonll(dp_key)); > > > + match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, port_key); > > > + match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0, > > > + MLF_LOCALPORT, MLF_LOCALPORT); > > > + ofctrl_add_flow(flow_table, OFTABLE_CHECK_LOOPBACK, 160, > > > + binding->header_.uuid.parts[0], &match, > > > + ofpacts_p, &binding->header_.uuid); > > > + } > > > + > > > } else if (!tun && !is_ha_remote) { > > > /* Remote port connected by localnet port */ > > > /* Table 33, priority 100. > > > diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h > > > index d44b30b30..ef97117b9 100644 > > > --- a/include/ovn/logical-fields.h > > > +++ b/include/ovn/logical-fields.h > > > @@ -67,6 +67,7 @@ enum mff_log_flags_bits { > > > MLF_LOOKUP_LB_HAIRPIN_BIT = 7, > > > MLF_LOOKUP_FDB_BIT = 8, > > > MLF_SKIP_SNAT_FOR_LB_BIT = 9, > > > + MLF_LOCALPORT_BIT = 10, > > > }; > > > > > > /* MFF_LOG_FLAGS_REG flag assignments */ > > > @@ -107,6 +108,9 @@ enum mff_log_flags { > > > /* Indicate that a packet must not SNAT in the gateway router when > > > * load-balancing has taken place. */ > > > MLF_SKIP_SNAT_FOR_LB = (1 << MLF_SKIP_SNAT_FOR_LB_BIT), > > > + > > > + /* Indicate the packet has been received from a localport */ > > > + MLF_LOCALPORT = (1 << MLF_LOCALPORT_BIT), > > > }; > > > > > > /* OVN logical fields > > > diff --git a/tests/ovn.at b/tests/ovn.at > > > index fe6a7c85b..99764b24b 100644 > > > --- a/tests/ovn.at > > > +++ b/tests/ovn.at > > > @@ -11823,10 +11823,17 @@ OVN_FOR_EACH_NORTHD([ > > > AT_SETUP([ovn -- localport suppress gARP]) > > > ovn_start > > > > > > +send_garp() { > > > + local inport=$1 eth_src=$2 eth_dst=$3 spa=$4 tpa=$5 > > > + local > > > request=${eth_dst}${eth_src}08060001080006040001${eth_src}${spa}${eth_dst}${tpa} > > > + as hv1 ovs-appctl netdev-dummy/receive vif$inport $request > > > +} > > > + > > > net_add n1 > > > sim_add hv1 > > > as hv1 > > > check ovs-vsctl add-br br-phys > > > +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys > > > ovn_attach n1 br-phys 192.168.0.1 > > > > > > check ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys > > > @@ -11837,6 +11844,7 @@ check ovn-nbctl ls-add ls \ > > > -- lsp-set-addresses lp "00:00:00:00:00:01 10.0.0.1" \ > > > -- lsp-add ls ln \ > > > -- lsp-set-type ln localnet \ > > > + -- lsp-set-addresses ln unknown \ > > > -- lsp-set-options ln network_name=phys \ > > > -- lsp-add ls lsp \ > > > -- lsp-set-addresses lsp "00:00:00:00:00:02 10.0.0.2" > > > @@ -11870,6 +11878,15 @@ AT_CHECK([ > > > test 0 -eq $pkts > > > ]) > > > > > > +spa=$(ip_to_hex 10 0 0 1) > > > +tpa=$(ip_to_hex 10 0 0 100) > > > +send_garp 1 000000000001 ffffffffffff $spa $tpa > > > + > > > +dnl traffic from localport should not be sent to localnet > > > +AT_CHECK([tcpdump -r hv1/br-phys_n1-tx.pcap arp[[24:4]]=0x0a000064 | wc > > > -l],[0],[dnl > > > +0 > > > +],[ignore]) > > > + > > > OVN_CLEANUP([hv1]) > > > AT_CLEANUP > > > ]) > > > > > > > _______________________________________________ > > 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
