* [PATCH net-next 0/2] packet socket cleanup @ 2026-08-11 18:47 Joe Damato 2026-08-11 18:47 ` [PATCH net-next 1/2] net/packet: Reduce VLAN tag code duplication Joe Damato 2026-08-11 18:47 ` [PATCH net-next 2/2] net/packet: Use copy_safe_from_sockptr to dedupe code Joe Damato 0 siblings, 2 replies; 5+ messages in thread From: Joe Damato @ 2026-08-11 18:47 UTC (permalink / raw) To: netdev Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, willemb, Joe Damato, linux-kernel, Willem de Bruijn Greetings: This series does some minor cleanup in the packet socket code to reduce code duplication and use existing helpers (like copy_safe_from_sockptr). No functional changes are introduced with this series. Thanks, Joe Joe Damato (2): net/packet: Reduce VLAN tag code duplication net/packet: Use copy_safe_from_sockptr to dedupe code net/packet/af_packet.c | 116 ++++++++++++++++++++++------------------- 1 file changed, 62 insertions(+), 54 deletions(-) base-commit: 31397cf1819210bd63fa3d2c7d8c24f7c8667d99 -- 2.53.0-Meta ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net-next 1/2] net/packet: Reduce VLAN tag code duplication 2026-08-11 18:47 [PATCH net-next 0/2] packet socket cleanup Joe Damato @ 2026-08-11 18:47 ` Joe Damato 2026-08-12 12:28 ` Willem de Bruijn 2026-08-11 18:47 ` [PATCH net-next 2/2] net/packet: Use copy_safe_from_sockptr to dedupe code Joe Damato 1 sibling, 1 reply; 5+ messages in thread From: Joe Damato @ 2026-08-11 18:47 UTC (permalink / raw) To: netdev, Willem de Bruijn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman Cc: andrew+netdev, willemb, Joe Damato, linux-kernel Reduce code duplication for VLAN tag extraction by factoring the repeated code into a helper and using it. Signed-off-by: Joe Damato <joe@dama.to> --- net/packet/af_packet.c | 95 +++++++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 42 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 435756877aba..ee60dcc639ad 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -568,6 +568,27 @@ static u16 vlan_get_tci(const struct sk_buff *skb, struct net_device *dev) return ntohs(vh->h_vlan_TCI); } +static bool packet_get_vlan_tci_tpid(const struct sk_buff *skb, + struct net_device *dev, + u16 *tci, u16 *tpid) +{ + if (skb_vlan_tag_present(skb)) { + *tci = skb_vlan_tag_get(skb); + *tpid = ntohs(skb->vlan_proto); + return true; + } + + if (unlikely(dev && eth_type_vlan(skb->protocol))) { + *tci = vlan_get_tci(skb, dev); + *tpid = ntohs(skb->protocol); + return true; + } + + *tci = 0; + *tpid = 0; + return false; +} + static __be16 vlan_get_protocol_dgram(const struct sk_buff *skb) { __be16 proto = skb->protocol; @@ -997,20 +1018,19 @@ static void prb_fill_vlan_info(struct tpacket_kbdq_core *pkc, struct tpacket3_hdr *ppd) { struct packet_sock *po = container_of(pkc, struct packet_sock, rx_ring.prb_bdqc); + struct net_device *dev = NULL; + u16 tci, tpid; - if (skb_vlan_tag_present(pkc->skb)) { - ppd->hv1.tp_vlan_tci = skb_vlan_tag_get(pkc->skb); - ppd->hv1.tp_vlan_tpid = ntohs(pkc->skb->vlan_proto); - ppd->tp_status = TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; - } else if (unlikely(po->sk.sk_type == SOCK_DGRAM && eth_type_vlan(pkc->skb->protocol))) { - ppd->hv1.tp_vlan_tci = vlan_get_tci(pkc->skb, pkc->skb->dev); - ppd->hv1.tp_vlan_tpid = ntohs(pkc->skb->protocol); + if (po->sk.sk_type == SOCK_DGRAM) + dev = pkc->skb->dev; + + if (packet_get_vlan_tci_tpid(pkc->skb, dev, &tci, &tpid)) ppd->tp_status = TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; - } else { - ppd->hv1.tp_vlan_tci = 0; - ppd->hv1.tp_vlan_tpid = 0; + else ppd->tp_status = TP_STATUS_AVAILABLE; - } + + ppd->hv1.tp_vlan_tci = tci; + ppd->hv1.tp_vlan_tpid = tpid; } static void prb_run_all_ft_ops(struct tpacket_kbdq_core *pkc, @@ -2247,6 +2267,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev) { enum skb_drop_reason drop_reason = SKB_CONSUMED; + struct net_device *vlan_dev = NULL; struct sock *sk = NULL; struct packet_sock *po; struct sockaddr_ll *sll; @@ -2262,6 +2283,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, __u32 ts_status; unsigned int slot_id = 0; int vnet_hdr_sz = 0; + u16 tci, tpid; /* struct tpacket{2,3}_hdr is aligned to a multiple of TPACKET_ALIGNMENT. * We may add members to them until current aligned size without forcing @@ -2436,18 +2458,12 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, h.h2->tp_net = netoff; h.h2->tp_sec = ts.tv_sec; h.h2->tp_nsec = ts.tv_nsec; - if (skb_vlan_tag_present(skb)) { - h.h2->tp_vlan_tci = skb_vlan_tag_get(skb); - h.h2->tp_vlan_tpid = ntohs(skb->vlan_proto); - status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; - } else if (unlikely(sk->sk_type == SOCK_DGRAM && eth_type_vlan(skb->protocol))) { - h.h2->tp_vlan_tci = vlan_get_tci(skb, skb->dev); - h.h2->tp_vlan_tpid = ntohs(skb->protocol); + if (sk->sk_type == SOCK_DGRAM) + vlan_dev = skb->dev; + if (packet_get_vlan_tci_tpid(skb, vlan_dev, &tci, &tpid)) status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; - } else { - h.h2->tp_vlan_tci = 0; - h.h2->tp_vlan_tpid = 0; - } + h.h2->tp_vlan_tci = tci; + h.h2->tp_vlan_tpid = tpid; memset(h.h2->tp_padding, 0, sizeof(h.h2->tp_padding)); hdrlen = sizeof(*h.h2); break; @@ -3547,7 +3563,9 @@ static int packet_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, } if (packet_sock_flag(pkt_sk(sk), PACKET_SOCK_AUXDATA)) { + struct net_device *vlan_dev = NULL; struct tpacket_auxdata aux; + u16 tci, tpid; aux.tp_status = TP_STATUS_USER; if (skb->ip_summed == CHECKSUM_PARTIAL) @@ -3562,29 +3580,22 @@ static int packet_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, aux.tp_snaplen = skb->len; aux.tp_mac = 0; aux.tp_net = skb_network_offset(skb); - if (skb_vlan_tag_present(skb)) { - aux.tp_vlan_tci = skb_vlan_tag_get(skb); - aux.tp_vlan_tpid = ntohs(skb->vlan_proto); - aux.tp_status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; - } else if (unlikely(sock->type == SOCK_DGRAM && eth_type_vlan(skb->protocol))) { + rcu_read_lock(); + if (unlikely(sock->type == SOCK_DGRAM && + !skb_vlan_tag_present(skb) && + eth_type_vlan(skb->protocol))) { struct sockaddr_ll *sll = &PACKET_SKB_CB(skb)->sa.ll; - struct net_device *dev; - rcu_read_lock(); - dev = dev_get_by_index_rcu(sock_net(sk), sll->sll_ifindex); - if (dev) { - aux.tp_vlan_tci = vlan_get_tci(skb, dev); - aux.tp_vlan_tpid = ntohs(skb->protocol); - aux.tp_status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; - } else { - aux.tp_vlan_tci = 0; - aux.tp_vlan_tpid = 0; - } - rcu_read_unlock(); - } else { - aux.tp_vlan_tci = 0; - aux.tp_vlan_tpid = 0; + vlan_dev = dev_get_by_index_rcu(sock_net(sk), + sll->sll_ifindex); } + if (packet_get_vlan_tci_tpid(skb, vlan_dev, &tci, &tpid)) + aux.tp_status |= TP_STATUS_VLAN_VALID | + TP_STATUS_VLAN_TPID_VALID; + rcu_read_unlock(); + + aux.tp_vlan_tci = tci; + aux.tp_vlan_tpid = tpid; put_cmsg(msg, SOL_PACKET, PACKET_AUXDATA, sizeof(aux), &aux); } -- 2.53.0-Meta ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 1/2] net/packet: Reduce VLAN tag code duplication 2026-08-11 18:47 ` [PATCH net-next 1/2] net/packet: Reduce VLAN tag code duplication Joe Damato @ 2026-08-12 12:28 ` Willem de Bruijn 0 siblings, 0 replies; 5+ messages in thread From: Willem de Bruijn @ 2026-08-12 12:28 UTC (permalink / raw) To: Joe Damato, netdev, Willem de Bruijn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman Cc: andrew+netdev, willemb, Joe Damato, linux-kernel Joe Damato wrote: > Reduce code duplication for VLAN tag extraction by factoring the > repeated code into a helper and using it. > > Signed-off-by: Joe Damato <joe@dama.to> Especially with the improving AI bots, we're getting even more fixes to PF_PACKET lately. Cleanup patches can block fix backports to stable. The bar for pure cleanup patches has to be high to warrant that. Plus, they add risk, if it is not trivial to review that they are NOOPs. Subjective, but not sure this one warrants the cost. > --- > net/packet/af_packet.c | 95 +++++++++++++++++++++++------------------- > 1 file changed, 53 insertions(+), 42 deletions(-) > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index 435756877aba..ee60dcc639ad 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c > @@ -568,6 +568,27 @@ static u16 vlan_get_tci(const struct sk_buff *skb, struct net_device *dev) > return ntohs(vh->h_vlan_TCI); > } > > +static bool packet_get_vlan_tci_tpid(const struct sk_buff *skb, > + struct net_device *dev, > + u16 *tci, u16 *tpid) > +{ > + if (skb_vlan_tag_present(skb)) { > + *tci = skb_vlan_tag_get(skb); > + *tpid = ntohs(skb->vlan_proto); > + return true; > + } > + > + if (unlikely(dev && eth_type_vlan(skb->protocol))) { > + *tci = vlan_get_tci(skb, dev); > + *tpid = ntohs(skb->protocol); > + return true; > + } > + > + *tci = 0; > + *tpid = 0; > + return false; > +} > + > static __be16 vlan_get_protocol_dgram(const struct sk_buff *skb) > { > __be16 proto = skb->protocol; > @@ -997,20 +1018,19 @@ static void prb_fill_vlan_info(struct tpacket_kbdq_core *pkc, > struct tpacket3_hdr *ppd) > { > struct packet_sock *po = container_of(pkc, struct packet_sock, rx_ring.prb_bdqc); > + struct net_device *dev = NULL; > + u16 tci, tpid; > > - if (skb_vlan_tag_present(pkc->skb)) { > - ppd->hv1.tp_vlan_tci = skb_vlan_tag_get(pkc->skb); > - ppd->hv1.tp_vlan_tpid = ntohs(pkc->skb->vlan_proto); > - ppd->tp_status = TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; > - } else if (unlikely(po->sk.sk_type == SOCK_DGRAM && eth_type_vlan(pkc->skb->protocol))) { > - ppd->hv1.tp_vlan_tci = vlan_get_tci(pkc->skb, pkc->skb->dev); > - ppd->hv1.tp_vlan_tpid = ntohs(pkc->skb->protocol); > + if (po->sk.sk_type == SOCK_DGRAM) > + dev = pkc->skb->dev; > + > + if (packet_get_vlan_tci_tpid(pkc->skb, dev, &tci, &tpid)) > ppd->tp_status = TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; Does this change behavior, now setting tp_status also for the second branch, where previously the flags were not set? > - } else { > - ppd->hv1.tp_vlan_tci = 0; > - ppd->hv1.tp_vlan_tpid = 0; > + else > ppd->tp_status = TP_STATUS_AVAILABLE; > - } > + > + ppd->hv1.tp_vlan_tci = tci; > + ppd->hv1.tp_vlan_tpid = tpid; > } > > static void prb_run_all_ft_ops(struct tpacket_kbdq_core *pkc, > @@ -2247,6 +2267,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, > struct packet_type *pt, struct net_device *orig_dev) > { > enum skb_drop_reason drop_reason = SKB_CONSUMED; > + struct net_device *vlan_dev = NULL; > struct sock *sk = NULL; > struct packet_sock *po; > struct sockaddr_ll *sll; > @@ -2262,6 +2283,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, > __u32 ts_status; > unsigned int slot_id = 0; > int vnet_hdr_sz = 0; > + u16 tci, tpid; > > /* struct tpacket{2,3}_hdr is aligned to a multiple of TPACKET_ALIGNMENT. > * We may add members to them until current aligned size without forcing > @@ -2436,18 +2458,12 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, > h.h2->tp_net = netoff; > h.h2->tp_sec = ts.tv_sec; > h.h2->tp_nsec = ts.tv_nsec; > - if (skb_vlan_tag_present(skb)) { > - h.h2->tp_vlan_tci = skb_vlan_tag_get(skb); > - h.h2->tp_vlan_tpid = ntohs(skb->vlan_proto); > - status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; > - } else if (unlikely(sk->sk_type == SOCK_DGRAM && eth_type_vlan(skb->protocol))) { > - h.h2->tp_vlan_tci = vlan_get_tci(skb, skb->dev); > - h.h2->tp_vlan_tpid = ntohs(skb->protocol); > + if (sk->sk_type == SOCK_DGRAM) > + vlan_dev = skb->dev; > + if (packet_get_vlan_tci_tpid(skb, vlan_dev, &tci, &tpid)) > status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; > - } else { > - h.h2->tp_vlan_tci = 0; > - h.h2->tp_vlan_tpid = 0; > - } > + h.h2->tp_vlan_tci = tci; > + h.h2->tp_vlan_tpid = tpid; > memset(h.h2->tp_padding, 0, sizeof(h.h2->tp_padding)); > hdrlen = sizeof(*h.h2); > break; > @@ -3547,7 +3563,9 @@ static int packet_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, > } > > if (packet_sock_flag(pkt_sk(sk), PACKET_SOCK_AUXDATA)) { > + struct net_device *vlan_dev = NULL; > struct tpacket_auxdata aux; > + u16 tci, tpid; > > aux.tp_status = TP_STATUS_USER; > if (skb->ip_summed == CHECKSUM_PARTIAL) > @@ -3562,29 +3580,22 @@ static int packet_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, > aux.tp_snaplen = skb->len; > aux.tp_mac = 0; > aux.tp_net = skb_network_offset(skb); > - if (skb_vlan_tag_present(skb)) { > - aux.tp_vlan_tci = skb_vlan_tag_get(skb); > - aux.tp_vlan_tpid = ntohs(skb->vlan_proto); > - aux.tp_status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; > - } else if (unlikely(sock->type == SOCK_DGRAM && eth_type_vlan(skb->protocol))) { > + rcu_read_lock(); > + if (unlikely(sock->type == SOCK_DGRAM && > + !skb_vlan_tag_present(skb) && > + eth_type_vlan(skb->protocol))) { > struct sockaddr_ll *sll = &PACKET_SKB_CB(skb)->sa.ll; > - struct net_device *dev; > > - rcu_read_lock(); > - dev = dev_get_by_index_rcu(sock_net(sk), sll->sll_ifindex); > - if (dev) { > - aux.tp_vlan_tci = vlan_get_tci(skb, dev); > - aux.tp_vlan_tpid = ntohs(skb->protocol); > - aux.tp_status |= TP_STATUS_VLAN_VALID | TP_STATUS_VLAN_TPID_VALID; > - } else { > - aux.tp_vlan_tci = 0; > - aux.tp_vlan_tpid = 0; > - } > - rcu_read_unlock(); > - } else { > - aux.tp_vlan_tci = 0; > - aux.tp_vlan_tpid = 0; > + vlan_dev = dev_get_by_index_rcu(sock_net(sk), > + sll->sll_ifindex); > } > + if (packet_get_vlan_tci_tpid(skb, vlan_dev, &tci, &tpid)) > + aux.tp_status |= TP_STATUS_VLAN_VALID | > + TP_STATUS_VLAN_TPID_VALID; > + rcu_read_unlock(); > + > + aux.tp_vlan_tci = tci; > + aux.tp_vlan_tpid = tpid; > put_cmsg(msg, SOL_PACKET, PACKET_AUXDATA, sizeof(aux), &aux); > } > > -- > 2.53.0-Meta > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net-next 2/2] net/packet: Use copy_safe_from_sockptr to dedupe code 2026-08-11 18:47 [PATCH net-next 0/2] packet socket cleanup Joe Damato 2026-08-11 18:47 ` [PATCH net-next 1/2] net/packet: Reduce VLAN tag code duplication Joe Damato @ 2026-08-11 18:47 ` Joe Damato 2026-08-12 12:19 ` Willem de Bruijn 1 sibling, 1 reply; 5+ messages in thread From: Joe Damato @ 2026-08-11 18:47 UTC (permalink / raw) To: netdev, Willem de Bruijn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman Cc: andrew+netdev, willemb, Joe Damato, linux-kernel Reduce code duplication by using copy_safe_from_sockptr instead of repeated length checks followed by a copy. Signed-off-by: Joe Damato <joe@dama.to> --- net/packet/af_packet.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index ee60dcc639ad..9d326c2b04b3 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -3961,10 +3961,9 @@ packet_setsockopt(struct socket *sock, int level, int optname, sockptr_t optval, { int val; - if (optlen < sizeof(val)) - return -EINVAL; - if (copy_from_sockptr(&val, optval, sizeof(val))) - return -EFAULT; + ret = copy_safe_from_sockptr(&val, sizeof(val), optval, optlen); + if (ret) + return ret; packet_sock_flag_set(po, PACKET_SOCK_AUXDATA, val); return 0; @@ -3973,10 +3972,9 @@ packet_setsockopt(struct socket *sock, int level, int optname, sockptr_t optval, { int val; - if (optlen < sizeof(val)) - return -EINVAL; - if (copy_from_sockptr(&val, optval, sizeof(val))) - return -EFAULT; + ret = copy_safe_from_sockptr(&val, sizeof(val), optval, optlen); + if (ret) + return ret; packet_sock_flag_set(po, PACKET_SOCK_ORIGDEV, val); return 0; @@ -3988,10 +3986,9 @@ packet_setsockopt(struct socket *sock, int level, int optname, sockptr_t optval, if (sock->type != SOCK_RAW) return -EINVAL; - if (optlen < sizeof(val)) - return -EINVAL; - if (copy_from_sockptr(&val, optval, sizeof(val))) - return -EFAULT; + ret = copy_safe_from_sockptr(&val, sizeof(val), optval, optlen); + if (ret) + return ret; if (optname == PACKET_VNET_HDR_SZ) { if (val && val != sizeof(struct virtio_net_hdr) && -- 2.53.0-Meta ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 2/2] net/packet: Use copy_safe_from_sockptr to dedupe code 2026-08-11 18:47 ` [PATCH net-next 2/2] net/packet: Use copy_safe_from_sockptr to dedupe code Joe Damato @ 2026-08-12 12:19 ` Willem de Bruijn 0 siblings, 0 replies; 5+ messages in thread From: Willem de Bruijn @ 2026-08-12 12:19 UTC (permalink / raw) To: Joe Damato, netdev, Willem de Bruijn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman Cc: andrew+netdev, willemb, Joe Damato, linux-kernel Joe Damato wrote: > Reduce code duplication by using copy_safe_from_sockptr instead of > repeated length checks followed by a copy. > > Signed-off-by: Joe Damato <joe@dama.to> Reviewed-by: Willem de Bruijn <willemb@google.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-12 12:28 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-11 18:47 [PATCH net-next 0/2] packet socket cleanup Joe Damato 2026-08-11 18:47 ` [PATCH net-next 1/2] net/packet: Reduce VLAN tag code duplication Joe Damato 2026-08-12 12:28 ` Willem de Bruijn 2026-08-11 18:47 ` [PATCH net-next 2/2] net/packet: Use copy_safe_from_sockptr to dedupe code Joe Damato 2026-08-12 12:19 ` Willem de Bruijn
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.