[PATCH net-next 0/2] bonding: use correct ether type for alb

[PATCH net-next 0/2] bonding: use correct ether type for alb

[Veaceslav Falico]

Hi,

There have been reports that, while using the ETH_P_LOOP ether type
(0x0060), the ether type is treated as its packet length.

To avoid that and to not break already existing apps - add new ether type
ETH_P_LOOPBACK that contains the correct id - 0x9000.


CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: "David S. Miller" <davem@davemloft.net>
CC: Stefan Richter <stefanr@s5r6.in-berlin.de>
CC: Simon Wunderlich <sw@simonwunderlich.de>
CC: Neil Jerram <Neil.Jerram@metaswitch.com>
CC: Simon Horman <horms@verge.net.au>
CC: Arvid Brodin <Arvid.Brodin@xdin.com>
CC: netdev@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>

---
 drivers/net/bonding/bond_alb.c | 2 +-
 include/uapi/linux/if_ether.h  | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

[PATCH net-next 1/2] ether: add loopback type ETH_P_LOOPBACK

[Veaceslav Falico]

Per IEEE 802.3*, the correct packet type for loopback 0x9000. There's
already one ETH_P_LOOP 0x0060, which has been there for ages, however it's
plainly wrong as anything that small is considered a length field.

We can't remove it because legacy, so add a new type which corresponds to
the correct id.

http://www.iana.org/assignments/ieee-802-numbers/ieee-802-numbers.xhtml

CC: "David S. Miller" <davem@davemloft.net>
CC: Stefan Richter <stefanr@s5r6.in-berlin.de>
CC: Simon Wunderlich <sw@simonwunderlich.de>
CC: Neil Jerram <Neil.Jerram@metaswitch.com>
CC: Simon Horman <horms@verge.net.au>
CC: Arvid Brodin <Arvid.Brodin@xdin.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 include/uapi/linux/if_ether.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/if_ether.h b/include/uapi/linux/if_ether.h
index 750ba67..0f8210b 100644
--- a/include/uapi/linux/if_ether.h
+++ b/include/uapi/linux/if_ether.h
@@ -90,6 +90,7 @@
 #define ETH_P_TDLS	0x890D          /* TDLS */
 #define ETH_P_FIP	0x8914		/* FCoE Initialization Protocol */
 #define ETH_P_80221	0x8917		/* IEEE 802.21 Media Independent Handover Protocol */
+#define ETH_P_LOOPBACK	0x9000		/* Ethernet loopback packet, per IEEE 802.3 */
 #define ETH_P_QINQ1	0x9100		/* deprecated QinQ VLAN [ NOT AN OFFICIALLY REGISTERED ID ] */
 #define ETH_P_QINQ2	0x9200		/* deprecated QinQ VLAN [ NOT AN OFFICIALLY REGISTERED ID ] */
 #define ETH_P_QINQ3	0x9300		/* deprecated QinQ VLAN [ NOT AN OFFICIALLY REGISTERED ID ] */
-- 
1.8.4

[PATCH net-next 2/2] bonding: use the correct ether type for alb

[Veaceslav Falico]

Currently it's using the wrong ETH_P_LOOP type, which is sometimes treated
as packet length instead of ether type (because it's 0x0060).

Use the new ETH_P_LOOPBACK type.

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_alb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 9cf836b6..060b611 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1005,7 +1005,7 @@ static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
 	memset(&pkt, 0, size);
 	ether_addr_copy(pkt.mac_dst, mac_addr);
 	ether_addr_copy(pkt.mac_src, mac_addr);
-	pkt.type = cpu_to_be16(ETH_P_LOOP);
+	pkt.type = cpu_to_be16(ETH_P_LOOPBACK);
 
 	skb = dev_alloc_skb(size);
 	if (!skb)
-- 
1.8.4

Re: [PATCH net-next 2/2] bonding: use the correct ether type for alb

[David Miller]

From: Veaceslav Falico <vfalico@redhat.com>
Date: Thu, 13 Mar 2014 12:41:58 +0100

> @@ -1005,7 +1005,7 @@ static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
>  	memset(&pkt, 0, size);
>  	ether_addr_copy(pkt.mac_dst, mac_addr);
>  	ether_addr_copy(pkt.mac_src, mac_addr);
> -	pkt.type = cpu_to_be16(ETH_P_LOOP);
> +	pkt.type = cpu_to_be16(ETH_P_LOOPBACK);

Had this really never worked properly at all?

It's been this way since the beginning of GIT history, and I imagine
since the learning packet code even existed.

Re: [PATCH net-next 2/2] bonding: use the correct ether type for alb

[Veaceslav Falico]

On Fri, Mar 14, 2014 at 02:29:35PM -0400, David Miller wrote:
>From: Veaceslav Falico <vfalico@redhat.com>
>Date: Thu, 13 Mar 2014 12:41:58 +0100
>
>> @@ -1005,7 +1005,7 @@ static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
>>  	memset(&pkt, 0, size);
>>  	ether_addr_copy(pkt.mac_dst, mac_addr);
>>  	ether_addr_copy(pkt.mac_src, mac_addr);
>> -	pkt.type = cpu_to_be16(ETH_P_LOOP);
>> +	pkt.type = cpu_to_be16(ETH_P_LOOPBACK);
>
>Had this really never worked properly at all?

Yep, but only in cases where the hw/sw didn't filter out broken packets
*before* arp processing, as far as I can tell.

>
>It's been this way since the beginning of GIT history, and I imagine
>since the learning packet code even existed.

Re: [PATCH net-next 2/2] bonding: use the correct ether type for alb

[David Miller]

From: Veaceslav Falico <vfalico@redhat.com>
Date: Fri, 14 Mar 2014 19:42:52 +0100

> On Fri, Mar 14, 2014 at 02:29:35PM -0400, David Miller wrote:
>>From: Veaceslav Falico <vfalico@redhat.com>
>>Date: Thu, 13 Mar 2014 12:41:58 +0100
>>
>>> @@ -1005,7 +1005,7 @@ static void alb_send_lp_vid(struct slave *slave,
>>> u8 mac_addr[],
>>>  	memset(&pkt, 0, size);
>>>  	ether_addr_copy(pkt.mac_dst, mac_addr);
>>>  	ether_addr_copy(pkt.mac_src, mac_addr);
>>> -	pkt.type = cpu_to_be16(ETH_P_LOOP);
>>> +	pkt.type = cpu_to_be16(ETH_P_LOOPBACK);
>>
>>Had this really never worked properly at all?
> 
> Yep, but only in cases where the hw/sw didn't filter out broken
> packets
> *before* arp processing, as far as I can tell.

Hmmm... what I meant was, was anyone able to receive these packets?
I think the answer is no.

Re: [PATCH net-next 2/2] bonding: use the correct ether type for alb

[Jay Vosburgh]

David Miller <davem@davemloft.net> wrote:

>From: Veaceslav Falico <vfalico@redhat.com>
>Date: Fri, 14 Mar 2014 19:42:52 +0100
>
>> On Fri, Mar 14, 2014 at 02:29:35PM -0400, David Miller wrote:
>>>From: Veaceslav Falico <vfalico@redhat.com>
>>>Date: Thu, 13 Mar 2014 12:41:58 +0100
>>>
>>>> @@ -1005,7 +1005,7 @@ static void alb_send_lp_vid(struct slave *slave,
>>>> u8 mac_addr[],
>>>>  	memset(&pkt, 0, size);
>>>>  	ether_addr_copy(pkt.mac_dst, mac_addr);
>>>>  	ether_addr_copy(pkt.mac_src, mac_addr);
>>>> -	pkt.type = cpu_to_be16(ETH_P_LOOP);
>>>> +	pkt.type = cpu_to_be16(ETH_P_LOOPBACK);
>>>
>>>Had this really never worked properly at all?
>> 
>> Yep, but only in cases where the hw/sw didn't filter out broken
>> packets
>> *before* arp processing, as far as I can tell.
>
>Hmmm... what I meant was, was anyone able to receive these packets?
>I think the answer is no.

	My recollection is that the intended target of the packets was
the switch the system was connected to, and their only function was to
update the MAC table of the switch (via the source MAC of the frame).

	How well they really work today is debateable (meaning that I'm
not even sure they're necessary, at least not as a regular
transmission), but I recall discussing this with somebody at Intel years
ago and these were evidently necessary at one time.

	The ETH_P_LOOP Linux ended up with was (again, as I recall) a
simplification of some kind of active monitoring system on the Intel ANS
product that the alb/tlb code was derived from, wherein some agent would
receive and reply to the frames.  The ANS product used ethertype 0x886d
for its probes, however.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

Re: [PATCH net-next 2/2] bonding: use the correct ether type for alb

[Veaceslav Falico]

On Fri, Mar 14, 2014 at 03:12:33PM -0400, David Miller wrote:
>From: Veaceslav Falico <vfalico@redhat.com>
>Date: Fri, 14 Mar 2014 19:42:52 +0100
>
>> On Fri, Mar 14, 2014 at 02:29:35PM -0400, David Miller wrote:
>>>From: Veaceslav Falico <vfalico@redhat.com>
>>>Date: Thu, 13 Mar 2014 12:41:58 +0100
>>>
>>>> @@ -1005,7 +1005,7 @@ static void alb_send_lp_vid(struct slave *slave,
>>>> u8 mac_addr[],
>>>>  	memset(&pkt, 0, size);
>>>>  	ether_addr_copy(pkt.mac_dst, mac_addr);
>>>>  	ether_addr_copy(pkt.mac_src, mac_addr);
>>>> -	pkt.type = cpu_to_be16(ETH_P_LOOP);
>>>> +	pkt.type = cpu_to_be16(ETH_P_LOOPBACK);
>>>
>>>Had this really never worked properly at all?
>>
>> Yep, but only in cases where the hw/sw didn't filter out broken
>> packets
>> *before* arp processing, as far as I can tell.
>
>Hmmm... what I meant was, was anyone able to receive these packets?
>I think the answer is no.

Yep, as far as I can tell these packets would be dropped, however their
scope was only to update the CAM table and such on switches, which could
have worked, depending on the 'logic' level of their firmware.

Re: [PATCH net-next 0/2] bonding: use correct ether type for alb

[David Miller]

From: Veaceslav Falico <vfalico@redhat.com>
Date: Thu, 13 Mar 2014 12:41:56 +0100

> There have been reports that, while using the ETH_P_LOOP ether type
> (0x0060), the ether type is treated as its packet length.
> 
> To avoid that and to not break already existing apps - add new ether type
> ETH_P_LOOPBACK that contains the correct id - 0x9000.

Series applied, thanks Veaceslav.