[PATCH net-next] ipvlan: implemente .parse_protocol hook function in ipvlan_header_ops

The .parse_protocol hook function in the ipvlan_header_ops structure is
not implemented. As a result, when the AF_PACKET family is used to send
packets, skb->protocol will be set to 0.
The IPVLAN device must be of the Ethernet type. Therefore, use
eth_header_parse_protocol function to obtain the protocol.

Signed-off-by: Zhengchao Shao <redacted>
---
 drivers/net/ipvlan/ipvlan_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 57c79f5f2991..f28fd7b6b708 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c

@@ -387,6 +387,7 @@ static const struct header_ops ipvlan_header_ops = {
 	.parse		= eth_header_parse,
 	.cache		= eth_header_cache,
 	.cache_update	= eth_header_cache_update,
+	.parse_protocol	= eth_header_parse_protocol,
 };
 
 static void ipvlan_adjust_mtu(struct ipvl_dev *ipvlan, struct net_device *dev)

-- 
2.34.1

Zhengchao Shao wrote:

The .parse_protocol hook function in the ipvlan_header_ops structure is
not implemented. As a result, when the AF_PACKET family is used to send
packets, skb->protocol will be set to 0.
The IPVLAN device must be of the Ethernet type. Therefore, use
eth_header_parse_protocol function to obtain the protocol.

Signed-off-by: Zhengchao Shao <redacted>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Small typo in the subject line: implemente.

Ipvlan is a device of type ARPHRD_ETHER (ether_setup).

Tangential to this patch:

I checked that ipvlan_start_xmit indeed only expects packets with
skb->data at Ethernet header. ipvlan_queue_xmit checks

        if (unlikely(!pskb_may_pull(skb, sizeof(struct ethhdr))))
                goto out;

It may later call ipvlan_xmit_mode_l3 and ipvlan_get_L3_hdr, which
has such cases:

        case htons(ETH_P_IP): {
                u32 pktlen;
                struct iphdr *ip4h;
            
                if (unlikely(!pskb_may_pull(skb, sizeof(*ip4h))))
                        return NULL;

That pskb_may_pull should include the ethernet header. It gets
pulled for L3 mode in ipvlan_process_outbound, *after* the above.

On Thu, Nov 30, 2023 at 3:50 AM Zhengchao Shao [off-list ref] wrote:

The .parse_protocol hook function in the ipvlan_header_ops structure is
not implemented. As a result, when the AF_PACKET family is used to send
packets, skb->protocol will be set to 0.
The IPVLAN device must be of the Ethernet type. Therefore, use
eth_header_parse_protocol function to obtain the protocol.

Please add a Fixes: tag

Also, why macvlan would not need a similar patch ?

On 2023/12/1 2:00, Willem de Bruijn wrote:

Zhengchao Shao wrote:

quoted

The .parse_protocol hook function in the ipvlan_header_ops structure is
not implemented. As a result, when the AF_PACKET family is used to send
packets, skb->protocol will be set to 0.
The IPVLAN device must be of the Ethernet type. Therefore, use
eth_header_parse_protocol function to obtain the protocol.

Signed-off-by: Zhengchao Shao <redacted>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Small typo in the subject line: implemente.

Ipvlan is a device of type ARPHRD_ETHER (ether_setup).

Hi Willem:
	Thank you for your review. I will send v2.

Zhengchao Shao

Tangential to this patch:

I checked that ipvlan_start_xmit indeed only expects packets with
skb->data at Ethernet header. ipvlan_queue_xmit checks

         if (unlikely(!pskb_may_pull(skb, sizeof(struct ethhdr))))
                 goto out;

It may later call ipvlan_xmit_mode_l3 and ipvlan_get_L3_hdr, which
has such cases:

         case htons(ETH_P_IP): {
                 u32 pktlen;
                 struct iphdr *ip4h;
             
                 if (unlikely(!pskb_may_pull(skb, sizeof(*ip4h))))
                         return NULL;

That pskb_may_pull should include the ethernet header. It gets
pulled for L3 mode in ipvlan_process_outbound, *after* the above.

quoted

---
  drivers/net/ipvlan/ipvlan_main.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 57c79f5f2991..f28fd7b6b708 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c

@@ -387,6 +387,7 @@ static const struct header_ops ipvlan_header_ops = {
  	.parse		= eth_header_parse,
  	.cache		= eth_header_cache,
  	.cache_update	= eth_header_cache_update,
+	.parse_protocol	= eth_header_parse_protocol,
  };
  
  static void ipvlan_adjust_mtu(struct ipvl_dev *ipvlan, struct net_device *dev)

-- 
2.34.1

On 2023/12/1 2:05, Eric Dumazet wrote:

On Thu, Nov 30, 2023 at 3:50 AM Zhengchao Shao [off-list ref] wrote:

quoted

The .parse_protocol hook function in the ipvlan_header_ops structure is
not implemented. As a result, when the AF_PACKET family is used to send
packets, skb->protocol will be set to 0.
The IPVLAN device must be of the Ethernet type. Therefore, use
eth_header_parse_protocol function to obtain the protocol.

Please add a Fixes: tag

Hi Eric:
	Thank you for your reply. I will add it in v2.

Also, why macvlan would not need a similar patch ?

   Yes, I think macvlan also need to get protocol, although the protocol
is not used in TX of the macvlan driver. I will make a patch later.

Zhengchao Shao

quoted

Signed-off-by: Zhengchao Shao <redacted>
---
  drivers/net/ipvlan/ipvlan_main.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 57c79f5f2991..f28fd7b6b708 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c

@@ -387,6 +387,7 @@ static const struct header_ops ipvlan_header_ops = {
         .parse          = eth_header_parse,
         .cache          = eth_header_cache,
         .cache_update   = eth_header_cache_update,
+       .parse_protocol = eth_header_parse_protocol,
  };

  static void ipvlan_adjust_mtu(struct ipvl_dev *ipvlan, struct net_device *dev)
--

2.34.1