[PATCH net-next] net/packet: Reset MAC header for direct packet transmission

Subsystems: networking [general], packet sockets, the rest

STALE1954d

3 messages, 2 authors, 2021-03-29 · open the first message on its own page

[PATCH net-next] net/packet: Reset MAC header for direct packet transmission

From: Kurt Kanzenbach <kurt@linutronix.de>
Date: 2021-03-26 15:49:28

Reset MAC header in case of using packet_direct_xmit(), e.g. by specifying
PACKET_QDISC_BYPASS. This is needed, because other code such as the HSR layer
expects the MAC header to be correctly set.

This has been observed using the following setup:

|$ ip link add name hsr0 type hsr slave1 lan0 slave2 lan1 supervision 45 version 1
|$ ifconfig hsr0 up
|$ ./test hsr0

The test binary is using mmap'ed sockets and is specifying the
PACKET_QDISC_BYPASS socket option.

This patch resolves the following warning on a non-patched kernel:

|[  112.725394] ------------[ cut here ]------------
|[  112.731418] WARNING: CPU: 1 PID: 257 at net/hsr/hsr_forward.c:560 hsr_forward_skb+0x484/0x568
|[  112.739962] net/hsr/hsr_forward.c:560: Malformed frame (port_src hsr0)

The MAC header is also reset unconditionally in case of PACKET_QDISC_BYPASS is
not used.

Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
 net/packet/af_packet.c | 1 +
 1 file changed, 1 insertion(+)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 118d585337d7..6325c9b7df38 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -241,6 +241,7 @@ static void __fanout_link(struct sock *sk, struct packet_sock *po);
 
 static int packet_direct_xmit(struct sk_buff *skb)
 {
+	skb_reset_mac_header(skb);
 	return dev_direct_xmit(skb, packet_pick_tx_queue(skb));
 }
 
-- 
2.20.1

Re: [PATCH net-next] net/packet: Reset MAC header for direct packet transmission

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: 2021-03-28 14:10:10

On Fri, Mar 26, 2021 at 11:49 AM Kurt Kanzenbach [off-list ref] wrote:
Reset MAC header in case of using packet_direct_xmit(), e.g. by specifying
PACKET_QDISC_BYPASS. This is needed, because other code such as the HSR layer
expects the MAC header to be correctly set.

This has been observed using the following setup:

|$ ip link add name hsr0 type hsr slave1 lan0 slave2 lan1 supervision 45 version 1
|$ ifconfig hsr0 up
|$ ./test hsr0

The test binary is using mmap'ed sockets and is specifying the
PACKET_QDISC_BYPASS socket option.

This patch resolves the following warning on a non-patched kernel:

|[  112.725394] ------------[ cut here ]------------
|[  112.731418] WARNING: CPU: 1 PID: 257 at net/hsr/hsr_forward.c:560 hsr_forward_skb+0x484/0x568
|[  112.739962] net/hsr/hsr_forward.c:560: Malformed frame (port_src hsr0)

The MAC header is also reset unconditionally in case of PACKET_QDISC_BYPASS is
not used.
At the top of __dev_queue_xmit.

I think it is reasonable to expect the mac header to be set in
ndo_start_xmit. Not sure which other devices besides hsr truly
requires it.
Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
If this fixes a bug, it should target net.

Fixes: d346a3fae3ff ("packet: introduce PACKET_QDISC_BYPASS socket option")

This change belongs in __dev_direct_xmit unless all callers except
packet_direct_xmit do correctly set the mac header. xsk_generic_xmit
appears to miss it, too, so would equally trigger this warning.

Re: [PATCH net-next] net/packet: Reset MAC header for direct packet transmission

From: Kurt Kanzenbach <kurt@linutronix.de>
Date: 2021-03-29 07:06:59

On Sun Mar 28 2021, Willem de Bruijn wrote:
On Fri, Mar 26, 2021 at 11:49 AM Kurt Kanzenbach [off-list ref] wrote:
quoted
Reset MAC header in case of using packet_direct_xmit(), e.g. by specifying
PACKET_QDISC_BYPASS. This is needed, because other code such as the HSR layer
expects the MAC header to be correctly set.

This has been observed using the following setup:

|$ ip link add name hsr0 type hsr slave1 lan0 slave2 lan1 supervision 45 version 1
|$ ifconfig hsr0 up
|$ ./test hsr0

The test binary is using mmap'ed sockets and is specifying the
PACKET_QDISC_BYPASS socket option.

This patch resolves the following warning on a non-patched kernel:

|[  112.725394] ------------[ cut here ]------------
|[  112.731418] WARNING: CPU: 1 PID: 257 at net/hsr/hsr_forward.c:560 hsr_forward_skb+0x484/0x568
|[  112.739962] net/hsr/hsr_forward.c:560: Malformed frame (port_src hsr0)

The MAC header is also reset unconditionally in case of PACKET_QDISC_BYPASS is
not used.
At the top of __dev_queue_xmit.

I think it is reasonable to expect the mac header to be set in
ndo_start_xmit. Not sure which other devices besides hsr truly
requires it.
quoted
Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
If this fixes a bug, it should target net.

Fixes: d346a3fae3ff ("packet: introduce PACKET_QDISC_BYPASS socket
option")
OK.
This change belongs in __dev_direct_xmit unless all callers except
packet_direct_xmit do correctly set the mac header. xsk_generic_xmit
appears to miss it, too, so would equally trigger this warning.
It seems like there are only two callers and the XDP part is missing it
as well. I'll move it to __dev_direct_xmit().

Thanks,
Kurt
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help