RE: [PATCH net-next v2 1/2] net: txgbe: Support to set UDP tunnel port

["Jiawen Wu" <jiawenwu@trustnetic.com>]

On Wed, Apr 16, 2025 8:33 AM, Jakub Kicinski wrote:
> On Mon, 14 Apr 2025 17:10:21 +0800 Jiawen Wu wrote:
> > +	udp_tunnel_nic_reset_ntf(netdev);
> 
> you issue the reset here, without clearing the ports...
> 
> >  	return 0;
> >
> >  err_free_irq:
> > @@ -537,6 +540,87 @@ void txgbe_do_reset(struct net_device *netdev)
> >  		txgbe_reset(wx);
> >  }
> >
> > +static int txgbe_udp_tunnel_set(struct net_device *dev, unsigned int table,
> > +				unsigned int entry, struct udp_tunnel_info *ti)
> > +{
> > +	struct wx *wx = netdev_priv(dev);
> > +	struct txgbe *txgbe = wx->priv;
> > +
> > +	switch (ti->type) {
> > +	case UDP_TUNNEL_TYPE_VXLAN:
> > +		if (txgbe->vxlan_port == ti->port)
> 
> then you ignore the set if the port is already the same.
> 
> Why do you need the udp_tunnel_nic_reset_ntf() call?
> Read the kdoc on that function.
> It doesn't seem like your NIC loses the state.
> 
> > +			break;
> > +
> > +		if (txgbe->vxlan_port) {
> > +			wx_err(wx, "VXLAN port %d set, not adding port %d\n",
> > +			       txgbe->vxlan_port, ti->port);
> > +			return -EINVAL;
> > +		}
> 
> Why...
> 
> > +		txgbe->vxlan_port = ti->port;
> > +		wr32(wx, TXGBE_CFG_VXLAN, ntohs(ti->port));
> > +		break;
> 
> > +static const struct udp_tunnel_nic_info txgbe_udp_tunnels = {
> > +	.set_port	= txgbe_udp_tunnel_set,
> > +	.unset_port	= txgbe_udp_tunnel_unset,
> > +	.flags		= UDP_TUNNEL_NIC_INFO_MAY_SLEEP,
> 
> Where do the callbacks sleep?

I now understand how the "reset" works, it will be changed to
UDP_TUNNEL_NIC_INFO_OPEN_ONLY, and use .sync_table to simplify the flow.