| Message ID | 20231201025528.2216489-1-shaozhengchao@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] ipvlan: implement .parse_protocol hook function in ipvlan_header_ops | expand |
On 2023/12/1 10:55, 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. > Ipvlan is a device of type ARPHRD_ETHER (ether_setup). Therefore, use > eth_header_parse_protocol function to obtain the protocol. > > Fixes: 2ad7bf363841 ("ipvlan: Initial check-in of the IPVLAN driver.") Maybe Fixes should be: 75c65772c3d1 ("net/packet: Ask driver for protocol if not provided by user") > Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> > Reviewed-by: Willem de Bruijn <willemb@google.com> > --- > v2: modify commit info and add Fixes tag > --- > 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)
shaozhengchao wrote: > > > On 2023/12/1 10:55, 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. > > Ipvlan is a device of type ARPHRD_ETHER (ether_setup). Therefore, use > > eth_header_parse_protocol function to obtain the protocol. > > > > Fixes: 2ad7bf363841 ("ipvlan: Initial check-in of the IPVLAN driver.") > > Maybe Fixes should be: 75c65772c3d1 ("net/packet: Ask driver for > protocol if not provided by user") Definitely not anything older than the introduction of header_ops.parse_protocol. I gave my +1 when it targeted net-next, so imho this is not really stable material anyhow.
On 2023/12/1 23:22, Willem de Bruijn wrote: > shaozhengchao wrote: >> >> >> On 2023/12/1 10:55, 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. >>> Ipvlan is a device of type ARPHRD_ETHER (ether_setup). Therefore, use >>> eth_header_parse_protocol function to obtain the protocol. >>> >>> Fixes: 2ad7bf363841 ("ipvlan: Initial check-in of the IPVLAN driver.") >> >> Maybe Fixes should be: 75c65772c3d1 ("net/packet: Ask driver for >> protocol if not provided by user") > Hi Willem: > Definitely not anything older than the introduction of > header_ops.parse_protocol. > Yes, I think so. > I gave my +1 when it targeted net-next, so imho this is not really > stable material anyhow. But, if skb->protocol = 0, no matter what type of packet it is, it will be discarded directly in ipvlan_process_outbound(). So net branch will be OK? What I missed? Thanks. Zhengchao Shao
On Sat, Dec 2, 2023 at 3:14 AM shaozhengchao <shaozhengchao@huawei.com> wrote: > > > > On 2023/12/1 23:22, Willem de Bruijn wrote: > > shaozhengchao wrote: > >> > >> > >> On 2023/12/1 10:55, 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. > >>> Ipvlan is a device of type ARPHRD_ETHER (ether_setup). Therefore, use > >>> eth_header_parse_protocol function to obtain the protocol. > >>> > >>> Fixes: 2ad7bf363841 ("ipvlan: Initial check-in of the IPVLAN driver.") > >> > >> Maybe Fixes should be: 75c65772c3d1 ("net/packet: Ask driver for > >> protocol if not provided by user") > > > Hi Willem: > > Definitely not anything older than the introduction of > > header_ops.parse_protocol. > > > Yes, I think so. > > I gave my +1 when it targeted net-next, so imho this is not really > > stable material anyhow. > But, if skb->protocol = 0, no matter what type of packet it is, it > will be discarded directly in ipvlan_process_outbound(). > So net branch will be OK? What I missed? > Thanks. This never worked, and nobody ever claimed it has ever worked: this is a new functionality. net-next seems appropriate to me. It seems that skb->protocol == 0 is only used by fuzzers, or careless applications ?
On 2023/12/2 18:34, Eric Dumazet wrote: > On Sat, Dec 2, 2023 at 3:14 AM shaozhengchao <shaozhengchao@huawei.com> wrote: >> >> >> >> On 2023/12/1 23:22, Willem de Bruijn wrote: >>> shaozhengchao wrote: >>>> >>>> >>>> On 2023/12/1 10:55, 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. >>>>> Ipvlan is a device of type ARPHRD_ETHER (ether_setup). Therefore, use >>>>> eth_header_parse_protocol function to obtain the protocol. >>>>> >>>>> Fixes: 2ad7bf363841 ("ipvlan: Initial check-in of the IPVLAN driver.") >>>> >>>> Maybe Fixes should be: 75c65772c3d1 ("net/packet: Ask driver for >>>> protocol if not provided by user") >>> >> Hi Willem: >>> Definitely not anything older than the introduction of >>> header_ops.parse_protocol. >>> >> Yes, I think so. >>> I gave my +1 when it targeted net-next, so imho this is not really >>> stable material anyhow. >> But, if skb->protocol = 0, no matter what type of packet it is, it >> will be discarded directly in ipvlan_process_outbound(). >> So net branch will be OK? What I missed? >> Thanks. > > This never worked, and nobody ever claimed it has ever worked: this is > a new functionality. > > net-next seems appropriate to me. > > It seems that skb->protocol == 0 is only used by fuzzers, or careless > applications ? Yes. I will send v3. Thanks. Zhengchao Shao
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. Ipvlan is a device of type ARPHRD_ETHER (ether_setup). Therefore, use eth_header_parse_protocol function to obtain the protocol. Fixes: 2ad7bf363841 ("ipvlan: Initial check-in of the IPVLAN driver.") Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> Reviewed-by: Willem de Bruijn <willemb@google.com> --- v2: modify commit info and add Fixes tag --- drivers/net/ipvlan/ipvlan_main.c | 1 + 1 file changed, 1 insertion(+)