All of lore.kernel.org
 help / color / mirror / Atom feed
* Wrong source MAC for 8021q VLAN interfaces with TX VLAN offload enabled
@ 2014-03-04 19:38 Peter Boström
  2014-03-09 16:39 ` Ben Hutchings
  0 siblings, 1 reply; 2+ messages in thread
From: Peter Boström @ 2014-03-04 19:38 UTC (permalink / raw)
  To: netdev

Hello

The patch committed as 2205369a314e12fcec4781cc73ac9c08fc2b47de (vlan: Fix header ops passthru when doing TX VLAN offload.) seems to have introduced a bug for me. It's reproducible in the current 3.14-rc5 as well as older versions for which the patch was applied.

By configuring a VLAN interface and changing the MAC address to something different from the parent interface, frames will be sent using the wrong source address. Tagged frames will have the source MAC address of the parent interface.

This affects only interfaces with TX VLAN offload enabled. By turning off the TX VLAN offload before creating the VLAN interface it works as expected.



How to reproduce:
# ip link add link eth1 name eth1.2 type vlan proto 802.1q id 2
# ip link set dev eth1.2 address 02:aa:aa:aa:aa:aa
# ip addr add dev eth1.2 10.0.2.1/24
# ip link set dev eth1 up
# ping 10.0.2.2

tcpdump on the receiver shows
16:42:26.757634 02:aa:aa:aa:aa:aa > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 56: Request who-has 10.0.2.2 tell 10.0.2.1, length 42
16:42:26.757644 00:25:90:20:54:b9 > 02:aa:aa:aa:aa:aa, ethertype ARP (0x0806), length 42: Reply 10.0.2.2 is-at 00:25:90:20:54:b9, length 28
16:42:26.757783 00:25:90:d4:30:90 > 00:25:90:20:54:b9, ethertype IPv4 (0x0800), length 98: 10.0.2.1 > 10.0.2.2: ICMP echo request, id 1893, seq 1, length 64
16:42:26.757791 00:25:90:20:54:b9 > 02:aa:aa:aa:aa:aa, ethertype IPv4 (0x0800), length 98: 10.0.2.2 > 10.0.2.1: ICMP echo reply, id 1893, seq 1, length 64
16:42:27.756677 00:25:90:d4:30:90 > 00:25:90:20:54:b9, ethertype IPv4 (0x0800), length 98: 10.0.2.1 > 10.0.2.2: ICMP echo request, id 1893, seq 2, length 64
16:42:27.756683 00:25:90:20:54:b9 > 02:aa:aa:aa:aa:aa, ethertype IPv4 (0x0800), length 98: 10.0.2.2 > 10.0.2.1: ICMP echo reply, id 1893, seq 2, length 64

Notice that the ARP packets are actually sent with the correct(configured) source MAC while the ICMP is sent using the address of the parent interface(00:25:90:d4:30:90).



A potential fix is to set the saddr in the vlan_passthru_hard_header function.
This works for me. But since I lack a good understanding of how the TX VLAN offload actually works I can't really tell whether this is a proper solution or not.
Any input would be appreciated.

--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -538,6 +538,9 @@
      struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
      struct net_device *real_dev = vlan->real_dev;

+    if (!saddr)
+        saddr = dev->dev_addr;
+
      return dev_hard_header(skb, real_dev, type, daddr, saddr, len);
  }

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Wrong source MAC for 8021q VLAN interfaces with TX VLAN offload enabled
  2014-03-04 19:38 Wrong source MAC for 8021q VLAN interfaces with TX VLAN offload enabled Peter Boström
@ 2014-03-09 16:39 ` Ben Hutchings
  0 siblings, 0 replies; 2+ messages in thread
From: Ben Hutchings @ 2014-03-09 16:39 UTC (permalink / raw)
  To: Peter Boström; +Cc: netdev

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

On Tue, 2014-03-04 at 20:38 +0100, Peter Boström wrote:
> Hello
> 
> The patch committed as 2205369a314e12fcec4781cc73ac9c08fc2b47de (vlan:
> Fix header ops passthru when doing TX VLAN offload.) seems to have
> introduced a bug for me. It's reproducible in the current 3.14-rc5 as
> well as older versions for which the patch was applied.
[...]
> A potential fix is to set the saddr in the vlan_passthru_hard_header
> function.
> This works for me. But since I lack a good understanding of how the TX
> VLAN offload actually works I can't really tell whether this is a
> proper solution or not.
> Any input would be appreciated.
> 
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -538,6 +538,9 @@
>       struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
>       struct net_device *real_dev = vlan->real_dev;
> 
> +    if (!saddr)
> +        saddr = dev->dev_addr;
> +
>       return dev_hard_header(skb, real_dev, type, daddr, saddr, len);
>   }
> 

This makes sense to me.  You should re-send this patch with proper
formatting and your sign-off (see Documentation/SubmittingPatches,
Documentation/email-clients.txt).

Ben.

-- 
Ben Hutchings
I say we take off; nuke the site from orbit.  It's the only way to be sure.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-03-09 16:39 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-04 19:38 Wrong source MAC for 8021q VLAN interfaces with TX VLAN offload enabled Peter Boström
2014-03-09 16:39 ` Ben Hutchings

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.