[ovs-dev] [PATCH ovn] physical: do not forward traffic from localport to a localnet one
Lorenzo Bianconi
lorenzo.bianconi at redhat.com
Fri May 21 11:20:18 UTC 2021
> On Thu, May 20, 2021 at 5:05 PM Mark Michelson <mmichels at redhat.com> 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 <lorenzo.bianconi at redhat.com>
> > > ---
> > > 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
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
More information about the dev
mailing list