netdev - Re: [PATCH net-next v3 01/26] switchdev: introduce get/set attrs ops

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:	Thu, 2 Apr 2015 10:52:28 -0700
From:	Scott Feldman <sfeldma@...il.com>
To:	Florian Fainelli <f.fainelli@...il.com>
Cc:	netdev <netdev@...r.kernel.org>,
	Jiří Pírko <jiri@...nulli.us>,
	Roopa Prabhu <roopa@...ulusnetworks.com>,
	Guenter Roeck <linux@...ck-us.net>,
	"Samudrala, Sridhar" <sridhar.samudrala@...el.com>,
	"Arad, Ronen" <ronen.arad@...el.com>
Subject: Re: [PATCH net-next v3 01/26] switchdev: introduce get/set attrs ops

On Thu, Apr 2, 2015 at 2:09 AM, Florian Fainelli <f.fainelli@...il.com> wrote:
> [snip]
>
>> +       int err = -EOPNOTSUPP;
>> +
>> +       if (!(dev->features & NETIF_F_HW_SWITCH_OFFLOAD))
>> +               return err;
>
> That check is currently going to prevent DSA from working, since we do
> not yet advertise NETIF_HW_SWITCH_OFFLOAD (which should be fixed). In
> general, though I am not sure this is entirely desirable to check that
> here for multiple reasons:
>
> - stacked devices typically propagate lower devices dev->features, but
> if they are purposely not doing it, this might start breaking
> - is not that check, used as it is now, that unconditionally, end-up
> being redundant with e.g: getting the switch device id to identify
> this net_device as a switch port net_device?
>
> I kind of preferred when we had this moved into the
> __swdev_attr_{get,set} caller, such we had finer control over whether
> or not checking for these kinds of features makes sense for a
> particular operation.

I dropped it in my v1 patch set all together, but then it came back in
v2.  I understand how it works, giving the user the ability to turn
on/off offload support on a port at run-time, but I don't understand
the application.  I agree with you that we already have finer-grained
ability to know if a sub-feature is supported or not based on what the
driver implements (like switch ID or STP state).  I don't know what
this master switch is used for.  Why would the user turn off
offloading on a port at run-time after the device has already been
programmed with some offloading tasks?  What tells the device to stop
those offloads now.  And then later, the user flips the switch to turn
back on offloads on the port.   How do we restore the device?

Roopa, can you help us understand how NETIF_F_HW_SWITCH_OFFLOAD is used?
--
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