[ovs-dev] [PATCH ovn] controller: fix ip buffering with static routes
Numan Siddique
numans at ovn.org
Thu May 28 12:28:01 UTC 2020
On Thu, May 28, 2020 at 5:45 PM Lorenzo Bianconi <
lorenzo.bianconi at redhat.com> wrote:
> > On Fri, May 22, 2020 at 1:19 PM Lorenzo Bianconi <
> > lorenzo.bianconi at redhat.com> wrote:
> >
> > > > Hi Lorenzo,
> > >
> > > Hi Ankur,
> > >
> > > >
> > > > Good catch.
> > > >
> > > > a. If not already considered, then i think it is a candidate for
> 20.06
> > > > b. Need not be done in this patch, but it will be good to have a
> > > testcase which validates the draining of buffered packets.
> > >
> > > We already have some tests in system-ovn.at and in ovn.at. Do you mean
> > > something more specific?
> > >
> > > > c. Not sure about the commit header. I mean this issue is not
> specific
> > > to static route right? By default there will be a gateway and
> destination
> > > ip will not be that of gateway ip.
> > > > That's a regular NS workflow.
> > >
> > > I think ovn does not add any "default" route by default. You need to
> add
> > > it doing
> > > something like:
> > >
> > > ovn-nbctl lr-route-add <logical-router> 0.0.0.0/0 <next-hop-ip>
> > >
> > > Regards,
> > > Lorenzo
> > >
> > > >
> > > > Acked-by: Ankur Sharma <ankur.sharma at nutanix.com>
> > >
> >
> > Thanks Lorenzo and Ankur (for the reviews).
> > I applied this patch to master.
> >
> > I agree with Ankur that having a test case covering this scenario will
> be
> > helpful.
> >
> > @Lorenzo - Would you mind a follow up patch for this if it's possible ?
>
> Hi Numan,
>
> I have already intoduced a test case for this scenario here:
> https://github.com/ovn-org/ovn/blob/master/tests/ovn.at#L15022
> and here:
> https://github.com/ovn-org/ovn/blob/master/tests/system-ovn.at#L2751
>
> Do you mean something more specific?
>
>
Can you please see the reply from Ankur here -
https://mail.openvswitch.org/pipermail/ovs-dev/2020-May/370908.html
I think he has some suggestions about the test cases. I should have replied
in that thread.
Thanks
Numan
Regards,
> Lorenzo
>
> >
> > Do we need to apply this patch to 20.03 ?
> >
> > Thanks
> > Numan
> >
> > >
> > > > Regards,
> > > > Ankur
> > > > ________________________________
> > > > From: dev <ovs-dev-bounces at openvswitch.org> on behalf of Lorenzo
> > > Bianconi <lorenzo.bianconi at redhat.com>
> > > > Sent: Thursday, May 21, 2020 6:46 AM
> > > > To: ovs-dev at openvswitch.org <ovs-dev at openvswitch.org>
> > > > Subject: [ovs-dev] [PATCH ovn] controller: fix ip buffering with
> static
> > > routes
> > > >
> > > > When the ARP request is sent to a gw router and not to the final
> > > > destination of the packet buffered_packets_map needs to be updated
> using
> > > > next-hop IP address and not the destination one.
> > > >
> > > > Fixes: 2e5cdb4b1392 ("OVN: add buffering support for ip packets")
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> > > > ---
> > > > controller/pinctrl.c | 12 ++++++------
> > > > 1 file changed, 6 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> > > > index bea446c89..bb90edd1f 100644
> > > > --- a/controller/pinctrl.c
> > > > +++ b/controller/pinctrl.c
> > > > @@ -1381,8 +1381,7 @@ pinctrl_find_buffered_packets(const struct
> > > in6_addr *ip, uint32_t hash)
> > > >
> > > > /* Called with in the pinctrl_handler thread context. */
> > > > static int
> > > > -pinctrl_handle_buffered_packets(const struct flow *ip_flow,
> > > > - struct dp_packet *pkt_in,
> > > > +pinctrl_handle_buffered_packets(struct dp_packet *pkt_in,
> > > > const struct match *md, bool is_arp)
> > > > OVS_REQUIRES(pinctrl_mutex)
> > > > {
> > > > @@ -1391,9 +1390,10 @@ pinctrl_handle_buffered_packets(const struct
> flow
> > > *ip_flow,
> > > > struct in6_addr addr;
> > > >
> > > > if (is_arp) {
> > > > - addr = in6_addr_mapped_ipv4(ip_flow->nw_dst);
> > > > + addr = in6_addr_mapped_ipv4(htonl(md->flow.regs[0]));
> > > > } else {
> > > > - addr = ip_flow->ipv6_dst;
> > > > + ovs_be128 ip6 = hton128(flow_get_xxreg(&md->flow, 0));
> > > > + memcpy(&addr, &ip6, sizeof addr);
> > > > }
> > > >
> > > > uint32_t hash = hash_bytes(&addr, sizeof addr, 0);
> > > > @@ -1434,7 +1434,7 @@ pinctrl_handle_arp(struct rconn *swconn, const
> > > struct flow *ip_flow,
> > > > }
> > > >
> > > > ovs_mutex_lock(&pinctrl_mutex);
> > > > - pinctrl_handle_buffered_packets(ip_flow, pkt_in, md, true);
> > > > + pinctrl_handle_buffered_packets(pkt_in, md, true);
> > > > ovs_mutex_unlock(&pinctrl_mutex);
> > > >
> > > > /* Compose an ARP packet. */
> > > > @@ -5281,7 +5281,7 @@ pinctrl_handle_nd_ns(struct rconn *swconn,
> const
> > > struct flow *ip_flow,
> > > > }
> > > >
> > > > ovs_mutex_lock(&pinctrl_mutex);
> > > > - pinctrl_handle_buffered_packets(ip_flow, pkt_in, md, false);
> > > > + pinctrl_handle_buffered_packets(pkt_in, md, false);
> > > > ovs_mutex_unlock(&pinctrl_mutex);
> > > >
> > > > uint64_t packet_stub[128 / 8];
> > > > --
> > > > 2.26.2
> > > >
> > > > _______________________________________________
> > > > dev mailing list
> > > > dev at openvswitch.org
> > > >
> > >
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=s883GpUCOChKOHiocYtGcg&r=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=M_tzprKGSufeFSzeZOYjY5JUCnecaZWqQmYWNJazeiY&s=NQAYRfJ3Svolg8UWgwkkDxAH8FTT4PKayFV7Kw--fN8&e=
> > > _______________________________________________
> > > dev mailing list
> > > dev at openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >
> > >
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
More information about the dev
mailing list