[can-next 0/5] can: remove private skb headroom infrastructure

[can-next 0/5] can: remove private skb headroom infrastructure

[Oliver Hartkopp]

CAN bus related skbuffs (ETH_P_CAN/ETH_P_CANFD/ETH_P_CANXL) simply contain
CAN frame structs for CAN CC/FD/XL of skb->len length at skb->data.
Those CAN skbs do not have network/mac/transport headers nor other such
references for encapsulated protocols like ethernet/IP protocols.

To store data for CAN specific use-cases all CAN bus related skbuffs are
created with a 16 byte private skb headroom (struct can_skb_priv).
Using the skb headroom and accessing skb->head for this private data
led to several problems in the past likely due to "The struct can_skb_priv
business is highly unconventional for the networking stack." [1]

This patch set aims to remove the unconventional skb headroom usage for
CAN bus related skbuffs. To store the data for CAN specific use-cases
unused space in CAN skbs is used, namely the inner protocol space for
ethernet/IP encapsulation.

[1] https://lore.kernel.org/linux-can/20260104074222.29e660ac@kernel.org/

Oliver Hartkopp (5):
  can: use skb hash instead of private variable in headroom
  can: move can_iif from private headroom to struct sk_buff
  can: move frame length from private headroom to struct sk_buff
  can: remove private skb headroom infrastructure
  can: gw: use new can_gw_hops variable instead of re-using csum_start

 drivers/net/can/dev/skb.c | 45 ++++++++++++++++-----------------------
 include/linux/can/core.h  |  1 +
 include/linux/can/skb.h   | 33 ----------------------------
 include/linux/skbuff.h    | 27 +++++++++++++++++------
 net/can/af_can.c          | 35 +++++++++++++++++++-----------
 net/can/bcm.c             | 13 ++++-------
 net/can/gw.c              | 25 ++++++----------------
 net/can/isotp.c           | 18 ++++++----------
 net/can/j1939/socket.c    |  7 ++----
 net/can/j1939/transport.c | 13 ++++-------
 net/can/raw.c             | 14 ++++++------
 11 files changed, 92 insertions(+), 139 deletions(-)

-- 
2.47.3

[can-next 1/5] can: use skb hash instead of private variable in headroom

[Oliver Hartkopp]

The can_skb_priv::skbcnt variable is used to identify CAN skbs in the rx
path analogue to the skb->hash. As the skb hash is not filled in CAN skbs
we can move the private skbcnt value to skb->hash and set skb->sw_hash
accordingly.

Patch 1/5 to remove the private CAN bus skb headroom infrastructure.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 drivers/net/can/dev/skb.c |  2 --
 include/linux/can/core.h  |  1 +
 include/linux/can/skb.h   |  2 --
 net/can/af_can.c          | 14 +++++++++++---
 net/can/bcm.c             |  2 --
 net/can/isotp.c           |  3 ---
 net/can/j1939/socket.c    |  1 -
 net/can/j1939/transport.c |  2 --
 net/can/raw.c             |  7 +++----
 9 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/net/can/dev/skb.c b/drivers/net/can/dev/skb.c
index 3ebd4f779b9b..0da615afa04d 100644
--- a/drivers/net/can/dev/skb.c
+++ b/drivers/net/can/dev/skb.c
@@ -200,11 +200,10 @@ static void init_can_skb_reserve(struct sk_buff *skb)
 	skb_reset_mac_header(skb);
 	skb_reset_network_header(skb);
 	skb_reset_transport_header(skb);
 
 	can_skb_reserve(skb);
-	can_skb_prv(skb)->skbcnt = 0;
 }
 
 struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf)
 {
 	struct sk_buff *skb;
@@ -310,11 +309,10 @@ static bool can_skb_headroom_valid(struct net_device *dev, struct sk_buff *skb)
 
 	/* af_packet does not apply CAN skb specific settings */
 	if (skb->ip_summed == CHECKSUM_NONE) {
 		/* init headroom */
 		can_skb_prv(skb)->ifindex = dev->ifindex;
-		can_skb_prv(skb)->skbcnt = 0;
 
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 		/* perform proper loopback on capable devices */
 		if (dev->flags & IFF_ECHO)
diff --git a/include/linux/can/core.h b/include/linux/can/core.h
index 5fb8d0e3f9c1..5c382ed61755 100644
--- a/include/linux/can/core.h
+++ b/include/linux/can/core.h
@@ -56,8 +56,9 @@ extern void can_rx_unregister(struct net *net, struct net_device *dev,
 			      canid_t can_id, canid_t mask,
 			      void (*func)(struct sk_buff *, void *),
 			      void *data);
 
 extern int can_send(struct sk_buff *skb, int loop);
+extern void can_set_skb_uid(struct sk_buff *skb);
 void can_sock_destruct(struct sock *sk);
 
 #endif /* !_CAN_CORE_H */
diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
index 1abc25a8d144..869ea574a40a 100644
--- a/include/linux/can/skb.h
+++ b/include/linux/can/skb.h
@@ -47,17 +47,15 @@ bool can_dropped_invalid_skb(struct net_device *dev, struct sk_buff *skb);
  */
 
 /**
  * struct can_skb_priv - private additional data inside CAN sk_buffs
  * @ifindex:	ifindex of the first interface the CAN frame appeared on
- * @skbcnt:	atomic counter to have an unique id together with skb pointer
  * @frame_len:	length of CAN frame in data link layer
  * @cf:		align to the following CAN frame at skb->data
  */
 struct can_skb_priv {
 	int ifindex;
-	int skbcnt;
 	unsigned int frame_len;
 	struct can_frame cf[];
 };
 
 static inline struct can_skb_priv *can_skb_prv(struct sk_buff *skb)
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 770173d8db42..70659987ef4d 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -639,10 +639,20 @@ static int can_rcv_filter(struct can_dev_rcv_lists *dev_rcv_lists, struct sk_buf
 	}
 
 	return matches;
 }
 
+void can_set_skb_uid(struct sk_buff *skb)
+{
+	/* create non-zero unique skb identifier together with *skb */
+	while (!(skb->hash))
+		skb->hash = atomic_inc_return(&skbcounter);
+
+	skb->sw_hash = 1;
+}
+EXPORT_SYMBOL(can_set_skb_uid);
+
 static void can_receive(struct sk_buff *skb, struct net_device *dev)
 {
 	struct can_dev_rcv_lists *dev_rcv_lists;
 	struct net *net = dev_net(dev);
 	struct can_pkg_stats *pkg_stats = net->can.pkg_stats;
@@ -650,13 +660,11 @@ static void can_receive(struct sk_buff *skb, struct net_device *dev)
 
 	/* update statistics */
 	atomic_long_inc(&pkg_stats->rx_frames);
 	atomic_long_inc(&pkg_stats->rx_frames_delta);
 
-	/* create non-zero unique skb identifier together with *skb */
-	while (!(can_skb_prv(skb)->skbcnt))
-		can_skb_prv(skb)->skbcnt = atomic_inc_return(&skbcounter);
+	can_set_skb_uid(skb);
 
 	rcu_read_lock();
 
 	/* deliver the packet to sockets listening on all devices */
 	matches = can_rcv_filter(net->can.rx_alldev_list, skb);
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 7eba8ae01a5b..8ed60f18c2ea 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -314,11 +314,10 @@ static void bcm_can_tx(struct bcm_op *op)
 	if (!skb)
 		goto out;
 
 	can_skb_reserve(skb);
 	can_skb_prv(skb)->ifindex = dev->ifindex;
-	can_skb_prv(skb)->skbcnt = 0;
 
 	skb_put_data(skb, cf, op->cfsiz);
 
 	/* send with loopback */
 	skb->dev = dev;
@@ -1342,11 +1341,10 @@ static int bcm_tx_send(struct msghdr *msg, int ifindex, struct sock *sk,
 		kfree_skb(skb);
 		return -ENODEV;
 	}
 
 	can_skb_prv(skb)->ifindex = dev->ifindex;
-	can_skb_prv(skb)->skbcnt = 0;
 	skb->dev = dev;
 	can_skb_set_owner(skb, sk);
 	err = can_send(skb, 1); /* send with loopback */
 	dev_put(dev);
 
diff --git a/net/can/isotp.c b/net/can/isotp.c
index ce588b85665a..4bb60b8f9b96 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -228,11 +228,10 @@ static int isotp_send_fc(struct sock *sk, int ae, u8 flowstatus)
 		return 1;
 	}
 
 	can_skb_reserve(nskb);
 	can_skb_prv(nskb)->ifindex = dev->ifindex;
-	can_skb_prv(nskb)->skbcnt = 0;
 
 	nskb->dev = dev;
 	can_skb_set_owner(nskb, sk);
 	ncf = (struct canfd_frame *)nskb->data;
 	skb_put_zero(nskb, so->ll.mtu);
@@ -778,11 +777,10 @@ static void isotp_send_cframe(struct isotp_sock *so)
 		return;
 	}
 
 	can_skb_reserve(skb);
 	can_skb_prv(skb)->ifindex = dev->ifindex;
-	can_skb_prv(skb)->skbcnt = 0;
 
 	cf = (struct canfd_frame *)skb->data;
 	skb_put_zero(skb, so->ll.mtu);
 
 	/* create consecutive frame */
@@ -1007,11 +1005,10 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 		goto err_out_drop;
 	}
 
 	can_skb_reserve(skb);
 	can_skb_prv(skb)->ifindex = dev->ifindex;
-	can_skb_prv(skb)->skbcnt = 0;
 
 	so->tx.len = size;
 	so->tx.idx = 0;
 
 	cf = (struct canfd_frame *)skb->data;
diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index ff9c4fd7b433..1589e8ca634e 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -895,11 +895,10 @@ static struct sk_buff *j1939_sk_alloc_skb(struct net_device *ndev,
 	if (!skb)
 		goto failure;
 
 	can_skb_reserve(skb);
 	can_skb_prv(skb)->ifindex = ndev->ifindex;
-	can_skb_prv(skb)->skbcnt = 0;
 	skb_reserve(skb, offsetof(struct can_frame, data));
 
 	ret = memcpy_from_msg(skb_put(skb, size), msg, size);
 	if (ret < 0)
 		goto free_skb;
diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index 613a911dda10..d4be13422f50 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -599,11 +599,10 @@ sk_buff *j1939_tp_tx_dat_new(struct j1939_priv *priv,
 		return ERR_PTR(-ENOMEM);
 
 	skb->dev = priv->ndev;
 	can_skb_reserve(skb);
 	can_skb_prv(skb)->ifindex = priv->ndev->ifindex;
-	can_skb_prv(skb)->skbcnt = 0;
 	/* reserve CAN header */
 	skb_reserve(skb, offsetof(struct can_frame, data));
 
 	/* skb->cb must be large enough to hold a j1939_sk_buff_cb structure */
 	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(*re_skcb));
@@ -1534,11 +1533,10 @@ j1939_session *j1939_session_fresh_new(struct j1939_priv *priv,
 		return NULL;
 
 	skb->dev = priv->ndev;
 	can_skb_reserve(skb);
 	can_skb_prv(skb)->ifindex = priv->ndev->ifindex;
-	can_skb_prv(skb)->skbcnt = 0;
 	skcb = j1939_skb_to_cb(skb);
 	memcpy(skcb, rel_skcb, sizeof(*skcb));
 
 	session = j1939_session_new(priv, skb, size);
 	if (!session) {
diff --git a/net/can/raw.c b/net/can/raw.c
index d66036da6753..d276eb92ea81 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -74,11 +74,11 @@ MODULE_ALIAS("can-proto-1");
  * storing the single filter in dfilter, to avoid using dynamic memory.
  */
 
 struct uniqframe {
 	const struct sk_buff *skb;
-	int skbcnt;
+	__u32 hash;
 	unsigned int join_rx_count;
 };
 
 struct raw_sock {
 	struct sock sk;
@@ -162,21 +162,21 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
 		}
 	}
 
 	/* eliminate multiple filter matches for the same skb */
 	if (this_cpu_ptr(ro->uniq)->skb == oskb &&
-	    this_cpu_ptr(ro->uniq)->skbcnt == can_skb_prv(oskb)->skbcnt) {
+	    this_cpu_ptr(ro->uniq)->hash == oskb->hash) {
 		if (!ro->join_filters)
 			return;
 
 		this_cpu_inc(ro->uniq->join_rx_count);
 		/* drop frame until all enabled filters matched */
 		if (this_cpu_ptr(ro->uniq)->join_rx_count < ro->count)
 			return;
 	} else {
 		this_cpu_ptr(ro->uniq)->skb = oskb;
-		this_cpu_ptr(ro->uniq)->skbcnt = can_skb_prv(oskb)->skbcnt;
+		this_cpu_ptr(ro->uniq)->hash = oskb->hash;
 		this_cpu_ptr(ro->uniq)->join_rx_count = 1;
 		/* drop first frame to check all enabled filters? */
 		if (ro->join_filters && ro->count > 1)
 			return;
 	}
@@ -954,11 +954,10 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	if (!skb)
 		goto put_dev;
 
 	can_skb_reserve(skb);
 	can_skb_prv(skb)->ifindex = dev->ifindex;
-	can_skb_prv(skb)->skbcnt = 0;
 
 	/* fill the skb before testing for valid CAN frames */
 	err = memcpy_from_msg(skb_put(skb, size), msg, size);
 	if (err < 0)
 		goto free_skb;
-- 
2.47.3

[can-next 2/5] can: move can_iif from private headroom to struct sk_buff

[Oliver Hartkopp]

When routing CAN frames over different CAN interfaces the interface index
skb->iif is overwritten with every single hop. To prevent sending a CAN
frame back to its originating (first) incoming CAN interface another
ifindex variable is needed.

can_iif is the first variable moved into the formerly unused space in CAN
skbs (the inner protocol space for ethernet/IP encapsulation). To make
sure the CAN skb was not crafted e.g. by PF_PACKET including encapsulated
data the received CAN skb is checked for skb->encapsulation to be false.

Patch 2/5 to remove the private CAN bus skb headroom infrastructure.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 drivers/net/can/dev/skb.c |  8 ++++----
 include/linux/can/skb.h   |  2 --
 include/linux/skbuff.h    | 23 +++++++++++++++++------
 net/can/af_can.c          | 21 ++++++++++++---------
 net/can/bcm.c             |  4 ++--
 net/can/gw.c              |  2 +-
 net/can/isotp.c           |  6 +++---
 net/can/j1939/socket.c    |  2 +-
 net/can/j1939/transport.c |  4 ++--
 net/can/raw.c             |  2 +-
 10 files changed, 43 insertions(+), 31 deletions(-)

diff --git a/drivers/net/can/dev/skb.c b/drivers/net/can/dev/skb.c
index 0da615afa04d..b54474687aaa 100644
--- a/drivers/net/can/dev/skb.c
+++ b/drivers/net/can/dev/skb.c
@@ -216,11 +216,11 @@ struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf)
 		return NULL;
 	}
 
 	skb->protocol = htons(ETH_P_CAN);
 	init_can_skb_reserve(skb);
-	can_skb_prv(skb)->ifindex = dev->ifindex;
+	skb->can_iif = dev->ifindex;
 
 	*cf = skb_put_zero(skb, sizeof(struct can_frame));
 
 	return skb;
 }
@@ -239,11 +239,11 @@ struct sk_buff *alloc_canfd_skb(struct net_device *dev,
 		return NULL;
 	}
 
 	skb->protocol = htons(ETH_P_CANFD);
 	init_can_skb_reserve(skb);
-	can_skb_prv(skb)->ifindex = dev->ifindex;
+	skb->can_iif = dev->ifindex;
 
 	*cfd = skb_put_zero(skb, sizeof(struct canfd_frame));
 
 	/* set CAN FD flag by default */
 	(*cfd)->flags = CANFD_FDF;
@@ -266,11 +266,11 @@ struct sk_buff *alloc_canxl_skb(struct net_device *dev,
 	if (unlikely(!skb))
 		goto out_error;
 
 	skb->protocol = htons(ETH_P_CANXL);
 	init_can_skb_reserve(skb);
-	can_skb_prv(skb)->ifindex = dev->ifindex;
+	skb->can_iif = dev->ifindex;
 
 	*cxl = skb_put_zero(skb, CANXL_HDR_SIZE + data_len);
 
 	/* set CAN XL flag and length information by default */
 	(*cxl)->flags = CANXL_XLF;
@@ -308,11 +308,11 @@ static bool can_skb_headroom_valid(struct net_device *dev, struct sk_buff *skb)
 		return false;
 
 	/* af_packet does not apply CAN skb specific settings */
 	if (skb->ip_summed == CHECKSUM_NONE) {
 		/* init headroom */
-		can_skb_prv(skb)->ifindex = dev->ifindex;
+		skb->can_iif = dev->ifindex;
 
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 		/* perform proper loopback on capable devices */
 		if (dev->flags & IFF_ECHO)
diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
index 869ea574a40a..679ea4c851ac 100644
--- a/include/linux/can/skb.h
+++ b/include/linux/can/skb.h
@@ -46,16 +46,14 @@ bool can_dropped_invalid_skb(struct net_device *dev, struct sk_buff *skb);
  * skb_copy() needs to be used instead of skb_clone().
  */
 
 /**
  * struct can_skb_priv - private additional data inside CAN sk_buffs
- * @ifindex:	ifindex of the first interface the CAN frame appeared on
  * @frame_len:	length of CAN frame in data link layer
  * @cf:		align to the following CAN frame at skb->data
  */
 struct can_skb_priv {
-	int ifindex;
 	unsigned int frame_len;
 	struct can_frame cf[];
 };
 
 static inline struct can_skb_priv *can_skb_prv(struct sk_buff *skb)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 86737076101d..ab415de74466 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -861,10 +861,11 @@ enum skb_tstamp_type {
  *	@reserved_tailroom: (aka @mark) number of bytes of free space available
  *		at the tail of an sk_buff
  *	@vlan_all: vlan fields (proto & tci)
  *	@vlan_proto: vlan encapsulation protocol
  *	@vlan_tci: vlan tag control information
+ *	@can_iif: ifindex of the first interface the CAN frame appeared on
  *	@inner_protocol: Protocol (encapsulation)
  *	@inner_ipproto: (aka @inner_protocol) stores ipproto when
  *		skb->inner_protocol_type == ENCAP_TYPE_IPPROTO;
  *	@inner_transport_header: Inner transport layer header (encapsulation)
  *	@inner_network_header: Network layer header (encapsulation)
@@ -1067,17 +1068,27 @@ struct sk_buff {
 		__u32		mark;
 		__u32		reserved_tailroom;
 	};
 
 	union {
-		__be16		inner_protocol;
-		__u8		inner_ipproto;
-	};
+		/* inner protocol data for eth/ip encapsulation */
+		struct {
+			union {
+				__be16	inner_protocol;
+				__u8	inner_ipproto;
+			};
 
-	__u16			inner_transport_header;
-	__u16			inner_network_header;
-	__u16			inner_mac_header;
+			__u16	inner_transport_header;
+			__u16	inner_network_header;
+			__u16	inner_mac_header;
+		};
+
+		/* space for protocols without protocol/header encapsulation */
+		struct {
+			int	can_iif;
+		};
+	};
 
 	__be16			protocol;
 	__u16			transport_header;
 	__u16			network_header;
 	__u16			mac_header;
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 70659987ef4d..37be7e4921c8 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -685,13 +685,14 @@ static void can_receive(struct sk_buff *skb, struct net_device *dev)
 }
 
 static int can_rcv(struct sk_buff *skb, struct net_device *dev,
 		   struct packet_type *pt, struct net_device *orig_dev)
 {
-	if (unlikely(dev->type != ARPHRD_CAN || !can_get_ml_priv(dev) || !can_is_can_skb(skb))) {
-		pr_warn_once("PF_CAN: dropped non conform CAN skbuff: dev type %d, len %d\n",
-			     dev->type, skb->len);
+	if (unlikely(dev->type != ARPHRD_CAN || !can_get_ml_priv(dev) ||
+		     !can_is_can_skb(skb) || skb->encapsulation)) {
+		pr_warn_once("PF_CAN: dropped non conform CAN skbuff: dev type %d, len %d, encap %d\n",
+			     dev->type, skb->len, skb->encapsulation);
 
 		kfree_skb_reason(skb, SKB_DROP_REASON_CAN_RX_INVALID_FRAME);
 		return NET_RX_DROP;
 	}
 
@@ -700,13 +701,14 @@ static int can_rcv(struct sk_buff *skb, struct net_device *dev,
 }
 
 static int canfd_rcv(struct sk_buff *skb, struct net_device *dev,
 		     struct packet_type *pt, struct net_device *orig_dev)
 {
-	if (unlikely(dev->type != ARPHRD_CAN || !can_get_ml_priv(dev) || !can_is_canfd_skb(skb))) {
-		pr_warn_once("PF_CAN: dropped non conform CAN FD skbuff: dev type %d, len %d\n",
-			     dev->type, skb->len);
+	if (unlikely(dev->type != ARPHRD_CAN || !can_get_ml_priv(dev) ||
+		     !can_is_canfd_skb(skb) || skb->encapsulation)) {
+		pr_warn_once("PF_CAN: dropped non conform CAN FD skbuff: dev type %d, len %d, encap %d\n",
+			     dev->type, skb->len, skb->encapsulation);
 
 		kfree_skb_reason(skb, SKB_DROP_REASON_CANFD_RX_INVALID_FRAME);
 		return NET_RX_DROP;
 	}
 
@@ -715,13 +717,14 @@ static int canfd_rcv(struct sk_buff *skb, struct net_device *dev,
 }
 
 static int canxl_rcv(struct sk_buff *skb, struct net_device *dev,
 		     struct packet_type *pt, struct net_device *orig_dev)
 {
-	if (unlikely(dev->type != ARPHRD_CAN || !can_get_ml_priv(dev) || !can_is_canxl_skb(skb))) {
-		pr_warn_once("PF_CAN: dropped non conform CAN XL skbuff: dev type %d, len %d\n",
-			     dev->type, skb->len);
+	if (unlikely(dev->type != ARPHRD_CAN || !can_get_ml_priv(dev) ||
+		     !can_is_canxl_skb(skb) || skb->encapsulation)) {
+		pr_warn_once("PF_CAN: dropped non conform CAN XL skbuff: dev type %d, len %d, encap %d\n",
+			     dev->type, skb->len, skb->encapsulation);
 
 		kfree_skb_reason(skb, SKB_DROP_REASON_CANXL_RX_INVALID_FRAME);
 		return NET_RX_DROP;
 	}
 
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 8ed60f18c2ea..a8867e7b77d2 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -313,11 +313,11 @@ static void bcm_can_tx(struct bcm_op *op)
 	skb = alloc_skb(op->cfsiz + sizeof(struct can_skb_priv), gfp_any());
 	if (!skb)
 		goto out;
 
 	can_skb_reserve(skb);
-	can_skb_prv(skb)->ifindex = dev->ifindex;
+	skb->can_iif = dev->ifindex;
 
 	skb_put_data(skb, cf, op->cfsiz);
 
 	/* send with loopback */
 	skb->dev = dev;
@@ -1340,11 +1340,11 @@ static int bcm_tx_send(struct msghdr *msg, int ifindex, struct sock *sk,
 	if (!dev) {
 		kfree_skb(skb);
 		return -ENODEV;
 	}
 
-	can_skb_prv(skb)->ifindex = dev->ifindex;
+	skb->can_iif = dev->ifindex;
 	skb->dev = dev;
 	can_skb_set_owner(skb, sk);
 	err = can_send(skb, 1); /* send with loopback */
 	dev_put(dev);
 
diff --git a/net/can/gw.c b/net/can/gw.c
index 55eccb1c7620..74d771a3540c 100644
--- a/net/can/gw.c
+++ b/net/can/gw.c
@@ -497,11 +497,11 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
 		return;
 	}
 
 	/* is sending the skb back to the incoming interface not allowed? */
 	if (!(gwj->flags & CGW_FLAGS_CAN_IIF_TX_OK) &&
-	    can_skb_prv(skb)->ifindex == gwj->dst.dev->ifindex)
+	    skb->can_iif == gwj->dst.dev->ifindex)
 		return;
 
 	/* clone the given skb, which has not been done in can_rcv()
 	 *
 	 * When there is at least one modification function activated,
diff --git a/net/can/isotp.c b/net/can/isotp.c
index 4bb60b8f9b96..e7623e5736ca 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -227,11 +227,11 @@ static int isotp_send_fc(struct sock *sk, int ae, u8 flowstatus)
 		kfree_skb(nskb);
 		return 1;
 	}
 
 	can_skb_reserve(nskb);
-	can_skb_prv(nskb)->ifindex = dev->ifindex;
+	nskb->can_iif = dev->ifindex;
 
 	nskb->dev = dev;
 	can_skb_set_owner(nskb, sk);
 	ncf = (struct canfd_frame *)nskb->data;
 	skb_put_zero(nskb, so->ll.mtu);
@@ -776,11 +776,11 @@ static void isotp_send_cframe(struct isotp_sock *so)
 		dev_put(dev);
 		return;
 	}
 
 	can_skb_reserve(skb);
-	can_skb_prv(skb)->ifindex = dev->ifindex;
+	skb->can_iif = dev->ifindex;
 
 	cf = (struct canfd_frame *)skb->data;
 	skb_put_zero(skb, so->ll.mtu);
 
 	/* create consecutive frame */
@@ -1004,11 +1004,11 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 		dev_put(dev);
 		goto err_out_drop;
 	}
 
 	can_skb_reserve(skb);
-	can_skb_prv(skb)->ifindex = dev->ifindex;
+	skb->can_iif = dev->ifindex;
 
 	so->tx.len = size;
 	so->tx.idx = 0;
 
 	cf = (struct canfd_frame *)skb->data;
diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index 1589e8ca634e..d2642d86b4a9 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -894,11 +894,11 @@ static struct sk_buff *j1939_sk_alloc_skb(struct net_device *ndev,
 				  msg->msg_flags & MSG_DONTWAIT, &ret);
 	if (!skb)
 		goto failure;
 
 	can_skb_reserve(skb);
-	can_skb_prv(skb)->ifindex = ndev->ifindex;
+	skb->can_iif = ndev->ifindex;
 	skb_reserve(skb, offsetof(struct can_frame, data));
 
 	ret = memcpy_from_msg(skb_put(skb, size), msg, size);
 	if (ret < 0)
 		goto free_skb;
diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index d4be13422f50..8a767b75194f 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -598,11 +598,11 @@ sk_buff *j1939_tp_tx_dat_new(struct j1939_priv *priv,
 	if (unlikely(!skb))
 		return ERR_PTR(-ENOMEM);
 
 	skb->dev = priv->ndev;
 	can_skb_reserve(skb);
-	can_skb_prv(skb)->ifindex = priv->ndev->ifindex;
+	skb->can_iif = priv->ndev->ifindex;
 	/* reserve CAN header */
 	skb_reserve(skb, offsetof(struct can_frame, data));
 
 	/* skb->cb must be large enough to hold a j1939_sk_buff_cb structure */
 	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(*re_skcb));
@@ -1532,11 +1532,11 @@ j1939_session *j1939_session_fresh_new(struct j1939_priv *priv,
 	if (unlikely(!skb))
 		return NULL;
 
 	skb->dev = priv->ndev;
 	can_skb_reserve(skb);
-	can_skb_prv(skb)->ifindex = priv->ndev->ifindex;
+	skb->can_iif = priv->ndev->ifindex;
 	skcb = j1939_skb_to_cb(skb);
 	memcpy(skcb, rel_skcb, sizeof(*skcb));
 
 	session = j1939_session_new(priv, skb, size);
 	if (!session) {
diff --git a/net/can/raw.c b/net/can/raw.c
index d276eb92ea81..21432e5567fa 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -953,11 +953,11 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 				  msg->msg_flags & MSG_DONTWAIT, &err);
 	if (!skb)
 		goto put_dev;
 
 	can_skb_reserve(skb);
-	can_skb_prv(skb)->ifindex = dev->ifindex;
+	skb->can_iif = dev->ifindex;
 
 	/* fill the skb before testing for valid CAN frames */
 	err = memcpy_from_msg(skb_put(skb, size), msg, size);
 	if (err < 0)
 		goto free_skb;
-- 
2.47.3

[can-next 3/5] can: move frame length from private headroom to struct sk_buff

[Oliver Hartkopp]

The can_skb_priv::frame_len variable is used to cache a previous
calculated CAN frame length to be passed to BQL queueing disciplines.

To maintain a compilable migration step the frame_len value in struct
can_skb_priv is only renamed but not removed.

Patch 3/5 to remove the private CAN bus skb headroom infrastructure.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 drivers/net/can/dev/skb.c | 8 +++-----
 include/linux/can/skb.h   | 3 +--
 include/linux/skbuff.h    | 2 ++
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/dev/skb.c b/drivers/net/can/dev/skb.c
index b54474687aaa..ffd71ad0252a 100644
--- a/drivers/net/can/dev/skb.c
+++ b/drivers/net/can/dev/skb.c
@@ -72,11 +72,11 @@ int can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
 		/* make settings for echo to reduce code in irq context */
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 		skb->dev = dev;
 
 		/* save frame_len to reuse it when transmission is completed */
-		can_skb_prv(skb)->frame_len = frame_len;
+		skb->can_framelen = frame_len;
 
 		if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)
 			skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
 
 		skb_tx_timestamp(skb);
@@ -109,20 +109,19 @@ __can_get_echo_skb(struct net_device *dev, unsigned int idx,
 	if (priv->echo_skb[idx]) {
 		/* Using "struct canfd_frame::len" for the frame
 		 * length is supported on both CAN and CANFD frames.
 		 */
 		struct sk_buff *skb = priv->echo_skb[idx];
-		struct can_skb_priv *can_skb_priv = can_skb_prv(skb);
 
 		if (skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)
 			skb_tstamp_tx(skb, skb_hwtstamps(skb));
 
 		/* get the real payload length for netdev statistics */
 		*len_ptr = can_skb_get_data_len(skb);
 
 		if (frame_len_ptr)
-			*frame_len_ptr = can_skb_priv->frame_len;
+			*frame_len_ptr = skb->can_framelen;
 
 		priv->echo_skb[idx] = NULL;
 
 		if (skb->pkt_type == PACKET_LOOPBACK) {
 			skb->pkt_type = PACKET_BROADCAST;
@@ -178,14 +177,13 @@ void can_free_echo_skb(struct net_device *dev, unsigned int idx,
 		return;
 	}
 
 	if (priv->echo_skb[idx]) {
 		struct sk_buff *skb = priv->echo_skb[idx];
-		struct can_skb_priv *can_skb_priv = can_skb_prv(skb);
 
 		if (frame_len_ptr)
-			*frame_len_ptr = can_skb_priv->frame_len;
+			*frame_len_ptr = skb->can_framelen;
 
 		dev_kfree_skb_any(skb);
 		priv->echo_skb[idx] = NULL;
 	}
 }
diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
index 679ea4c851ac..eba9557e2c1e 100644
--- a/include/linux/can/skb.h
+++ b/include/linux/can/skb.h
@@ -46,15 +46,14 @@ bool can_dropped_invalid_skb(struct net_device *dev, struct sk_buff *skb);
  * skb_copy() needs to be used instead of skb_clone().
  */
 
 /**
  * struct can_skb_priv - private additional data inside CAN sk_buffs
- * @frame_len:	length of CAN frame in data link layer
  * @cf:		align to the following CAN frame at skb->data
  */
 struct can_skb_priv {
-	unsigned int frame_len;
+	unsigned int frame_len_to_be_removed;
 	struct can_frame cf[];
 };
 
 static inline struct can_skb_priv *can_skb_prv(struct sk_buff *skb)
 {
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index ab415de74466..eccd0b3898a0 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -862,10 +862,11 @@ enum skb_tstamp_type {
  *		at the tail of an sk_buff
  *	@vlan_all: vlan fields (proto & tci)
  *	@vlan_proto: vlan encapsulation protocol
  *	@vlan_tci: vlan tag control information
  *	@can_iif: ifindex of the first interface the CAN frame appeared on
+ *	@can_framelen: cached echo CAN frame length for bql
  *	@inner_protocol: Protocol (encapsulation)
  *	@inner_ipproto: (aka @inner_protocol) stores ipproto when
  *		skb->inner_protocol_type == ENCAP_TYPE_IPPROTO;
  *	@inner_transport_header: Inner transport layer header (encapsulation)
  *	@inner_network_header: Network layer header (encapsulation)
@@ -1083,10 +1084,11 @@ struct sk_buff {
 		};
 
 		/* space for protocols without protocol/header encapsulation */
 		struct {
 			int	can_iif;
+			__u16	can_framelen;
 		};
 	};
 
 	__be16			protocol;
 	__u16			transport_header;
-- 
2.47.3

[can-next 4/5] can: remove private skb headroom infrastructure

[Oliver Hartkopp]

This patch removes the now-empty private CAN skb headroom infrastructure.

Patch 4/5 to remove the private CAN bus skb headroom infrastructure.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 drivers/net/can/dev/skb.c | 27 +++++++++++----------------
 include/linux/can/skb.h   | 28 ----------------------------
 net/can/bcm.c             |  7 ++-----
 net/can/isotp.c           |  9 +++------
 net/can/j1939/socket.c    |  4 +---
 net/can/j1939/transport.c |  7 ++-----
 net/can/raw.c             |  5 ++---
 7 files changed, 21 insertions(+), 66 deletions(-)

diff --git a/drivers/net/can/dev/skb.c b/drivers/net/can/dev/skb.c
index ffd71ad0252a..2445f6c9f9d4 100644
--- a/drivers/net/can/dev/skb.c
+++ b/drivers/net/can/dev/skb.c
@@ -188,36 +188,33 @@ void can_free_echo_skb(struct net_device *dev, unsigned int idx,
 	}
 }
 EXPORT_SYMBOL_GPL(can_free_echo_skb);
 
 /* fill common values for CAN sk_buffs */
-static void init_can_skb_reserve(struct sk_buff *skb)
+static void init_can_skb(struct sk_buff *skb)
 {
 	skb->pkt_type = PACKET_BROADCAST;
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 	skb_reset_mac_header(skb);
 	skb_reset_network_header(skb);
 	skb_reset_transport_header(skb);
-
-	can_skb_reserve(skb);
 }
 
 struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf)
 {
 	struct sk_buff *skb;
 
-	skb = netdev_alloc_skb(dev, sizeof(struct can_skb_priv) +
-			       sizeof(struct can_frame));
+	skb = netdev_alloc_skb(dev, sizeof(struct can_frame));
 	if (unlikely(!skb)) {
 		*cf = NULL;
 
 		return NULL;
 	}
 
 	skb->protocol = htons(ETH_P_CAN);
-	init_can_skb_reserve(skb);
+	init_can_skb(skb);
 	skb->can_iif = dev->ifindex;
 
 	*cf = skb_put_zero(skb, sizeof(struct can_frame));
 
 	return skb;
@@ -227,20 +224,19 @@ EXPORT_SYMBOL_GPL(alloc_can_skb);
 struct sk_buff *alloc_canfd_skb(struct net_device *dev,
 				struct canfd_frame **cfd)
 {
 	struct sk_buff *skb;
 
-	skb = netdev_alloc_skb(dev, sizeof(struct can_skb_priv) +
-			       sizeof(struct canfd_frame));
+	skb = netdev_alloc_skb(dev, sizeof(struct canfd_frame));
 	if (unlikely(!skb)) {
 		*cfd = NULL;
 
 		return NULL;
 	}
 
 	skb->protocol = htons(ETH_P_CANFD);
-	init_can_skb_reserve(skb);
+	init_can_skb(skb);
 	skb->can_iif = dev->ifindex;
 
 	*cfd = skb_put_zero(skb, sizeof(struct canfd_frame));
 
 	/* set CAN FD flag by default */
@@ -257,17 +253,16 @@ struct sk_buff *alloc_canxl_skb(struct net_device *dev,
 	struct sk_buff *skb;
 
 	if (data_len < CANXL_MIN_DLEN || data_len > CANXL_MAX_DLEN)
 		goto out_error;
 
-	skb = netdev_alloc_skb(dev, sizeof(struct can_skb_priv) +
-			       CANXL_HDR_SIZE + data_len);
+	skb = netdev_alloc_skb(dev, CANXL_HDR_SIZE + data_len);
 	if (unlikely(!skb))
 		goto out_error;
 
 	skb->protocol = htons(ETH_P_CANXL);
-	init_can_skb_reserve(skb);
+	init_can_skb(skb);
 	skb->can_iif = dev->ifindex;
 
 	*cxl = skb_put_zero(skb, CANXL_HDR_SIZE + data_len);
 
 	/* set CAN XL flag and length information by default */
@@ -297,14 +292,14 @@ struct sk_buff *alloc_can_err_skb(struct net_device *dev, struct can_frame **cf)
 	return skb;
 }
 EXPORT_SYMBOL_GPL(alloc_can_err_skb);
 
 /* Check for outgoing skbs that have not been created by the CAN subsystem */
-static bool can_skb_headroom_valid(struct net_device *dev, struct sk_buff *skb)
+static bool can_skb_init_valid(struct net_device *dev, struct sk_buff *skb)
 {
-	/* af_packet creates a headroom of HH_DATA_MOD bytes which is fine */
-	if (WARN_ON_ONCE(skb_headroom(skb) < sizeof(struct can_skb_priv)))
+	/* skb with inner protocols do not contain CAN content */
+	if (skb->encapsulation)
 		return false;
 
 	/* af_packet does not apply CAN skb specific settings */
 	if (skb->ip_summed == CHECKSUM_NONE) {
 		/* init headroom */
@@ -355,11 +350,11 @@ bool can_dropped_invalid_skb(struct net_device *dev, struct sk_buff *skb)
 
 	default:
 		goto inval_skb;
 	}
 
-	if (!can_skb_headroom_valid(dev, skb))
+	if (!can_skb_init_valid(dev, skb))
 		goto inval_skb;
 
 	return false;
 
 inval_skb:
diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
index eba9557e2c1e..c5d617a0f9be 100644
--- a/include/linux/can/skb.h
+++ b/include/linux/can/skb.h
@@ -35,38 +35,10 @@ struct sk_buff *alloc_canxl_skb(struct net_device *dev,
 				unsigned int data_len);
 struct sk_buff *alloc_can_err_skb(struct net_device *dev,
 				  struct can_frame **cf);
 bool can_dropped_invalid_skb(struct net_device *dev, struct sk_buff *skb);
 
-/*
- * The struct can_skb_priv is used to transport additional information along
- * with the stored struct can(fd)_frame that can not be contained in existing
- * struct sk_buff elements.
- * N.B. that this information must not be modified in cloned CAN sk_buffs.
- * To modify the CAN frame content or the struct can_skb_priv content
- * skb_copy() needs to be used instead of skb_clone().
- */
-
-/**
- * struct can_skb_priv - private additional data inside CAN sk_buffs
- * @cf:		align to the following CAN frame at skb->data
- */
-struct can_skb_priv {
-	unsigned int frame_len_to_be_removed;
-	struct can_frame cf[];
-};
-
-static inline struct can_skb_priv *can_skb_prv(struct sk_buff *skb)
-{
-	return (struct can_skb_priv *)(skb->head);
-}
-
-static inline void can_skb_reserve(struct sk_buff *skb)
-{
-	skb_reserve(skb, sizeof(struct can_skb_priv));
-}
-
 static inline void can_skb_set_owner(struct sk_buff *skb, struct sock *sk)
 {
 	/* If the socket has already been closed by user space, the
 	 * refcount may already be 0 (and the socket will be freed
 	 * after the last TX skb has been freed). So only increase
diff --git a/net/can/bcm.c b/net/can/bcm.c
index a8867e7b77d2..99dac9af2b86 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -308,15 +308,14 @@ static void bcm_can_tx(struct bcm_op *op)
 	if (!dev) {
 		/* RFC: should this bcm_op remove itself here? */
 		return;
 	}
 
-	skb = alloc_skb(op->cfsiz + sizeof(struct can_skb_priv), gfp_any());
+	skb = alloc_skb(op->cfsiz, gfp_any());
 	if (!skb)
 		goto out;
 
-	can_skb_reserve(skb);
 	skb->can_iif = dev->ifindex;
 
 	skb_put_data(skb, cf, op->cfsiz);
 
 	/* send with loopback */
@@ -1322,16 +1321,14 @@ static int bcm_tx_send(struct msghdr *msg, int ifindex, struct sock *sk,
 
 	/* we need a real device to send frames */
 	if (!ifindex)
 		return -ENODEV;
 
-	skb = alloc_skb(cfsiz + sizeof(struct can_skb_priv), GFP_KERNEL);
+	skb = alloc_skb(cfsiz, GFP_KERNEL);
 	if (!skb)
 		return -ENOMEM;
 
-	can_skb_reserve(skb);
-
 	err = memcpy_from_msg(skb_put(skb, cfsiz), msg, cfsiz);
 	if (err < 0) {
 		kfree_skb(skb);
 		return err;
 	}
diff --git a/net/can/isotp.c b/net/can/isotp.c
index e7623e5736ca..3486d558b3c0 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -216,21 +216,20 @@ static int isotp_send_fc(struct sock *sk, int ae, u8 flowstatus)
 	struct sk_buff *nskb;
 	struct canfd_frame *ncf;
 	struct isotp_sock *so = isotp_sk(sk);
 	int can_send_ret;
 
-	nskb = alloc_skb(so->ll.mtu + sizeof(struct can_skb_priv), gfp_any());
+	nskb = alloc_skb(so->ll.mtu, gfp_any());
 	if (!nskb)
 		return 1;
 
 	dev = dev_get_by_index(sock_net(sk), so->ifindex);
 	if (!dev) {
 		kfree_skb(nskb);
 		return 1;
 	}
 
-	can_skb_reserve(nskb);
 	nskb->can_iif = dev->ifindex;
 
 	nskb->dev = dev;
 	can_skb_set_owner(nskb, sk);
 	ncf = (struct canfd_frame *)nskb->data;
@@ -769,17 +768,16 @@ static void isotp_send_cframe(struct isotp_sock *so)
 
 	dev = dev_get_by_index(sock_net(sk), so->ifindex);
 	if (!dev)
 		return;
 
-	skb = alloc_skb(so->ll.mtu + sizeof(struct can_skb_priv), GFP_ATOMIC);
+	skb = alloc_skb(so->ll.mtu, GFP_ATOMIC);
 	if (!skb) {
 		dev_put(dev);
 		return;
 	}
 
-	can_skb_reserve(skb);
 	skb->can_iif = dev->ifindex;
 
 	cf = (struct canfd_frame *)skb->data;
 	skb_put_zero(skb, so->ll.mtu);
 
@@ -996,18 +994,17 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	if (!dev) {
 		err = -ENXIO;
 		goto err_out_drop;
 	}
 
-	skb = sock_alloc_send_skb(sk, so->ll.mtu + sizeof(struct can_skb_priv),
+	skb = sock_alloc_send_skb(sk, so->ll.mtu,
 				  msg->msg_flags & MSG_DONTWAIT, &err);
 	if (!skb) {
 		dev_put(dev);
 		goto err_out_drop;
 	}
 
-	can_skb_reserve(skb);
 	skb->can_iif = dev->ifindex;
 
 	so->tx.len = size;
 	so->tx.idx = 0;
 
diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index d2642d86b4a9..f7d0b3c4aeb4 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -887,17 +887,15 @@ static struct sk_buff *j1939_sk_alloc_skb(struct net_device *ndev,
 	int ret;
 
 	skb = sock_alloc_send_skb(sk,
 				  size +
 				  sizeof(struct can_frame) -
-				  sizeof(((struct can_frame *)NULL)->data) +
-				  sizeof(struct can_skb_priv),
+				  sizeof(((struct can_frame *)NULL)->data),
 				  msg->msg_flags & MSG_DONTWAIT, &ret);
 	if (!skb)
 		goto failure;
 
-	can_skb_reserve(skb);
 	skb->can_iif = ndev->ifindex;
 	skb_reserve(skb, offsetof(struct can_frame, data));
 
 	ret = memcpy_from_msg(skb_put(skb, size), msg, size);
 	if (ret < 0)
diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index 8a767b75194f..53308755a0c2 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -591,17 +591,15 @@ sk_buff *j1939_tp_tx_dat_new(struct j1939_priv *priv,
 			     bool swap_src_dst)
 {
 	struct sk_buff *skb;
 	struct j1939_sk_buff_cb *skcb;
 
-	skb = alloc_skb(sizeof(struct can_frame) + sizeof(struct can_skb_priv),
-			GFP_ATOMIC);
+	skb = alloc_skb(sizeof(struct can_frame), GFP_ATOMIC);
 	if (unlikely(!skb))
 		return ERR_PTR(-ENOMEM);
 
 	skb->dev = priv->ndev;
-	can_skb_reserve(skb);
 	skb->can_iif = priv->ndev->ifindex;
 	/* reserve CAN header */
 	skb_reserve(skb, offsetof(struct can_frame, data));
 
 	/* skb->cb must be large enough to hold a j1939_sk_buff_cb structure */
@@ -1526,16 +1524,15 @@ j1939_session *j1939_session_fresh_new(struct j1939_priv *priv,
 {
 	struct sk_buff *skb;
 	struct j1939_sk_buff_cb *skcb;
 	struct j1939_session *session;
 
-	skb = alloc_skb(size + sizeof(struct can_skb_priv), GFP_ATOMIC);
+	skb = alloc_skb(size, GFP_ATOMIC);
 	if (unlikely(!skb))
 		return NULL;
 
 	skb->dev = priv->ndev;
-	can_skb_reserve(skb);
 	skb->can_iif = priv->ndev->ifindex;
 	skcb = j1939_skb_to_cb(skb);
 	memcpy(skcb, rel_skcb, sizeof(*skcb));
 
 	session = j1939_session_new(priv, skb, size);
diff --git a/net/can/raw.c b/net/can/raw.c
index 21432e5567fa..c8cbbcecbfed 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -947,16 +947,15 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 
 	/* no sending on a CAN device in read-only mode */
 	if (can_cap_enabled(dev, CAN_CAP_RO))
 		return -EACCES;
 
-	skb = sock_alloc_send_skb(sk, size + sizeof(struct can_skb_priv),
-				  msg->msg_flags & MSG_DONTWAIT, &err);
+	skb = sock_alloc_send_skb(sk, size, msg->msg_flags & MSG_DONTWAIT,
+				  &err);
 	if (!skb)
 		goto put_dev;
 
-	can_skb_reserve(skb);
 	skb->can_iif = dev->ifindex;
 
 	/* fill the skb before testing for valid CAN frames */
 	err = memcpy_from_msg(skb_put(skb, size), msg, size);
 	if (err < 0)
-- 
2.47.3

[can-next 5/5] can: gw: use new can_gw_hops variable instead of re-using csum_start

[Oliver Hartkopp]

As CAN skbs don't use IP checksums the skb->csum_start variable was used
to store the can-gw CAN frame time-to-live counter together with
skb->ip_summed set to CHECKSUM_UNNECESSARY.

As we still have 16 bit left in the inner protocol space for ethernet/IP
encapsulation the time-to-live counter is moved there to remove the 'hack'
using the skb->csum_start variable.

Patch 5/5 to remove the private CAN bus skb headroom infrastructure.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 include/linux/skbuff.h |  2 ++
 net/can/gw.c           | 23 ++++++-----------------
 2 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index eccd0b3898a0..7ef0b8e24a30 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -863,10 +863,11 @@ enum skb_tstamp_type {
  *	@vlan_all: vlan fields (proto & tci)
  *	@vlan_proto: vlan encapsulation protocol
  *	@vlan_tci: vlan tag control information
  *	@can_iif: ifindex of the first interface the CAN frame appeared on
  *	@can_framelen: cached echo CAN frame length for bql
+ *	@can_gw_hops: can-gw CAN frame time-to-live counter
  *	@inner_protocol: Protocol (encapsulation)
  *	@inner_ipproto: (aka @inner_protocol) stores ipproto when
  *		skb->inner_protocol_type == ENCAP_TYPE_IPPROTO;
  *	@inner_transport_header: Inner transport layer header (encapsulation)
  *	@inner_network_header: Network layer header (encapsulation)
@@ -1085,10 +1086,11 @@ struct sk_buff {
 
 		/* space for protocols without protocol/header encapsulation */
 		struct {
 			int	can_iif;
 			__u16	can_framelen;
+			__u16	can_gw_hops;
 		};
 	};
 
 	__be16			protocol;
 	__u16			transport_header;
diff --git a/net/can/gw.c b/net/can/gw.c
index 74d771a3540c..fca0566963a2 100644
--- a/net/can/gw.c
+++ b/net/can/gw.c
@@ -68,12 +68,12 @@ MODULE_ALIAS(CAN_GW_NAME);
 
 #define CGW_MIN_HOPS 1
 #define CGW_MAX_HOPS 6
 #define CGW_DEFAULT_HOPS 1
 
-static unsigned int max_hops __read_mostly = CGW_DEFAULT_HOPS;
-module_param(max_hops, uint, 0444);
+static unsigned short max_hops __read_mostly = CGW_DEFAULT_HOPS;
+module_param(max_hops, ushort, 0444);
 MODULE_PARM_DESC(max_hops,
 		 "maximum " CAN_GW_NAME " routing hops for CAN frames "
 		 "(valid values: " __stringify(CGW_MIN_HOPS) "-"
 		 __stringify(CGW_MAX_HOPS) " hops, "
 		 "default: " __stringify(CGW_DEFAULT_HOPS) ")");
@@ -472,23 +472,12 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
 	}
 
 	/* Do not handle CAN frames routed more than 'max_hops' times.
 	 * In general we should never catch this delimiter which is intended
 	 * to cover a misconfiguration protection (e.g. circular CAN routes).
-	 *
-	 * The Controller Area Network controllers only accept CAN frames with
-	 * correct CRCs - which are not visible in the controller registers.
-	 * According to skbuff.h documentation the csum_start element for IP
-	 * checksums is undefined/unused when ip_summed == CHECKSUM_UNNECESSARY.
-	 * Only CAN skbs can be processed here which already have this property.
 	 */
-
-#define cgw_hops(skb) ((skb)->csum_start)
-
-	BUG_ON(skb->ip_summed != CHECKSUM_UNNECESSARY);
-
-	if (cgw_hops(skb) >= max_hops) {
+	if (skb->can_gw_hops >= max_hops) {
 		/* indicate deleted frames due to misconfiguration */
 		gwj->deleted_frames++;
 		return;
 	}
 
@@ -517,15 +506,15 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
 		gwj->dropped_frames++;
 		return;
 	}
 
 	/* put the incremented hop counter in the cloned skb */
-	cgw_hops(nskb) = cgw_hops(skb) + 1;
+	nskb->can_gw_hops = skb->can_gw_hops + 1;
 
 	/* first processing of this CAN frame -> adjust to private hop limit */
-	if (gwj->limit_hops && cgw_hops(nskb) == 1)
-		cgw_hops(nskb) = max_hops - gwj->limit_hops + 1;
+	if (gwj->limit_hops && nskb->can_gw_hops == 1)
+		nskb->can_gw_hops = max_hops - gwj->limit_hops + 1;
 
 	nskb->dev = gwj->dst.dev;
 
 	/* pointer to modifiable CAN frame */
 	cf = (struct canfd_frame *)nskb->data;
-- 
2.47.3

Re: [can-next 0/5] can: remove private skb headroom infrastructure

[Oleksij Rempel]

Hi Oliver,

On Mon, Jan 12, 2026 at 04:09:03PM +0100, Oliver Hartkopp wrote:
> CAN bus related skbuffs (ETH_P_CAN/ETH_P_CANFD/ETH_P_CANXL) simply contain
> CAN frame structs for CAN CC/FD/XL of skb->len length at skb->data.
> Those CAN skbs do not have network/mac/transport headers nor other such
> references for encapsulated protocols like ethernet/IP protocols.
>
> To store data for CAN specific use-cases all CAN bus related skbuffs are
> created with a 16 byte private skb headroom (struct can_skb_priv).
> Using the skb headroom and accessing skb->head for this private data
> led to several problems in the past likely due to "The struct can_skb_priv
> business is highly unconventional for the networking stack." [1]
>
> This patch set aims to remove the unconventional skb headroom usage for
> CAN bus related skbuffs. To store the data for CAN specific use-cases
> unused space in CAN skbs is used, namely the inner protocol space for
> ethernet/IP encapsulation.
>
> [1] https://lore.kernel.org/linux-can/20260104074222.29e660ac@kernel.org/
>
> Oliver Hartkopp (5):
>   can: use skb hash instead of private variable in headroom
>   can: move can_iif from private headroom to struct sk_buff
>   can: move frame length from private headroom to struct sk_buff
>   can: remove private skb headroom infrastructure
>   can: gw: use new can_gw_hops variable instead of re-using csum_start
>
>  drivers/net/can/dev/skb.c | 45 ++++++++++++++++-----------------------
>  include/linux/can/core.h  |  1 +
>  include/linux/can/skb.h   | 33 ----------------------------
>  include/linux/skbuff.h    | 27 +++++++++++++++++------
>  net/can/af_can.c          | 35 +++++++++++++++++++-----------
>  net/can/bcm.c             | 13 ++++-------
>  net/can/gw.c              | 25 ++++++----------------
>  net/can/isotp.c           | 18 ++++++----------
>  net/can/j1939/socket.c    |  7 ++----
>  net/can/j1939/transport.c | 13 ++++-------
>  net/can/raw.c             | 14 ++++++------
>  11 files changed, 92 insertions(+), 139 deletions(-)

J1939 related part seems to work without regressions.
For j1939:
Tested-by: Oleksij Rempel <o.rempel@pengutronix.de>
Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>

Thank you!

Best Regards,
Oleksij

Re: [can-next 0/5] can: remove private skb headroom infrastructure

[Paolo Abeni]

On 1/12/26 4:09 PM, Oliver Hartkopp wrote:
> This patch set aims to remove the unconventional skb headroom usage for
> CAN bus related skbuffs. To store the data for CAN specific use-cases
> unused space in CAN skbs is used, namely the inner protocol space for
> ethernet/IP encapsulation.

I don't like much that the CAN information are scattered in different
places (skb->hash and tunnel header section). Also it's unclear to me if
a can bus skb could end-up landing (even via completely
insane/intentionally evil configuration/setup) in a plain netdev interface.

In the such a case this solution will be problematic.

Could you please explain in details why the metadata_dst option has been
deemed unsuitable?!? I *think* something vaguely alike the following
would do?!?

---
diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h
index 1fc2fb03ce3f..d6ee45631fea 100644
--- a/include/net/dst_metadata.h
+++ b/include/net/dst_metadata.h
@@ -13,6 +13,13 @@ enum metadata_type {
 	METADATA_HW_PORT_MUX,
 	METADATA_MACSEC,
 	METADATA_XFRM,
+	METADATA_CAN,
+};
+
+struct can_md_info {
+	int can_iif;
+	int len;
+	int uid;
 };

 struct hw_port_info {
@@ -38,6 +45,7 @@ struct metadata_dst {
 		struct hw_port_info	port_info;
 		struct macsec_info	macsec_info;
 		struct xfrm_md_info	xfrm_info;
+		struct can_md_info	can_info;
 	} u;
 };

Re: [can-next 0/5] can: remove private skb headroom infrastructure

[Oliver Hartkopp]

Hello Paolo,

freshly created CAN skbs only contain a fixed struct can_frame (16 
byte!) where dev/priority/mark/tstamp are set together with

skb->protocol = htons(ETH_P_CAN);
skb->ip_summed = CHECKSUM_UNNECESSARY;
skb->pkt_type = PACKET_LOOPBACK;

All other settings that are relevant to ethernet/IP are unused and left 
at their initialization values (e.g. network/mac/transport headers or 
inner protocol values).

A single CAN skb can be passed to the driver layer and back several 
times. Because we need to place some additional data along with CAN skbs 
this was formerly stored in a 16 byte private skb headroom (struct 
can_skb_priv).

IIRC we had three issues (KMSAN, etc) with the headroom as someone 
between netif_rx() and can_rcv() was using the headroom for his purposes 
so that the access to struct can_skb_priv via skb->head was broken and 
not reliable for CAN skbs.

Skbs are mostly used for ethernet/IP and developers do not really 
know/care about CAN skbs. That's why this patch set aims to remove 
private CAN skb headroom infrastructure - and to minimize the (risky) 
interaction with other ethernet/IP code.

On 15.01.26 16:37, Paolo Abeni wrote:

> Could you please explain in details why the metadata_dst option has been
> deemed unsuitable?!? I *think* something vaguely alike the following
> would do?!?
> 
> ---
> diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h
> index 1fc2fb03ce3f..d6ee45631fea 100644
> --- a/include/net/dst_metadata.h
> +++ b/include/net/dst_metadata.h
> @@ -13,6 +13,13 @@ enum metadata_type {
>   	METADATA_HW_PORT_MUX,
>   	METADATA_MACSEC,
>   	METADATA_XFRM,
> +	METADATA_CAN,
> +};
> +
> +struct can_md_info {
> +	int can_iif;
> +	int len;
> +	int uid;
>   };
> 
>   struct hw_port_info {
> @@ -38,6 +45,7 @@ struct metadata_dst {
>   		struct hw_port_info	port_info;
>   		struct macsec_info	macsec_info;
>   		struct xfrm_md_info	xfrm_info;
> +		struct can_md_info	can_info;
>   	} u;
>   };
> 

Yes. I came to the same simple extensions for data structures but then 
looked into dst_metadata.h and the users code with mallocs, per_cpu 
code, unclone, refcounts, etc. - which was hard to understand for me and 
introduced complexity that is again needed and maintained by ethernet/IP 
users only. Not really appropriate for a CAN skb that transports 16 byte 
of data IMO.

For that reason I propose the common pattern to wrap a union around 
dual-usable skb space, which is simple efficient and easy to understand.

On 15.01.26 16:37, Paolo Abeni wrote:

 > I don't like much that the CAN information are scattered in different
 > places (skb->hash and tunnel header section).

This is not the case. According to the documentation the skb->hash is a 
value used for RPS to identify skbs. We would use it as intended.

And the tunnel header section is marked unused in CAN skbs. By setting 
"skb->encapsulation" to false (the init value) this section is not read 
by anyone. Wrapping a union around this dual-usable skb space is a safe 
solution here.

 > Also it's unclear to me if
 > a can bus skb could end-up landing (even via completely
 > insane/intentionally evil configuration/setup) in a plain netdev 
interface.
 >
 > In the such a case this solution will be problematic.

The CAN drivers and the CAN network layer code always checks the 
processed skbs for ETH_P_[CAN|CANFD|CANXL] and ARPHDR_CAN. So CAN skbs 
created by the CAN netlayer can only be sent to ARPHDR_CAN devices.

The only way to create weird CAN skbs is via PF_PACKET sockets that 
sends ETH_P_CAN skbs to ethernet devices. Beyond such PF_PACKET skbs the 
now suggested CAN skbs would not harm any driver or network layer as the 
described skb settings do not have any problematic content.

Netdev drivers can cope with it and the netlayer code using 
ETH_P_[CAN|CANFD|CANXL] or ETH_P_ALL is fine with it too.

Long story short: Using the common pattern to wrap a union around 
dual-usable skb space is the most efficient and least risky solution IMHO.

Best regards,
Oliver

Re: [can-next 0/5] can: remove private skb headroom infrastructure

[Jakub Kicinski]

On Fri, 16 Jan 2026 11:31:14 +0100 Oliver Hartkopp wrote:
> Long story short: Using the common pattern to wrap a union around 
> dual-usable skb space is the most efficient and least risky solution IMHO.

The concern is that we're making a precedent for, let's call it -
not-routable-networking technology to redefine fields in skb that
it doesn't need. From the maintainability perspective that's a big
risk, IMHO. I fully acknowledge tho that using md dst will be a lot
more work. Which makes this situation an unpleasant judgment call :(

Re: [can-next 0/5] can: remove private skb headroom infrastructure

[Oliver Hartkopp]

On 17.01.26 18:15, Jakub Kicinski wrote:
> On Fri, 16 Jan 2026 11:31:14 +0100 Oliver Hartkopp wrote:
>> Long story short: Using the common pattern to wrap a union around
>> dual-usable skb space is the most efficient and least risky solution IMHO.
> 
> The concern is that we're making a precedent for, let's call it -
> not-routable-networking technology to redefine fields in skb that
> it doesn't need. From the maintainability perspective that's a big
> risk, IMHO. I fully acknowledge tho that using md dst will be a lot
> more work. Which makes this situation an unpleasant judgment call :(
> 

I checked out more of the "destination cache" code, the dst_metadata and 
its users.

And also this scary union in struct sk_buff:

union {
         struct {
                 unsigned long   _skb_refdst;
                 void            (*destructor)(struct sk_buff *skb);
         };
         struct list_head        tcp_tsorted_anchor;
#ifdef CONFIG_NET_SOCK_MSG
         unsigned long           _sk_redir;
#endif
};

Despite the fact that the required 8 bytes content for the CAN skb has 
nothing to do with destinations or other of these above use cases that 
share the unsigned long pointer magic above, I doubt that the current 
flow of CAN skbs between socket layer, driver layer and forth and back 
would survive this.

My first approach to "just" extend the skb header space for CAN skbs did 
not work out because people obviously have other things in mind with 
skb->head. I'm sure I can count the days until something breaks with the 
CAN specific SKB data when attaching them to the next infrastructure 
build for ethernet/IP use cases for the same reason.

After all these experiences I would tend to add the required 8 bytes 
directly to struct sk_buff covered by "#if IS_ENABLED(CONFIG_CAN)".

Or save those 8 bytes by using the "inner protocol space" and 
additionally setting skb->encapsulation to false. Which is a safe thing 
to protect the CAN specific content against accidentally assaults and 
can be clearly proved to be correct.

Best regards,
Oliver

Re: [can-next 0/5] can: remove private skb headroom infrastructure

[Oliver Hartkopp]

Hello Marc,

I'm not sure how intense you followed my discussion with Jakub and Paolo 
about my attempt to move the CAN skb specifc content (8 bytes) away from 
the problematic skb->head reference an hold it directly in struct sk_buff?

Meanwhile I sent a v2 patch set which has been dropped from netdev 
patchwork because of its can-next mail subject:

https://lore.kernel.org/linux-can/20260117132824.3649-1-socketcan@hartkopp.net/

I've been running the patches for quite a while now and feel very 
confident that the solution is very efficient and safe for either CAN 
skbs and non-CAN skbs.

To be more clear in the struct sk_buff changes I would change the 
comments in my next respin like this:

	union {
		/* skb->encapsulation = true */
		struct {
			/* eth/ip encapsulation / tunneling */
			union {
				__be16	inner_protocol;
				__u8	inner_ipproto;
			};

			__u16	inner_transport_header;
			__u16	inner_network_header;
			__u16	inner_mac_header;
		};

		/* skb->encapsulation = false */
#if IS_ENABLED(CONFIG_CAN)
		struct {
			/* CAN skb content (ETH_P_CAN*) */
			int	can_iif;
			__u16	can_framelen;
			__u16	can_gw_hops;
		};
#endif
	};

And I wonder if it would make sense to add a WARN_ON_ONCE() in can_rcv() 
and friends?

What is your opinion about the patch set?

Should I make it a net-next patch set to restart the discussion there?

Best regards,
Oliver


On 18.01.26 13:53, Oliver Hartkopp wrote:
> 
> On 17.01.26 18:15, Jakub Kicinski wrote:
>> On Fri, 16 Jan 2026 11:31:14 +0100 Oliver Hartkopp wrote:
>>> Long story short: Using the common pattern to wrap a union around
>>> dual-usable skb space is the most efficient and least risky solution 
>>> IMHO.
>>
>> The concern is that we're making a precedent for, let's call it -
>> not-routable-networking technology to redefine fields in skb that
>> it doesn't need. From the maintainability perspective that's a big
>> risk, IMHO. I fully acknowledge tho that using md dst will be a lot
>> more work. Which makes this situation an unpleasant judgment call :(
>>
> 
> I checked out more of the "destination cache" code, the dst_metadata and 
> its users.
> 
> And also this scary union in struct sk_buff:
> 
> union {
>          struct {
>                  unsigned long   _skb_refdst;
>                  void            (*destructor)(struct sk_buff *skb);
>          };
>          struct list_head        tcp_tsorted_anchor;
> #ifdef CONFIG_NET_SOCK_MSG
>          unsigned long           _sk_redir;
> #endif
> };
> 
> Despite the fact that the required 8 bytes content for the CAN skb has 
> nothing to do with destinations or other of these above use cases that 
> share the unsigned long pointer magic above, I doubt that the current 
> flow of CAN skbs between socket layer, driver layer and forth and back 
> would survive this.
> 
> My first approach to "just" extend the skb header space for CAN skbs did 
> not work out because people obviously have other things in mind with 
> skb->head. I'm sure I can count the days until something breaks with the 
> CAN specific SKB data when attaching them to the next infrastructure 
> build for ethernet/IP use cases for the same reason.
> 
> After all these experiences I would tend to add the required 8 bytes 
> directly to struct sk_buff covered by "#if IS_ENABLED(CONFIG_CAN)".
> 
> Or save those 8 bytes by using the "inner protocol space" and 
> additionally setting skb->encapsulation to false. Which is a safe thing 
> to protect the CAN specific content against accidentally assaults and 
> can be clearly proved to be correct.
> 
> Best regards,
> Oliver

Re: [can-next 0/5] can: remove private skb headroom infrastructure

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1982 bytes --]

On 21.01.2026 13:55:47, Oliver Hartkopp wrote:
> I'm not sure how intense you followed my discussion with Jakub and Paolo
> about my attempt to move the CAN skb specifc content (8 bytes) away from the
> problematic skb->head reference an hold it directly in struct sk_buff?
>
> Meanwhile I sent a v2 patch set which has been dropped from netdev patchwork
> because of its can-next mail subject:
>
> https://lore.kernel.org/linux-can/20260117132824.3649-1-socketcan@hartkopp.net/
>
> I've been running the patches for quite a while now and feel very confident
> that the solution is very efficient and safe for either CAN skbs and non-CAN
> skbs.
>
> To be more clear in the struct sk_buff changes I would change the comments
> in my next respin like this:
>
> 	union {
> 		/* skb->encapsulation = true */
> 		struct {
> 			/* eth/ip encapsulation / tunneling */
> 			union {
> 				__be16	inner_protocol;
> 				__u8	inner_ipproto;
> 			};
>
> 			__u16	inner_transport_header;
> 			__u16	inner_network_header;
> 			__u16	inner_mac_header;
> 		};
>
> 		/* skb->encapsulation = false */
> #if IS_ENABLED(CONFIG_CAN)
> 		struct {
> 			/* CAN skb content (ETH_P_CAN*) */
> 			int	can_iif;
> 			__u16	can_framelen;
> 			__u16	can_gw_hops;
> 		};
> #endif
> 	};
>
> And I wonder if it would make sense to add a WARN_ON_ONCE() in can_rcv() and
> friends?
>
> What is your opinion about the patch set?

We have to convince the netdev people why we cannot use metadata_dst or
skb extentions but put things in other more os less arbitrary places.

> Should I make it a net-next patch set to restart the discussion there?

Rather continue the discussion :)

regards,
Marc

--
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [can-next 0/5] can: remove private skb headroom infrastructure

[Oliver Hartkopp]

On 21.01.26 15:37, Marc Kleine-Budde wrote:
> On 21.01.2026 13:55:47, Oliver Hartkopp wrote:
>> I'm not sure how intense you followed my discussion with Jakub and Paolo
>> about my attempt to move the CAN skb specifc content (8 bytes) away from the
>> problematic skb->head reference an hold it directly in struct sk_buff?
>>
>> Meanwhile I sent a v2 patch set which has been dropped from netdev patchwork
>> because of its can-next mail subject:
>>
>> https://lore.kernel.org/linux-can/20260117132824.3649-1-socketcan@hartkopp.net/
>>
>> I've been running the patches for quite a while now and feel very confident
>> that the solution is very efficient and safe for either CAN skbs and non-CAN
>> skbs.
>>
>> To be more clear in the struct sk_buff changes I would change the comments
>> in my next respin like this:
>>
>> 	union {
>> 		/* skb->encapsulation = true */
>> 		struct {
>> 			/* eth/ip encapsulation / tunneling */
>> 			union {
>> 				__be16	inner_protocol;
>> 				__u8	inner_ipproto;
>> 			};
>>
>> 			__u16	inner_transport_header;
>> 			__u16	inner_network_header;
>> 			__u16	inner_mac_header;
>> 		};
>>
>> 		/* skb->encapsulation = false */
>> #if IS_ENABLED(CONFIG_CAN)
>> 		struct {
>> 			/* CAN skb content (ETH_P_CAN*) */
>> 			int	can_iif;
>> 			__u16	can_framelen;
>> 			__u16	can_gw_hops;
>> 		};
>> #endif
>> 	};
>>
>> And I wonder if it would make sense to add a WARN_ON_ONCE() in can_rcv() and
>> friends?
>>
>> What is your opinion about the patch set?
> 
> We have to convince the netdev people why we cannot use metadata_dst or
> skb extentions but put things in other more os less arbitrary places.

I was only looking into metadata_dst which looked scary to me.

But the skb extensions look promising. It's pretty good to understand 
and sticks to the skb until it is free'd. And cloning/freeing works 
automatically and handles the skb extension seamlessly.

So it mainly costs another kmem_cache_alloc() ...

I'll give it a try.

Best regards,
Oliver

> 
>> Should I make it a net-next patch set to restart the discussion there?
> 
> Rather continue the discussion :)
> 
> regards,
> Marc
> 
> --
> Pengutronix e.K.                 | Marc Kleine-Budde          |
> Embedded Linux                   | https://www.pengutronix.de |
> Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |