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

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 10 Apr 2025 15:44:55 +0800 Jiawen Wu wrote:
> +	if (ti->type != UDP_TUNNEL_TYPE_VXLAN &&
> +	    ti->type != UDP_TUNNEL_TYPE_VXLAN_GPE &&
> +	    ti->type != UDP_TUNNEL_TYPE_GENEVE)
> +		return -EINVAL;

Why are you doing this validation?

> +	switch (ti->type) {
> +	case UDP_TUNNEL_TYPE_VXLAN:
> +		if (txgbe->vxlan_port != ti->port) {
> +			wx_err(wx, "VXLAN port %d not found\n", ti->port);
> +			return -EINVAL;
> +		}

And what is this check for? Is the core code calling your driver with
incorrect info? Please don't do this sort of defensive programming.
This patch is >50% pointless checks :(

> +		txgbe->vxlan_port = 0;
> +		break;
> +	case UDP_TUNNEL_TYPE_VXLAN_GPE:
> +		if (txgbe->vxlan_gpe_port != ti->port) {
> +			wx_err(wx, "VXLAN-GPE port %d not found\n", ti->port);
> +			return -EINVAL;
> +		}
> +
> +		txgbe->vxlan_gpe_port = 0;
> +		break;
> +	case UDP_TUNNEL_TYPE_GENEVE:
> +		if (txgbe->geneve_port != ti->port) {
> +			wx_err(wx, "GENEVE port %d not found\n", ti->port);
> +			return -EINVAL;
> +		}
> +
> +		txgbe->geneve_port = 0;
> +		break;
> +	default:
> +		return -EINVAL;

Also pointless. Unless you can show me in the core how it could
possibly call your driver with a table type that you didn't declare as
supported.
-- 
pw-bot: cr