| Message ID | 20231130030225.3571231-1-shaozhengchao@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] ipvlan: implemente .parse_protocol hook function in ipvlan_header_ops | expand |
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 <shaozhengchao@huawei.com> 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. > --- > 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 Thu, Nov 30, 2023 at 3:50 AM Zhengchao Shao <shaozhengchao@huawei.com> 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 ? > Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> > --- > 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:00, Willem de Bruijn wrote: > 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 <shaozhengchao@huawei.com> > > 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. > >> --- >> 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 <shaozhengchao@huawei.com> 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 > 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 >> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> >> --- >> 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 >>
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)
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 <shaozhengchao@huawei.com> --- drivers/net/ipvlan/ipvlan_main.c | 1 + 1 file changed, 1 insertion(+)