Re: [PATCH net v1] team: fix header_ops type confusion with non-Ethernet ports

[Jiayuan Chen <jiayuan.chen@linux.dev>]

On 3/14/26 7:14 PM, Eric Dumazet wrote:
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 707419270ebf217a71b0593880c7a9a1481b7171..33f414d03ab913c58cf2406a4ab25e611c528159
> 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1530,9 +1530,11 @@ static int bond_header_create(struct sk_buff
> *skb, struct net_device *bond_dev,
>          return ret;
>   }
>
> -static int bond_header_parse(const struct sk_buff *skb, unsigned char *haddr)
> +static int bond_header_parse(const struct sk_buff *skb,
> +                            const struct net_device *dev,
> +                            unsigned char *haddr)
>   {
> -       struct bonding *bond = netdev_priv(skb->dev);
> +       struct bonding *bond = netdev_priv(dev);
>          const struct header_ops *slave_ops;
>          struct slave *slave;
>          int ret = 0;
> @@ -1542,7 +1544,7 @@ static int bond_header_parse(const struct
> sk_buff *skb, unsigned char *haddr)
>          if (slave) {
>                  slave_ops = READ_ONCE(slave->dev->header_ops);
>                  if (slave_ops && slave_ops->parse)
> -                       ret = slave_ops->parse(skb, haddr);
> +                       ret = slave_ops->parse(skb, slave->dev, haddr);
>          }
>          rcu_read_unlock();
>          return ret;
> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> index 9a1eacf35d37087ba8877bf31c017445929041ed..df8f88f63a7063fbd1df5248d2fc02c859a7bc74
> 100644
> --- a/include/linux/etherdevice.h
> +++ b/include/linux/etherdevice.h
> @@ -42,7 +42,8 @@ extern const struct header_ops eth_header_ops;
>
>   int eth_header(struct sk_buff *skb, struct net_device *dev, unsigned
> short type,
>                 const void *daddr, const void *saddr, unsigned len);
> -int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr);
> +int eth_header_parse(const struct sk_buff *skb, const struct net_device *dev,
> +                    unsigned char *haddr);
>   int eth_header_cache(const struct neighbour *neigh, struct hh_cache *hh,
>                       __be16 type);
>   void eth_header_cache_update(struct hh_cache *hh, const struct net_device *dev,
> diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
> index 61b7335aa037c7232a0caa45572043057c02dde3..ca9afa824aa4faf832658043bda6fb430633e476
> 100644
> --- a/include/linux/if_ether.h
> +++ b/include/linux/if_ether.h
> @@ -40,7 +40,8 @@ static inline struct ethhdr *inner_eth_hdr(const
> struct sk_buff *skb)
>          return (struct ethhdr *)skb_inner_mac_header(skb);
>   }
>
> -int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr);
> +int eth_header_parse(const struct sk_buff *skb, const struct net_device *dev,
> +                    unsigned char *haddr);
>
>   extern ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len);
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index d7aac6f185bcab8a93a204c349272fc7c1b15ee7..7ca01eb3f7d2b22a188502583dc95121adff7cc9
> 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -311,7 +311,9 @@ struct header_ops {
>          int     (*create) (struct sk_buff *skb, struct net_device *dev,
>                             unsigned short type, const void *daddr,
>                             const void *saddr, unsigned int len);
> -       int     (*parse)(const struct sk_buff *skb, unsigned char *haddr);
> +       int     (*parse)(const struct sk_buff *skb,
> +                        const struct net_device *dev,
> +                        unsigned char *haddr);
>          int     (*cache)(const struct neighbour *neigh, struct
> hh_cache *hh, __be16 type);
>          void    (*cache_update)(struct hh_cache *hh,
>                                  const struct net_device *dev,
> @@ -3445,7 +3447,7 @@ static inline int dev_parse_header(const struct
> sk_buff *skb,
>
>          if (!dev->header_ops || !dev->header_ops->parse)
>                  return 0;
> -       return dev->header_ops->parse(skb, haddr);
> +       return dev->header_ops->parse(skb, dev, haddr);
>   }
>
>   static inline __be16 dev_parse_header_protocol(const struct sk_buff *skb)
> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
> index 13a63b48b7eeb896dfe98eb0070a261eed2c384b..9d159d1cc57d42747f794cdf43fe0ccaf04818b2
> 100644
> --- a/net/ethernet/eth.c
> +++ b/net/ethernet/eth.c
> @@ -198,7 +198,8 @@ EXPORT_SYMBOL(eth_type_trans);
>    * @skb: packet to extract header from
>    * @haddr: destination buffer
>    */
> -int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr)
> +int eth_header_parse(const struct sk_buff *skb, const struct net_device *dev,
> +                    unsigned char *haddr)
>   {
>          const struct ethhdr *eth = eth_hdr(skb);
>          memcpy(haddr, eth->h_source, ETH_ALEN);
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index e13244729ad8d5b1c2b9c483d25bff0e438134b5..35f0baa99d4092fd499a4795f7f52db33a1fe4e2
> 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -919,7 +919,8 @@ static int ipgre_header(struct sk_buff *skb,
> struct net_device *dev,
>          return -(t->hlen + sizeof(*iph));
>   }
>
> -static int ipgre_header_parse(const struct sk_buff *skb, unsigned char *haddr)
> +static int ipgre_header_parse(const struct sk_buff *skb, const struct
> net_device *dev,
> +                             unsigned char *haddr)
>   {
>          const struct iphdr *iph = (const struct iphdr *) skb_mac_header(skb);
>          memcpy(haddr, &iph->saddr, 4);



Thanks,

I reproduced the infinite recursion you described with stacked bonds
(bond1 -> bond0 -> gre0). With an AF_PACKET SOCK_DGRAM socket on bond1
and an inbound GRE packet, bond_header_parse() recurses endlessly
because skb->dev always points to bond1.

I can verify this patch, but changing the parse() signature touches
quite a few places beyond what's in the diff - net/mac802154/iface.c,
net/phonet/af_phonet.c, drivers/firewire/net.c all have their own
parse() implementations that need updating, plus eth_header_parse() is
declared in both etherdevice.h and if_ether.h.

I'm wondering if adding a separate callback (e.g. dev_parse) to
struct header_ops might be a cleaner approach - dev_parse_header()
would check for the new callback first and fall back to the existing
parse(). That way only bond (and team) need to implement it, and all
other drivers remain untouched. The tradeoff is one extra NULL check
in the receive path, which should be negligible.