Hi Eelco,
Sorry the delay repling for changing job. I will follow this patch with my new
email account. Thanks.
BR
wenxu
At 2022-04-01 22:35:03, "Eelco Chaudron" <[email protected]> wrote:
>
>
>On 14 Dec 2021, at 4:59, [email protected] wrote:
>
>> From: wenxu <[email protected]>
>>
>> netdev native tunnel encap process, when tunnel neighbor cache miss,
>> send arp/nd request. Before spa use tunnel source, change to:
>> find the spa which have the same subset with the nexthop of tunnel dst
>> on egress port, if false, use the tunnel src as spa.
>>
>> For example:
>> tunnel src is a vip with 10.0.0.7/32, tunnel dst is 10.0.1.7
>> the br-phy with address 192.168.0.7/24 and the default gateway is 192.168.0.1
>> So the spa of arp request for 192.168.0.1 should be 192.168.0.7 but not
>> 10.0.0.7
>>
>> Signed-off-by: wenxu <[email protected]>
>> ---
>> lib/ovs-router.c | 2 +-
>> lib/ovs-router.h | 2 ++
>> ofproto/ofproto-dpif-xlate.c | 12 +++++++++---
>> 3 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
>> index 09b81c6..6d3c731 100644
>> --- a/lib/ovs-router.c
>> +++ b/lib/ovs-router.c
>> @@ -164,7 +164,7 @@ static void rt_init_match(struct match *match, uint32_t
>> mark,
>> match->flow.pkt_mark = mark;
>> }
>>
>> -static int
>> +int
>> get_src_addr(const struct in6_addr *ip6_dst,
>
>Rather than directly expose this function as is, maybe we should have a more
>generic name. For example:
>
> ovs_router_get_netdev_source_address()
>
>> const char output_bridge[], struct in6_addr *psrc)
>> {
>> diff --git a/lib/ovs-router.h b/lib/ovs-router.h
>> index 34ea163..f7ce7cf 100644
>> --- a/lib/ovs-router.h
>> +++ b/lib/ovs-router.h
>> @@ -26,6 +26,8 @@
>> extern "C" {
>> #endif
>>
>> +int get_src_addr(const struct in6_addr *ip6_dst,
>> + const char output_bridge[], struct in6_addr *psrc);
>> bool ovs_router_lookup(uint32_t mark, const struct in6_addr *ip_dst,
>> char out_dev[],
>> struct in6_addr *src, struct in6_addr *gw);
>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>> index 9d336bc..c556312 100644
>> --- a/ofproto/ofproto-dpif-xlate.c
>> +++ b/ofproto/ofproto-dpif-xlate.c
>> @@ -3603,9 +3603,10 @@ native_tunnel_output(struct xlate_ctx *ctx, const
>> struct xport *xport,
>> struct netdev_tnl_build_header_params tnl_params;
>> struct ovs_action_push_tnl tnl_push_data;
>> struct xport *out_dev = NULL;
>> - ovs_be32 s_ip = 0, d_ip = 0;
>> + ovs_be32 s_ip = 0, d_ip = 0, neigh_s_ip = 0;
>
>It’s not really a neighbor source IP, I would call it nh_s_ip (nh = next-hop).
>
>> struct in6_addr s_ip6 = in6addr_any;
>> struct in6_addr d_ip6 = in6addr_any;
>> + struct in6_addr neigh_s_ip6 = in6addr_any;
>> struct eth_addr smac;
>> struct eth_addr dmac;
>> int err;
>> @@ -3646,8 +3647,13 @@ native_tunnel_output(struct xlate_ctx *ctx, const
>> struct xport *xport,
>> }
>>
>> d_ip = in6_addr_get_mapped_ipv4(&d_ip6);
>> + err = get_src_addr(&d_ip6, out_dev->xbridge->name, &neigh_s_ip6);
>> + if (err) {
>> + neigh_s_ip6 = s_ip6;
>> + }
>> if (d_ip) {
>> s_ip = in6_addr_get_mapped_ipv4(&s_ip6);
>> + neigh_s_ip = in6_addr_get_mapped_ipv4(&neigh_s_ip6);
>> }
>>
>> err = tnl_neigh_lookup(out_dev->xbridge->name, &d_ip6, &dmac);
>> @@ -3657,9 +3663,9 @@ native_tunnel_output(struct xlate_ctx *ctx, const
>> struct xport *xport,
>> "sending %s request",
>> buf_dip6, out_dev->xbridge->name, d_ip ? "ARP" : "ND");
>
>Maybe we can move all the d_ip handling to the “error” path? This makes it
>more compact. For example:
>
>
>
>diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>index cc9c1c628..e416a15be 100644
>--- a/ofproto/ofproto-dpif-xlate.c
>+++ b/ofproto/ofproto-dpif-xlate.c
>@@ -3673,14 +3673,25 @@ native_tunnel_output(struct xlate_ctx *ctx, const
>struct xport *xport,
>
> err = tnl_neigh_lookup(out_dev->xbridge->name, &d_ip6, &dmac);
> if (err) {
>+ struct in6_addr nh_s_ip6 = in6addr_any;
>+
> xlate_report(ctx, OFT_DETAIL,
> "neighbor cache miss for %s on bridge %s, "
> "sending %s request",
> buf_dip6, out_dev->xbridge->name, d_ip ? "ARP" : "ND");
>+
>+ err = get_src_addr(&d_ip6, out_dev->xbridge->name, &nh_s_ip6);
>+ if (err) {
>+ nh_s_ip6 = s_ip6;
>+ }
>+
> if (d_ip) {
>- tnl_send_arp_request(ctx, out_dev, smac, s_ip, d_ip);
>+ ovs_be32 nh_s_ip;
>+
>+ nh_s_ip = in6_addr_get_mapped_ipv4(&nh_s_ip6);
>+ tnl_send_arp_request(ctx, out_dev, smac, nh_s_ip, d_ip);
> } else {
>- tnl_send_nd_request(ctx, out_dev, smac, &s_ip6, &d_ip6);
>+ tnl_send_nd_request(ctx, out_dev, smac, &nh_s_ip6, &d_ip6);
> }
> return err;
> }
>
>
>
>
>> if (d_ip) {
>> - tnl_send_arp_request(ctx, out_dev, smac, s_ip, d_ip);
>> + tnl_send_arp_request(ctx, out_dev, smac, neigh_s_ip, d_ip);
>> } else {
>> - tnl_send_nd_request(ctx, out_dev, smac, &s_ip6, &d_ip6);
>> + tnl_send_nd_request(ctx, out_dev, smac, &neigh_s_ip6, &d_ip6);
>> }
>> return err;
>> }
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> 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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
