[ovs-dev] [PATCH ovn] controller: do not mark bfd and ipv6_pd msgs as local-only

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Mon Oct 11 10:01:29 UTC 2021


> On 10/9/21 1:05 AM, Ihar Hrachyshka wrote:
> > This is correct, thank you for fixing it.
> > 
> > One comment and one question:
> > 
> > 1) While the referred patch that suppressed LOCAL_ONLY traffic exposed
> > the issue, it was always a bug, though affecting a tiny minority of
> > scenarios.
> > 
> > For example for PD, one can probably imagine a scenario where PD server
> > is not located in a provider network but is hosted on a VIF that is
> > attached to a "public" logical switch (no localnet ports!). This VIF may
> > be located on a different chassis, in which case, even without the
> > suppressing patch, PD requests wouldn't reach PD server. On the other
> > hand, they would reach the server if its VIF would happen to land on the
> > same chassis. A similar scenario may be envisioned for BFD, where router
> > ports are distributed across multiple chassis.
> 
> I agree, the "Fixes" tag looks wrong.  I think it should be:
> 
> Fixes: e3a398e9146e ("controller: Add ipv6 prefix delegation state machine")
> Fixes: 365a8a696afb ("bfd: support demand mode on rx side.")

ack, do I need to repost to fix them?

Regards,
Lorenzo

> 
> > 
> > 2) I see you covered all pinctrl handlers that were setting the
> > LOCAL_ONLY bit via MLF_LOCAL_ONLY_BIT macro. I've noticed that there are
> > several other handlers that use MLF_LOCAL_ONLY macro instead, with (as
> > far as I can see) exactly the same effect (any idea why we have both?).
> > An example of that would be e.g. ip_mcast_querier_send_igmp. Do we
> > believe that other packet injections are meant to stay inside the same
> > chassis, or is it just something you missed / left out of scope for this
> > patch?
> 
> The other use cases are, AFAICT:
> 
> - IGMP/MLD querier
> - service monitor
> 
> The multicast queriers use per logical switch source mac address, and
> are generated by ovn-controller on each hypervisor where the logical
> switch has bound ports.  As far as I see it's similar for service
> monitor.  So using MLF_LOCAL_ONLY seems correct to me.
> 
> > 
> > Thanks again,
> > 
> > Ihar
> 
> Regards,
> Dumitru
> 
> > 
> > On 10/8/21 10:54 AM, Lorenzo Bianconi wrote:
> >> Do not mark BFD and IPv6 Prefix Delegation messages as local only since
> >> they will be sent through a localnet port.
> >>
> >> Fixes: 14c6dac51dc8 ("Suppress LOCAL_ONLY traffic for localnet ports")
> >> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> >> ---
> >>   controller/pinctrl.c | 2 --
> >>   1 file changed, 2 deletions(-)
> >>
> >> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> >> index cc3edaaf4..7d01b18a3 100644
> >> --- a/controller/pinctrl.c
> >> +++ b/controller/pinctrl.c
> >> @@ -1108,7 +1108,6 @@ ipv6_prefixd_send(struct rconn *swconn, struct
> >> ipv6_prefixd_state *pfd)
> >>       uint32_t port_key = pfd->port_key;
> >>       put_load(dp_key, MFF_LOG_DATAPATH, 0, 64, &ofpacts);
> >>       put_load(port_key, MFF_LOG_INPORT, 0, 32, &ofpacts);
> >> -    put_load(1, MFF_LOG_FLAGS, MLF_LOCAL_ONLY_BIT, 1, &ofpacts);
> >>       struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&ofpacts);
> >>       resubmit->in_port = OFPP_CONTROLLER;
> >>       resubmit->table_id = OFTABLE_LOG_INGRESS_PIPELINE;
> >> @@ -6815,7 +6814,6 @@ pinctrl_send_bfd_tx_msg(struct rconn *swconn,
> >> struct bfd_entry *entry,
> >>       uint32_t port_key = entry->port_key;
> >>       put_load(dp_key, MFF_LOG_DATAPATH, 0, 64, &ofpacts);
> >>       put_load(port_key, MFF_LOG_INPORT, 0, 32, &ofpacts);
> >> -    put_load(1, MFF_LOG_FLAGS, MLF_LOCAL_ONLY_BIT, 1, &ofpacts);
> >>       struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&ofpacts);
> >>       resubmit->in_port = OFPP_CONTROLLER;
> >>       resubmit->table_id = OFTABLE_LOG_INGRESS_PIPELINE;
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > 
> 


More information about the dev mailing list