netdev - Re: [PATCH net-next 08/12] ice: Add VLAN FDB support in switchdev mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 8 May 2023 16:09:46 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: "Drewek, Wojciech" <wojciech.drewek@...el.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Ertman, David M"
	<david.m.ertman@...el.com>, "michal.swiatkowski@...ux.intel.com"
	<michal.swiatkowski@...ux.intel.com>, "marcin.szycik@...ux.intel.com"
	<marcin.szycik@...ux.intel.com>, "Chmielewski, Pawel"
	<pawel.chmielewski@...el.com>, "Samudrala, Sridhar"
	<sridhar.samudrala@...el.com>
Subject: Re: [PATCH net-next 08/12] ice: Add VLAN FDB support in switchdev
 mode

From: Wojciech Drewek <wojciech.drewek@...el.com>
Date: Thu, 27 Apr 2023 12:28:21 +0200

> 
> 
>> -----Original Message-----
>> From: Lobakin, Aleksander <aleksander.lobakin@...el.com>
>> Sent: piątek, 21 kwietnia 2023 17:25
>> To: Drewek, Wojciech <wojciech.drewek@...el.com>
>> Cc: intel-wired-lan@...ts.osuosl.org; netdev@...r.kernel.org; Lobakin, Aleksander <aleksander.lobakin@...el.com>; Ertman, David M
>> <david.m.ertman@...el.com>; michal.swiatkowski@...ux.intel.com; marcin.szycik@...ux.intel.com; Chmielewski, Pawel
>> <pawel.chmielewski@...el.com>; Samudrala, Sridhar <sridhar.samudrala@...el.com>
>> Subject: Re: [PATCH net-next 08/12] ice: Add VLAN FDB support in switchdev mode

[...]

>>> +		break;
>>> +	default:
>>> +		return -EOPNOTSUPP;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>
>> [...]
>>
>>> +	br_offloads->switchdev_blk.notifier_call =
>>> +		ice_eswitch_br_event_blocking;
>>
>> Oh, you have two usages of ->switchdev_blk here, so you can add an
>> intermediate variable to avoid line breaking, which would also shorten
>> the line below :D
>>
>> 	nb = &br_offloads->switchdev_blk;
>> 	nb->notifier_call = ice_eswitch_br_event_blocking;
>> 	...
> 
> Hmmm, I feel like it is more readable right now. It's clear that we're registering
> switchdev blocking notifier block (switchdev_blk). Introducing generic variable (nb)
> might a bit ambiguous IMO. So if you have nothing against it I'd leave it as it is.

Noprob, up to you :)

[...]

>>>  	int ifindex;
>>
>> (BTW, ifindex is also usually unsigned unless it's not an error)
> 
> It is defined as int in net_device, so I don’t know

I know, but back then people don't care much and were usually defining
everything that doesn't need explicit size as `int` :D No interfaces
have a negative index, indexes start from 1, which is reserved for
loopback, so even 0 is not used. Up to you anyway, I just usually don't
use sign when it's not needed. But it's more of a personal.

[...]

Thanks,
Olek