Re: [PATCH net-next 1/2] geneve: convert config to RCU-protected pointer

[Paolo Abeni <pabeni@redhat.com>]

On 7/1/26 2:04 PM, Eric Dumazet wrote:
> @@ -1137,21 +1144,22 @@ static int geneve_sock_add(struct geneve_dev *geneve, bool ipv6)
>  static int geneve_open(struct net_device *dev)
>  {
>  	struct geneve_dev *geneve = netdev_priv(dev);
> -	bool dualstack = geneve->cfg.dualstack;
> +	const struct geneve_config *cfg = rtnl_dereference(geneve->cfg);

Minor nit only mentioned because a repost is likely needed: here and in
a few other places the variable declaration order is not respected.

[...]
> @@ -1539,28 +1551,36 @@ static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  static netdev_tx_t geneve_xmit(struct sk_buff *skb, struct net_device *dev)
>  {
>  	struct geneve_dev *geneve = netdev_priv(dev);
> -	struct ip_tunnel_info *info = NULL;
> +	const struct ip_tunnel_info *info = NULL;
> +	const struct geneve_config *cfg;
>  	int err;
>  
> -	if (geneve->cfg.collect_md) {
> +	rcu_read_lock();
> +	cfg = rcu_dereference(geneve->cfg);
> +	if (unlikely(!cfg)) {

I also *think* the conditional is not needed here.

[...]
> @@ -1717,6 +1790,8 @@ static void geneve_setup(struct net_device *dev)
>  	dev->netdev_ops = &geneve_netdev_ops;
>  	dev->ethtool_ops = &geneve_ethtool_ops;
>  	dev->needs_free_netdev = true;
> +	dev->priv_destructor = geneve_free_dev;

Both sashikos point out this may cause double free.

Side note: I think that ideally it would be good to break this patch into 2:
- passing an explicit `const struct geneve_config *` argument where needed.
- doing the rcu conversion.

/P