netdev - Re: [patch net-next 09/14] rocker: add rtnl ops for port mode [gs]etting

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]

Message-ID: <20151005165004.GQ2278@nanopsycho.orion>
Date:	Mon, 5 Oct 2015 18:50:04 +0200
From:	Jiri Pirko <jiri@...nulli.us>
To:	Scott Feldman <sfeldma@...il.com>
Cc:	Netdev <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Ido Schimmel <idosch@...lanox.com>, eladr@...lanox.com,
	Thomas Graf <tgraf@...g.ch>,
	Alexei Starovoitov <ast@...mgrid.com>
Subject: Re: [patch net-next 09/14] rocker: add rtnl ops for port mode
 [gs]etting

Mon, Oct 05, 2015 at 06:37:20PM CEST, sfeldma@...il.com wrote:
>On Mon, Oct 5, 2015 at 8:53 AM, Jiri Pirko <jiri@...nulli.us> wrote:
>> Mon, Oct 05, 2015 at 05:41:29PM CEST, sfeldma@...il.com wrote:
>>>On Sun, Oct 4, 2015 at 2:25 PM, Jiri Pirko <jiri@...nulli.us> wrote:
>>>> From: Jiri Pirko <jiri@...lanox.com>
>>>>
>>>> Introduce a stub for allowing user to change rocker port world/mode.
>>>> This is implemented using rtnl changelink op.
>>>>
>>>> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
>>>
>>>[cut]
>>>
>>>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>>>> index 3a5f263..7da768e 100644
>>>> --- a/include/uapi/linux/if_link.h
>>>> +++ b/include/uapi/linux/if_link.h
>>>> @@ -416,6 +416,17 @@ enum {
>>>>  };
>>>>  #define IFLA_GENEVE_MAX        (__IFLA_GENEVE_MAX - 1)
>>>>
>>>> +/* Rocker section */
>>>> +enum {
>>>> +       IFLA_ROCKER_UNSPEC,
>>>> +       IFLA_ROCKER_MODE,
>>>> +       __IFLA_ROCKER_MAX,
>>>> +};
>>>> +
>>>> +#define IFLA_ROCKER_MAX        (__IFLA_ROCKER_MAX - 1)
>>>> +
>>>> +#define ROCKER_MODE_MAX 16
>>>
>>>Does this mean there is going to be a "ip link set dev DEV type rocker
>>>mode MODE" command option?
>>>
>>>It doesn't seem right to be adding driver-specific IFLA_'s here.  I
>>>think this sets bad precedence for other drivers to add their own
>>>knobs without thinking about a generic shared mechanism.
>>
>> I understand you point. I somehow share it as well. But on the other
>> hand, That is neat way to set mode of the port, and I cannot find other
>> this nice.
>>
>>
>>>
>>>Actually, I don't see the point of letting the user dynamically change
>>>the port mode.  I would prefer this knob be moved to qemu/rocker.  Let
>>>the port mode be specified on device creation.
>>
>> Hmm, interesting, why? I find it great for user to be able to switch the
>> port mode easily on the running system. It is a setting of a port.
>> I don't see why this should be a hard-coded hw setting. We just have
>> to find a way to expose this to user.
>
>You could just as easy restart the qemu session with different port
>mode.  Even if rocker was a real device, I don't see (real) users
>wanting to change the port mode at run-time.  It just doesn't seem
>like a real-world scenario.   I know it's convenient for developers,

We the hw-iface is already designed for on-fly changes.
I'm convinced that this is one of the situation where we should not do
stuff in HW just because we can. (Or we should do both).

We should develop in-kernel solution in order to wrap up HW functionality.
Even if not easy. That is the purpose of rocker from day 1, and that is
why we love it :)


>but a qemu knob is just as convenient.  Doing this in qemu will keep
>the rocker driver less complicated, cleaner.

I'm very sure that in driver, there would be very minimal changes
between those two solutions. Couple lines. No problem there.
--
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