Re: [PATCH net v2] drivers/net/wan/hdlc_fr: Add needed_headroom for PVC devices
From: Jakub Kicinski <kuba@kernel.org>
Date: 2020-09-04 22:14:47
Also in:
lkml
On Wed, 2 Sep 2020 17:06:58 -0700 Xie He wrote:
PVC devices are virtual devices in this driver stacked on top of the actual HDLC device. They are the devices normal users would use. PVC devices have two types: normal PVC devices and Ethernet-emulating PVC devices. When transmitting data with PVC devices, the ndo_start_xmit function will prepend a header of 4 or 10 bytes. Currently this driver requests this headroom to be reserved for normal PVC devices by setting their hard_header_len to 10. However, this does not work when these devices are used with AF_PACKET/RAW sockets. Also, this driver does not request this headroom for Ethernet-emulating PVC devices (but deals with this problem by reallocating the skb when needed, which is not optimal). This patch replaces hard_header_len with needed_headroom, and set needed_headroom for Ethernet-emulating PVC devices, too. This makes the driver to request headroom for all PVC devices in all cases.
Since this is a tunnel protocol on top of HDLC interfaces, and hdlc_setup_dev() sets dev->hard_header_len = 16; should we actually set the needed_headroom to 10 + 16 = 26? I'm not clear on where/if hdlc devices actually prepend 16 bytes of header, though. CC: Willem as he was reviewing your similar patch recently.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c index 9acad651ea1f..12b35404cd8e 100644 --- a/drivers/net/wan/hdlc_fr.c +++ b/drivers/net/wan/hdlc_fr.c@@ -1041,7 +1041,7 @@ static void pvc_setup(struct net_device *dev) { dev->type = ARPHRD_DLCI; dev->flags = IFF_POINTOPOINT; - dev->hard_header_len = 10; + dev->hard_header_len = 0;
Is there a need to set this to 0? Will it not be zero after allocation?
quoted hunk ↗ jump to hunk
dev->addr_len = 2; netif_keep_dst(dev); }@@ -1093,6 +1093,7 @@ static int fr_add_pvc(struct net_device *frad, unsigned int dlci, int type) dev->mtu = HDLC_MAX_MTU; dev->min_mtu = 68; dev->max_mtu = HDLC_MAX_MTU; + dev->needed_headroom = 10; dev->priv_flags |= IFF_NO_QUEUE; dev->ml_priv = pvc;