On 3 Feb 2025, at 14:33, Roi Dayan wrote:
> On 31/01/2025 12:25, Eelco Chaudron wrote: >> >> >> On 20 Jan 2025, at 11:12, Roi Dayan via dev wrote: >> >>> From: Eli Britstein <[email protected]> >>> >>> The tnl/neigh table was used only for the bridge, even if the tunnel's >>> underlay port is not the bridge's one (internal port for example). >>> Set it to use the correct port. >>> >>> Signed-off-by: Eli Britstein <[email protected]> >>> Acked-by: Roi Dayan <[email protected]> >> >> Hi Eli, >> >> Can you give a better explanation/exmaple why we need this change? I assume >> we need a test case for this anyway. >> >> Quickly looking at the code, it assumes that the bridge’s internal port >> (which has the same name as the bridge), which has the IP address, is used. >> >> Cheers, >> >> Eelco > > > Hi Eelco, > > Eli is on vacation I'll try to answer here. > The case could be when the user set ip not on the bridge netdev which has the > bridge name but a different port. e.g. ip is set on a new internal port with > vlan on it. I think this is not what is supported. Looking at the code and all the data structures, it all references the bridge and not any port on a bridge. Maybe other people have more history on this? > Then it's more correct to show the interface name rather than the bridge name > for better visibility. > There is no need for a new test case. the existing test is updated to check > the output is the port name and not bridge name. > > Thanks, > Roi > >> >>> --- >>> lib/tnl-neigh-cache.c | 4 ++-- >>> ofproto/ofproto-dpif-xlate.c | 11 ++++++----- >>> tests/tunnel-push-pop.at | 2 +- >>> 3 files changed, 9 insertions(+), 8 deletions(-) >>> >>> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c >>> index bdff1debc805..7e2eaa8262bf 100644 >>> --- a/lib/tnl-neigh-cache.c >>> +++ b/lib/tnl-neigh-cache.c >>> @@ -375,8 +375,8 @@ tnl_neigh_cache_show(struct unixctl_conn *conn, int >>> argc OVS_UNUSED, >>> struct ds ds = DS_EMPTY_INITIALIZER; >>> struct tnl_neigh_entry *neigh; >>> >>> - ds_put_cstr(&ds, "IP MAC >>> Bridge\n"); >>> - ds_put_cstr(&ds, >>> "==========================================================================\n"); >>> + ds_put_cstr(&ds, "IP MAC >>> Port\n"); >>> + ds_put_cstr(&ds, >>> "========================================================================\n"); >>> ovs_mutex_lock(&mutex); >>> CMAP_FOR_EACH(neigh, cmap_node, &table) { >>> int start_len, need_ws; >>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c >>> index 4cc7001a5b25..d13547b4a7f9 100644 >>> --- a/ofproto/ofproto-dpif-xlate.c >>> +++ b/ofproto/ofproto-dpif-xlate.c >>> @@ -3916,17 +3916,18 @@ native_tunnel_output(struct xlate_ctx *ctx, const >>> struct xport *xport, >>> s_ip = in6_addr_get_mapped_ipv4(&s_ip6); >>> } >>> >>> - err = tnl_neigh_lookup(out_dev->xbridge->name, &d_ip6, &dmac); >>> + err = tnl_neigh_lookup(netdev_get_name(out_dev->netdev), &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"); >>> + buf_dip6, netdev_get_name(out_dev->netdev), >>> + d_ip ? "ARP" : "ND"); >>> >>> err = ovs_router_get_netdev_source_address(&d_ip6, >>> - out_dev->xbridge->name, >>> + >>> netdev_get_name(out_dev->netdev), >>> &nh_s_ip6); >>> if (err) { >>> nh_s_ip6 = s_ip6; >>> @@ -4414,7 +4415,7 @@ terminate_native_tunnel(struct xlate_ctx *ctx, const >>> struct xport *xport, >>> * do tunnel neighbor snooping. */ >>> if (*tnl_port == ODPP_NONE && >>> (check_neighbor_reply(ctx, flow) || is_garp(flow, wc))) { >>> - tnl_neigh_snoop(flow, wc, ctx->xbridge->name, >>> + tnl_neigh_snoop(flow, wc, netdev_get_name(xport->netdev), >>> ctx->xin->allow_side_effects); >>> } else if (*tnl_port != ODPP_NONE && >>> ctx->xin->allow_side_effects && >>> @@ -4428,7 +4429,7 @@ terminate_native_tunnel(struct xlate_ctx *ctx, const >>> struct xport *xport, >>> s_ip6 = flow->ipv6_src; >>> } >>> >>> - tnl_neigh_set(ctx->xbridge->name, &s_ip6, mac); >>> + tnl_neigh_set(netdev_get_name(xport->netdev), &s_ip6, mac); >>> } >>> } >>> >>> diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at >>> index cf4e622014b2..b6635d09afbd 100644 >>> --- a/tests/tunnel-push-pop.at >>> +++ b/tests/tunnel-push-pop.at >>> @@ -1153,7 +1153,7 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p0 >>> 'recirc_id(0),in_port(1),dnl >>> >>> arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=aa:55:aa:55:00:03)']) >>> >>> AT_CHECK([ovs-appctl tnl/neigh/show | tail -n+3 | sort], [0], [dnl >>> -1.1.2.92 f8:bc:12:44:34:b6 br0 >>> +1.1.2.92 f8:bc:12:44:34:b6 vtep0 >>> ]) >>> >>> dnl Check GRE tunnel pop. >>> -- >>> 2.21.0 >>> >>> _______________________________________________ >>> 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
