netdev - Re: [patch net-next v3 02/10] switchdev: introduce transaction item queue for attr_set and 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20151001161706.GA1836@ketchup.mtl.sfl>
Date:	Thu, 1 Oct 2015 12:17:06 -0400
From:	Vivien Didelot <vivien.didelot@...oirfairelinux.com>
To:	Andrew Lunn <andrew@...n.ch>
Cc:	Scott Feldman <sfeldma@...il.com>, Jiri Pirko <jiri@...nulli.us>,
	Netdev <netdev@...r.kernel.org>,
	Ido Schimmel <idosch@...lanox.com>, eladr@...lanox.com,
	Florian Fainelli <f.fainelli@...il.com>,
	Guenter Roeck <linux@...ck-us.net>,
	"Rosen, Rami" <rami.rosen@...el.com>,
	Roopa Prabhu <roopa@...ulusnetworks.com>,
	Premkumar Jonnala <pjonnala@...adcom.com>,
	Andy Gospodarek <gospo@...ulusnetworks.com>,
	Jiri Pirko <jiri@...lanox.com>
Subject: Re: [patch net-next v3 02/10] switchdev: introduce transaction item
 queue for attr_set and obj_add

Hi Andrew,

On Oct. Thursday 01 (40) 05:28 PM, Andrew Lunn wrote:
> > > Does this help?  Maybe we should walk before running and focus on
> > > getting non-batch ops working and then revisit?
> > 
> > I agree. I understand the need for a prepare phase, but it looks like it
> > exists for specific combinations, i.e. stacked and bonded devices.
> > 
> > For basic Ethernet switch chips (even DSA), it is *for the moment* a bit
> > too unnecessarily complex.
> 
> I think we are going to need it though. I have bonding on my TODO
> list. That will put DSA into a stacked system.
>  
> > What I will suggest next, is to explicitly skip the prepare phase in DSA
> > (with a good comment as you already suggested), and fix switchdev to
> > allow drivers to return -EOPNOTSUPP from its commit phase.
> 
> The switches have a limited number of bonds, called trunks in Marvells
> terminology. So we will need the prepare phase to say: Sorry, im out
> of trunks, do it in software. And different chips have different
> numbers of trunks, so it will need to go down into the chip driver,
> the DSA layer probably cannot decide.

I understand. Please consider my last comment, which is not invalidated
here: asking the hardware for a support, and the prepare transaction
framework are 2 differents things here.

In other words: struct switchdev_trans != -EOPNOTSUPP.

Even with Marvell switches, bonding 2 ports may not be supported, but if
it does, it won't need the switchdev_trans framework for this operation.

If we take a look at how dsa_slave_stp_update works today in
net/dsa/slave.c, it is quite complicated:

    dsa_slave_stp_update(dev, state)
        if (ds->drv->port_stp_update)
	    return ds->drv->port_stp_update(port, state);
        else
	    return -EOPNOTSUPP;

This checks that the driver implemented this operation. And:

    dsa_slave_port_attr_set(dev, attr, trans)
        ...
	case SWITCHDEV_ATTR_PORT_STP_STATE:
	    if (switchdev_trans_ph_prepare(trans))
	        return ds->drv->port_stp_update ? 0 : -EOPNOTSUPP;
	    else
	        return ds->drv->port_stp_update(port, state);

DSA drivers don't need dynamic resources from the kernel. But they may
not support an object. I would like to make the distinction, by skipping
the prepare phase in the DSA framework and allowing DSA drivers to
eventually return -EOPNOTSUPP from their feature implementation. It
makes no difference for the caller of switchdev_port_{obj_add,attr_set}
anyway.

Thanks,
-v
--
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