netdev - Re: [PATCH net-next v2 25/26] switchdev: convert swdev_fib_ipv4_add/del over to swdev_port_obj

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:	Mon, 6 Apr 2015 14:56:45 -0700
From:	Scott Feldman <sfeldma@...il.com>
To:	Florian Fainelli <f.fainelli@...il.com>
Cc:	Jiri Pirko <jiri@...nulli.us>, roopa <roopa@...ulusnetworks.com>,
	Netdev <netdev@...r.kernel.org>,
	Guenter Roeck <linux@...ck-us.net>,
	"Samudrala, Sridhar" <sridhar.samudrala@...el.com>,
	"Arad, Ronen" <ronen.arad@...el.com>
Subject: Re: [PATCH net-next v2 25/26] switchdev: convert swdev_fib_ipv4_add/del
 over to swdev_port_obj_add/del

On Fri, Apr 3, 2015 at 11:00 AM, Florian Fainelli <f.fainelli@...il.com> wrote:
> On 01/04/15 09:05, Jiri Pirko wrote:
>>> This looks nice. However, my only concern is we have now ended up adding a
>>> whole layer of abstraction to all objects. The api abstraction seemed fine.
>>> But, carrying it to objects and duplicating every object in the swdev layer
>>> seems too much duplication. Maybe its just me.
>>> we will end up adding this for bond attributes...vxlan...fdb and nftables
>>
>> +1
>
> +1.
>
>>
>>> etc.
>>>
>>> The advantage of switchdev in the kernel was to have access the existing
>>> kernel api and objects directly from switchdev layer.
>>> In which case, to me, it would be better to skip this extra layer of objects.
>>> And keep the way you had it originally.
>
> Same here, I am not exactly sure how much good the abstract object is
> giving us here since we are already in the kernel, and we cannot/do not
> necessarily want to eliminate a large number of swdev_ops, as these are
> precisely the API we want to define.
>
> NB: if we were to do that for swdev_ops, the next step would be doing
> the same thing for netdev_ops to reduce their number, would not we?
>
> Maybe for now solving the stacked device problem and
> prepare/commit/rollback is good enough to keep you busy that adding the
> object layer is a completely secondary problem to solve ;)?

To solve the stacked device problem and support
prepare/commit/rollback, for both attrs and objs, but using a
parameter list call structure for each attr/obj type will result in a
lot of duplicated code.  You'll end up replicating the basic
lower-dev-walk recursion algo for each, and also replicating the
prepare/commit/rollback logic.  It's the typed parameter list unique
for each obj or attr that messes things up.  Going to the attr/obj
model let's us write the algos once.

We're also eliminating the swdev ops which pass netlink args.  There
are cases where we want to originate the swdev ops call within the
kernel, outside of a netlink message context.  For example, flushing
FDB entries from the device.  Today, to flush FDB entries from the
device, we'd need to call ndo_fdb_del() and pass in netlink arguments
that are made-up.  We create a fake struct ndmsg *ndm and a fake
struct nlattr *tb[] and pass those in to ndo_fdb_del() and hope no-one
expects anything useful in those fake structures.  Moving to the
attr/obj model, we can make the direct in-kernel call to flush the FDB
entries from the device without having to fake a netlink call inside
the kernel.

FDB entry obj would be the next obj to add.  The stacked support and
transaction support come for free.

-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