netdev - Re: [RFC v2, net-next, PATCH 4/4] net/cpsw_switchdev: add switchdev mode of operation on cpsw driver

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, 18 Jun 2018 18:20:37 -0500
From:   Grygorii Strashko <grygorii.strashko@...com>
To:     Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Andrew Lunn <andrew@...n.ch>
CC:     <netdev@...r.kernel.org>, <ivan.khoronzhuk@...aro.org>,
        <nsekhar@...com>, <jiri@...nulli.us>, <ivecera@...hat.com>,
        <f.fainelli@...il.com>, <francois.ozog@...aro.org>,
        <yogeshs@...com>, <spatton@...com>, <Jose.Abreu@...opsys.com>
Subject: Re: [RFC v2, net-next, PATCH 4/4] net/cpsw_switchdev: add switchdev
 mode of operation on cpsw driver



On 06/18/2018 03:19 PM, Ilias Apalodimas wrote:
> On Mon, Jun 18, 2018 at 06:16:27PM +0200, Andrew Lunn wrote:
>>> @@ -2711,6 +2789,10 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
>>>   	if (of_property_read_bool(node, "dual_emac"))
>>>   		data->switch_mode = CPSW_DUAL_EMAC;
>>>   
>>> +	/* switchdev overrides DTS */
>>> +	if (IS_ENABLED(CONFIG_TI_CPSW_SWITCHDEV))
>>> +		data->switch_mode = CPSW_SWITCHDEV;
>>> +
>>
>> I know you discussed this a bit with Jiri, but i still think if
>> 'dual_mac" is found, you should do dual mac. The DT clearly requests
>> dual mac, and doing anything else is going to cause confusion.
>>
>> The device tree binding is ambiguous what should happen when dual-mac
>> is missing. So i would only enable swithdev mode in that case.
> At the moment if no 'dual_emac;' is found on DTS the driver operates in "switch
> mode". It only registers 1 ethernet interface with no ability (unless you patch
> the kernel) to configure the switch. If we use DTS instead of a .config option
> we should add parsing for something like 'switchdev;' in the DTS.
> Jiri proposed using devlink, which makes sense, but i am not sure it's
> applicable on this patchset. This will change the driver completely and will
> totally break backwards compatibility.
> 
> Ideally i'd prefer something like:
> 1. Add a DTS option and continue the current behavior. I agree with you that
> this will cause less confusion (in fact i prefer it for the current state of the
> driver compared to the .config).
> or
> 2. Keep the .config option which is better suited over DTS but might cause some
> confusion.
>>
>> But ideally, it should be a new driver with a new binding.
> TI is better suited to comment on this. The work proposed here is mostly to
> accomodate future TSN related configuration for switches. This patchset has
> been tested against Ivan's patchset for CBS and is working as expected
> configuration wise.
> (http://lkml.iu.edu/hypermail/linux/kernel/1806.1/05302.html)

We'd try the new driver in the future

-- 
regards,
-grygorii