On 5 Feb 2025, at 11:42, Eelco Chaudron wrote:
> On 5 Feb 2025, at 11:26, Kevin Traynor wrote: > >> On 03/02/2025 15:20, Eelco Chaudron wrote: >>> >>> >>> 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? >>> >> >> Hi Eelco, I don't have history on this either. I agree some of the data >> structs/tracing and ovs_router_insert() etc are still using 'bridge'. >> Also, the set commands (below) are looking for BRIDGE and using that in >> tnl_neigh_set(). >> >> If there is additional information really needed, perhaps it could be >> added and shown with a '-v' option like the tnl/ports/show ? That would >> avoid changing existing user commands/output. >> >> # ovs-appctl list-commands | grep tnl >> tnl/arp/aging [SECS] >> tnl/arp/flush >> tnl/arp/set BRIDGE IP MAC >> tnl/arp/show >> tnl/egress_port_range min max >> tnl/neigh/aging [SECS] >> tnl/neigh/flush >> tnl/neigh/set BRIDGE IP MAC >> tnl/neigh/show >> tnl/ports/show -v > > I didn’t spend too much time on this, but we also need to ensure that the > port is part of the (correct) bridge. When it is removed or added, we must > handle all the additional processing, including revalidation and other > necessary steps. That’s why I believe the original design only supported the > IP on the bridge interface. Hi Roi, Ilya made me aware of the below commit, so I’ll try to take another look later this week, or next week. dc0bd12f5b04 ("userspace: Enable non-bridge port as tunnel endpoint.") Cheers, Eelco _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
