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.
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
