[PATCH net-next 0/2] packet socket cleanup

[PATCH net-next 0/2] packet socket cleanup

[Joe Damato]

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

[PATCH net-next 1/2] net/packet: Reduce VLAN tag code duplication

[Joe Damato]

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

[PATCH net-next 2/2] net/packet: Use copy_safe_from_sockptr to dedupe code

[Joe Damato]

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