[Bridge] Bridge sysfs port_no overflow

[Osama Abu Elsorour]

Yes. I guess the unused field came in handy. Thanks!

On Mar 31, 2008, at 5:42 PM, Stephen Hemminger wrote:
> On Mon, 31 Mar 2008 09:11:31 +0200
> Osama Abu Elsorour <fobowise at gmail.com> wrote:
>
>> All
>>
>> We are running a setup with a large number of bridge ports that
>> reaches the 900 ports. After switching to recent kernel and brctl-
>> utils that uses the sysfs interface, we started noticing that the  
>> port
>> numbers are mis-reported when issues the command:
>> brctl showmacs br1
>> After tracing the code, we found that the problem lies in the sysfs
>> structure called __fdb_entry. The port_no is declared as a u8 while  
>> it
>> is u16 in the rest of the bridge structure. This causes the port_no  
>> to
>> overflow when the bridge port number exceeds 255.
>>
>> The overflow line is in file br_fdb.c function br_fdb_fillbuf:
>>                         fe->port_no = f->dst->port_no;
>> where left hand port_no is _u8 and right hand is _u16.
>>
>> Even if it is unusual to have this number of ports on a single bridge
>> it should be changed to the sake of consistency.
>> 	
>> This patch shows the change:
>>
>> @@ -94,7 +94,7 @@ struct __port_info
>> struct __fdb_entry
>> {
>> 	__u8 mac_addr[6];
>> -	__u8 port_no;
>> +	__u16 port_no;
>> 	__u8 is_local;
>> 	__u32 ageing_timer_value;
>> 	__u32 unused;
>
> The problem is that this changes the size of the binary data structure
> and therefore changes the API. Better to do something with the unused
> field and maintain binary compatibility.
>
> Like this:
>
> --- a/include/linux/if_bridge.h	2008-03-31 08:37:57.000000000 -0700
> +++ b/include/linux/if_bridge.h	2008-03-31 08:39:02.000000000 -0700
> @@ -94,10 +94,11 @@ struct __port_info
> struct __fdb_entry
> {
> 	__u8 mac_addr[6];
> -	__u8 port_no;
> +	__u8 old_port_no;
> 	__u8 is_local;
> 	__u32 ageing_timer_value;
> -	__u32 unused;
> +	__u16 port_no;
> +	__u16 unused;
> };
>
> #ifdef __KERNEL__
> --- a/net/bridge/br_fdb.c	2008-03-31 08:39:23.000000000 -0700
> +++ b/net/bridge/br_fdb.c	2008-03-31 08:41:32.000000000 -0700
> @@ -285,6 +285,7 @@ int br_fdb_fillbuf(struct net_bridge *br
>
> 			/* convert from internal format to API */
> 			memcpy(fe->mac_addr, f->addr.addr, ETH_ALEN);
> +			fe->old_port_no = f->dst->port_no;
> 			fe->port_no = f->dst->port_no;
> 			fe->is_local = f->is_local;
> 			if (!f->is_static)