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