net/vxlan: Fix kernel unaligned access in __vxlan_find_mac

RFC PATCH Superseded Sowmini Varadhan 3 participants 9 messages Jul 17, 2015 – Jul 19, 2015

Subsystems (3)

Networking Network drivers SPARC + UltraSPARC (sparc/sparc64)

lore

Share & tools

lore

Sowmini Varadhan

author

2015-07-17 20:00 UTC #0 lore raw

RFC PATCH v2 1/1 net/vxlan: Fix kernel unaligned access in __vxlan_find_mac

__vxlan_find_mac invokes ether_addr_equal on the eth_addr field, which triggers unaligned access messages, so rearrange vxlan_fdb to avoid this in the most non-intrusive way.

Signed-off-by Sowmini Varadhan <[email protected]>

---
v2: Alexander Duyck comments: make eth_addr[] 64b aligned.

 drivers/net/vxlan.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/net/vxlan.c +1 -1

index 34c519e..ec86a11 100644
@@ -106,9 +106,9 @@ struct vxlan_fdb {
 	unsigned long	  updated;	/* jiffies */
 	unsigned long	  used;
 	struct list_head  remotes;
+	u8		  eth_addr[ETH_ALEN];
 	u16		  state;	/* see ndm_state */
 	u8		  flags;	/* see ndm_flags */
-	u8		  eth_addr[ETH_ALEN];
 };
 
 /* Pseudo network device */

Signature · 1 line

1.7.1

Joe Perches

2015-07-17 23:07 UTC #1 lore raw

Quoted 3 lines · Sowmini Varadhan

__vxlan_find_mac invokes ether_addr_equal on the eth_addr field, which triggers unaligned access messages, so rearrange vxlan_fdb to avoid this in the most non-intrusive way.

What arch does this?

David Miller

maintainer

2015-07-18 01:05 UTC #2 lore raw

From: Joe Perches <[email protected]>
Date: Fri, 17 Jul 2015 16:07:02 -0700

Quoted 6 lines · Sowmini Varadhan

Quoted 3 lines
__vxlan_find_mac invokes ether_addr_equal on the eth_addr field, which triggers unaligned access messages, so rearrange vxlan_fdb to avoid this in the most non-intrusive way.
What arch does this?

Sparc, MIPS, etc.

Joe Perches

2015-07-18 18:06 UTC #3 lore raw

Quoted 9 lines · David Miller

From: Joe Perches <[email protected]> Date: Fri, 17 Jul 2015 16:07:02 -0700
Quoted 6 lines · Sowmini Varadhan
Quoted 3 lines
__vxlan_find_mac invokes ether_addr_equal on the eth_addr field, which triggers unaligned access messages, so rearrange vxlan_fdb to avoid this in the most non-intrusive way.
What arch does this?
Sparc, MIPS, etc.

It seems that this code has had unaligned accesses on this field even before compare_ether_addr was converted to ether_addr_equal.

Is sparc64 the only one that emits / ratelimits that unaligned access message? I looked a little, but I didn't find a fixup message when MIPS does unaligned accesses. Are all the other arches silent when fixing up unaligned accesses? Maye adding a generic debug only ratelimited message might help remove more of these. As it's not fatal, naybe the sparc64 message should be KERN_DEBUG/pr_debug.

---

 arch/sparc/kernel/unaligned_64.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

arch/sparc/kernel/unaligned_64.c +2 -6

index 62098a8..6b7aeb7 100644
@@ -294,13 +294,9 @@ static void kernel_mna_trap_fault(int fixup_tstate_asi)
 
 static void log_unaligned(struct pt_regs *regs)
 {
-	static DEFINE_RATELIMIT_STATE(ratelimit, 5 * HZ, 5);
+	pr_debug_ratelimited("Kernel unaligned access at TPC[%lx] %pS\n",
+			     regs->tpc, (void *)regs->tpc);
 
-	if (__ratelimit(&ratelimit)) {
-		printk("Kernel unaligned access at TPC[%lx] %pS\n",
-		       regs->tpc, (void *) regs->tpc);
-	}
-}
 
 asmlinkage void kernel_unaligned_trap(struct pt_regs *regs, unsigned int insn)
 {

David Miller

maintainer

2015-07-19 05:23 UTC #4 lore raw

From: Joe Perches <[email protected]>
Date: Sat, 18 Jul 2015 11:06:26 -0700

Quoted 2 lines

As it's not fatal, naybe the sparc64 message should be KERN_DEBUG/pr_debug.

It's not fatal insofar as it doesn't crash the system.

But performance wise is _IS_ fatal, and I want the kernel to bark at the user so that they report the issue or fix it sooner rather than later.

Sowmini Varadhan

author

2015-07-19 12:01 UTC #5 lore raw

In reply to Joe Perches

Quoted 4 lines · Joe Perches

It seems that this code has had unaligned accesses on this field even before compare_ether_addr was converted to ether_addr_equal.

Quoted 8 lines

Is sparc64 the only one that emits / ratelimits that unaligned access message? I looked a little, but I didn't find a fixup message when MIPS does unaligned accesses. Are all the other arches silent when fixing up unaligned accesses? Maye adding a generic debug only ratelimited message might help remove more of these. As it's not fatal, naybe the sparc64 message should be KERN_DEBUG/pr_debug.

I'm confused, are we suggesting that we "fix" the unaligned access by snuffing out the message that complains loudly and correctly about it?

See also: large block comment above __pksb_trim about correctly using skb_reserve(). Evidently not being correctly done for the IPv6-vxlan code path (and possibly for other encaps too?)

--Sowmini

David Miller

maintainer

2015-07-19 18:38 UTC #6 lore raw

From: Sowmini Varadhan <[email protected]>
Date: Sun, 19 Jul 2015 14:01:34 +0200

Quoted 15 lines · Joe Perches

Quoted 5 lines
It seems that this code has had unaligned accesses on this field even before compare_ether_addr was converted to ether_addr_equal.
Quoted 8 lines
Is sparc64 the only one that emits / ratelimits that unaligned access message? I looked a little, but I didn't find a fixup message when MIPS does unaligned accesses. Are all the other arches silent when fixing up unaligned accesses? Maye adding a generic debug only ratelimited message might help remove more of these. As it's not fatal, naybe the sparc64 message should be KERN_DEBUG/pr_debug.

Quoted 8 lines

I'm confused, are we suggesting that we "fix" the unaligned access by snuffing out the message that complains loudly and correctly about it?
See also: large block comment above __pksb_trim about correctly using skb_reserve(). Evidently not being correctly done for the IPv6-vxlan code path (and possibly for other encaps too?)

We should fix the unaligned accesses, rather than quiet the warning.

Joe Perches

2015-07-19 19:07 UTC #7 lore raw

Quoted 22 lines · David Miller

From: Sowmini Varadhan <[email protected]>
Date: Sun, 19 Jul 2015 14:01:34 +0200
Quoted 19 lines · Joe Perches
Quoted 5 lines
It seems that this code has had unaligned accesses on this field even before compare_ether_addr was converted to ether_addr_equal.
Quoted 8 lines
Is sparc64 the only one that emits / ratelimits that unaligned access message? I looked a little, but I didn't find a fixup message when MIPS does unaligned accesses. Are all the other arches silent when fixing up unaligned accesses? Maye adding a generic debug only ratelimited message might help remove more of these. As it's not fatal, naybe the sparc64 message should be KERN_DEBUG/pr_debug.
I'm confused, are we suggesting that we "fix" the unaligned access by snuffing out the message that complains loudly and correctly about it?

Quoted 7 lines

Quoted 4 lines
See also: large block comment above __pksb_trim about correctly using skb_reserve(). Evidently not being correctly done for the IPv6-vxlan code path (and possibly for other encaps too?)
We should fix the unaligned accesses, rather than quiet the warning.

Definitely so, the question I have is whether or not the the message should be able to be silenced, not just ratelimited.

I think there are likely to be occasions when, given arbitrary protocol stacking, unaligned accesses are unavoidable.

Perhaps batman might have this as an actual issue.

Sowmini Varadhan

author

2015-07-18 04:46 UTC #8 lore raw

In reply to Joe Perches

Quoted 6 lines · Joe Perches

Quoted 3 lines · Sowmini Varadhan
__vxlan_find_mac invokes ether_addr_equal on the eth_addr field, which triggers unaligned access messages, so rearrange vxlan_fdb to avoid this in the most non-intrusive way.
What arch does this?

sparc.

BTW, I was also getting a lot of alignment errors from vxlan_xmit_skb (vxh comes out unaligned) for the IPv6 path. I did not have time to investigate/fix this correctly- not sure if this is specific to v6.

--Sowmini