This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible.
For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- net: dsa: add NETC switch tag support This commit introduces support for the NXP NETC switch tag protocol, a proprietary header used by NETC switches. The tag supports three types (Forward, To_Port, and To_Host) with variable length (6-14 bytes). > diff --git a/include/linux/dsa/tag_netc.h b/include/linux/dsa/tag_netc.h > new file mode 100644 > index 000000000000..fe964722e5b0 > --- /dev/null > +++ b/include/linux/dsa/tag_netc.h > @@ -0,0 +1,14 @@ > +/* SPDX-License-Identifier: GPL-2.0 > + * > + * Copyright 2025-2026 NXP > + */ > + > +#ifndef __NET_DSA_TAG_NETC_H > +#define __NET_DSA_TAG_NETC_H > + > +#include <linux/skbuff.h> > +#include <net/dsa.h> > + > +#define NETC_TAG_MAX_LEN 14 > + > +#endif [ ... ] > diff --git a/net/dsa/tag_netc.c b/net/dsa/tag_netc.c > new file mode 100644 > index 000000000000..15b0dd5c5408 > --- /dev/null > +++ b/net/dsa/tag_netc.c [ ... ] > @@ -0,0 +1,180 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright 2025-2026 NXP > + */ > + > +#include <linux/dsa/tag_netc.h> > + > +#include "tag.h" > + > +#define NETC_NAME "nxp_netc" > + > +/* Forward NXP switch tag */ > +#define NETC_TAG_FORWARD 0 > + > +/* To_Port NXP switch tag */ > +#define NETC_TAG_TO_PORT 1 > +/* SubType0: No request to perform timestamping */ > +#define NETC_TAG_TP_SUBTYPE0 0 > + > +/* To_Host NXP switch tag */ > +#define NETC_TAG_TO_HOST 2 > +/* SubType0: frames redirected or copied to CPU port */ > +#define NETC_TAG_TH_SUBTYPE0 0 > +/* SubType1: frames redirected or copied to CPU port with timestamp */ > +#define NETC_TAG_TH_SUBTYPE1 1 > +/* SubType2: Transmit timestamp response (two-step timestamping) */ > +#define NETC_TAG_TH_SUBTYPE2 2 > + > +/* NETC switch tag lengths */ > +#define NETC_TAG_FORWARD_LEN 6 > +#define NETC_TAG_TP_SUBTYPE0_LEN 6 > +#define NETC_TAG_TH_SUBTYPE0_LEN 6 > +#define NETC_TAG_TH_SUBTYPE1_LEN 14 > +#define NETC_TAG_TH_SUBTYPE2_LEN 14 > +#define NETC_TAG_CMN_LEN 5 > + > +#define NETC_TAG_SUBTYPE GENMASK(3, 0) > +#define NETC_TAG_TYPE GENMASK(7, 4) > +#define NETC_TAG_QV BIT(0) > +#define NETC_TAG_IPV GENMASK(4, 2) > +#define NETC_TAG_SWITCH GENMASK(2, 0) > +#define NETC_TAG_PORT GENMASK(7, 3) > + > +struct netc_tag_cmn { > + __be16 tpid; > + u8 type; > + u8 qos; > + u8 switch_port; > +} __packed; [ ... ] > +static int netc_get_rx_tag_len(int rx_type) > +{ > + int type = FIELD_GET(NETC_TAG_TYPE, rx_type); > + > + if (type == NETC_TAG_TO_HOST) { > + u8 subtype = rx_type & NETC_TAG_SUBTYPE; > + > + if (subtype == NETC_TAG_TH_SUBTYPE1) > + return NETC_TAG_TH_SUBTYPE1_LEN; > + else if (subtype == NETC_TAG_TH_SUBTYPE2) > + return NETC_TAG_TH_SUBTYPE2_LEN; > + else > + return NETC_TAG_TH_SUBTYPE0_LEN; > + } > + > + return NETC_TAG_FORWARD_LEN; > +} > + > +static struct sk_buff *netc_rcv(struct sk_buff *skb, > + struct net_device *ndev) > +{ > + struct netc_tag_cmn *tag_cmn = dsa_etype_header_pos_rx(skb); > + int tag_len = netc_get_rx_tag_len(tag_cmn->type); ^^^^^^^^^^^^ Is the tag header guaranteed to be in the linear region of the skb? Looking at other EtherType-based DSA tag drivers (tag_ar9331, tag_lan9303, tag_dsa, tag_edsa, tag_a5psw, tag_brcm_legacy, tag_mxl-gsw1xx, tag_gswip, and tag_mtk), they all call pskb_may_pull() before accessing tag data to ensure the header is linearized. For example, tag_ar9331.c: static struct sk_buff *ar9331_tag_rcv(struct sk_buff *skb, struct net_device *ndev) { if (unlikely(!pskb_may_pull(skb, AR9331_HDR_LEN))) return NULL; ... } Similarly, tag_lan9303.c: static struct sk_buff *lan9303_rcv(struct sk_buff *skb, struct net_device *ndev) { if (unlikely(!pskb_may_pull(skb, LAN9303_TAG_LEN))) return NULL; ... } Without this check, netc_rcv() accesses tag_cmn->type, tag_cmn->tpid, tag_cmn->qos, and tag_cmn->switch_port before verifying these bytes are linearized. The struct netc_tag_cmn is 5 bytes, with 3 bytes at skb->data through skb->data+2 that may not be guaranteed to be in the linear region. Additionally, netc_get_rx_tag_len() can return up to 14 bytes (NETC_TAG_TH_SUBTYPE1_LEN or NETC_TAG_TH_SUBTYPE2_LEN), and this value is passed directly to skb_pull_rcsum(): > + int tag_len = netc_get_rx_tag_len(tag_cmn->type); > + int sw_id, port; > + > + if (ntohs(tag_cmn->tpid) != ETH_P_NXP_NETC) { > + dev_warn_ratelimited(&ndev->dev, "Unknown TPID 0x%04x\n", > + ntohs(tag_cmn->tpid)); > + > + return NULL; > + } > + > + if (tag_cmn->qos & NETC_TAG_QV) > + skb->priority = FIELD_GET(NETC_TAG_IPV, tag_cmn->qos); > + > + sw_id = NETC_TAG_SWITCH & tag_cmn->switch_port; > + /* ENETC VEPA switch ID (0) is not supported yet */ > + if (!sw_id) { > + dev_warn_ratelimited(&ndev->dev, > + "VEPA switch ID is not supported yet\n"); > + > + return NULL; > + } > + > + port = FIELD_GET(NETC_TAG_PORT, tag_cmn->switch_port); > + skb->dev = dsa_conduit_find_user(ndev, sw_id, port); > + if (!skb->dev) > + return NULL; > + > + if (tag_cmn->type == NETC_TAG_FORWARD) > + dsa_default_offload_fwd_mark(skb); > + > + /* Remove Switch tag from the frame */ > + skb_pull_rcsum(skb, tag_len); ^^^^^^^^^^^^^^^^^^ The skb_pull_rcsum() function contains BUG_ON(len > skb->len). If a truncated or malformed packet arrives with fewer than tag_len bytes remaining, the kernel will panic rather than gracefully handling the error. Should a check like this be added at the beginning of netc_rcv(): if (unlikely(!pskb_may_pull(skb, NETC_TAG_MAX_LEN - 2))) return NULL; The -2 would account for the EtherType bytes already consumed by eth_type_trans(). > + dsa_strip_etype_header(skb, tag_len); > + > + return skb; > +} [ ... ]
