netdev - Re: [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures

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] [day] [month] [year] [list]

Date:	Wed, 9 Sep 2015 09:25:14 -0700
From:	Scott Feldman <sfeldma@...il.com>
To:	Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Cc:	Netdev <netdev@...r.kernel.org>, Jiri Pirko <jiri@...nulli.us>,
	Florian Fainelli <f.fainelli@...il.com>,
	Andrew Lunn <andrew@...n.ch>,
	kernel <kernel@...oirfairelinux.com>
Subject: Re: [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures

On Tue, Sep 8, 2015 at 1:47 PM, Vivien Didelot
<vivien.didelot@...oirfairelinux.com> wrote:
> Hi!
>
> Current implementations of .switchdev_port_obj_add and .switchdev_port_obj_dump
> must pass the generic switchdev_obj structure instead of a specific one (e.g.
> switchdev_obj_fdb) to the related driver accessors, because it contains the
> object transaction and callback. This is not very convenient for drivers.
>
> Instead of having all specific structures included into a switchdev_obj union,
> it would be simpler to embed a switchdev_obj structure as the first member of
> the more specific ones. That way, a driver can cast the switchdev_obj pointer
> to the specific structure and have access to all the information from it.
>
> As an example, this allows Rocker to change its two FDB accessors:
>
>     rocker_port_fdb_add(rocker_port, obj->trans, &obj->u.fdb);
>     rocker_port_fdb_dump(rocker_port, obj);
>
> for these most consistent ones:
>
>     fdb = (struct switchdev_obj_fdb *) obj;
>     rocker_port_fdb_add(rocker_port, fdb);
>     rocker_port_fdb_dump(rocker_port, fdb);
>
> This is what struct netdev_notifier_info and its specific supersets (e.g.
> struct netdev_notifier_changeupper_info) do in include/linux/netdevice.h.
>
> This patchset does that and updates bridge, Rocker and DSA accordingly.
>
> Note that this patchset was sent as an RFC not to bother David with new
> net-next stuffs, but if the changes look good, it is ready to merge.
>
> Also, please take note that the change sits on top of:
> http://patchwork.ozlabs.org/patch/514894/
>

HI Vivien, looks good. Some requests for the next version:

1) Move the cb() member out of struct switchdev_obj and into the
containing struct switchdev_obj_xxx structure.  This way the second
arg to cb() can be struct switchdev_obj_xxx *.  (And, I have pending
patches where the first arg to cb() is different, so making the dump
cb() func specific to obj type will help).

2) As already mentioned, use container_of where possible to avoid
locking struct member at a position within struct.

Should the same treatment be done for switchdev_attr?  Maybe that's a
second patchset.

-scott
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html