Hi Lorenzo,
Please find my replies inline.
Regards,
Ankur
________________________________
From: Lorenzo Bianconi
Sent: Friday, May 22, 2020 12:48 AM
To: Ankur Sharma
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH ovn] controller: fix ip buffering with static
routes
> 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?
[ANKUR]: Yes, looking at the testcase "[ovn -- IP packet buffering]", looks
like it is testing buffering for connected subnets (logical switches), i.e EW
traffic, using "unknown" address. While scenario is not invalid, however
feature is mostly applicable where "get_arp" will be called and hence buffering
will happen is N-S traffic. Having a testcase where get_arp is called for a
gateway, while destination ip is that of atleast more than a hop away endpoint.
> 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>
[ANKUR]: Not a big deal, i was trying to convey that having a default route
(i.e static route which you have called out) is implicit for NS traffic. This
point is not worth further discussion.
Regards,
Lorenzo
>
> Acked-by: Ankur Sharma <ankur.sha...@nutanix.com>
>
> Regards,
> Ankur
> ________________________________
> From: dev <ovs-dev-boun...@openvswitch.org> on behalf of Lorenzo Bianconi
> <lorenzo.bianc...@redhat.com>
> Sent: Thursday, May 21, 2020 6:46 AM
> To: ovs-dev@openvswitch.org <ovs-dev@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.bianc...@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
> d...@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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
