netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
To: Jiri Pirko <jiri@resnulli.us>, sfeldma@gmail.com
Cc: sfeldma@gmail.com, jiri@resnulli.us, netdev@vger.kernel.org,
	idosch@mellanox.com, eladr@mellanox.com, f.fainelli@gmail.com,
	linux@roeck-us.net, rami.rosen@intel.com,
	roopa@cumulusnetworks.com, pjonnala@broadcom.com, andrew@lunn.ch,
	gospo@cumulusnetworks.com, jiri@mellanox.com
Subject: Re: [patch net-next v3 02/10] switchdev: introduce transaction item queue for attr_set and obj_add
Date: Wed, 30 Sep 2015 14:56:52 -0400	[thread overview]
Message-ID: <20150930185652.GA2455@ketchup.lan> (raw)
In-Reply-To: <20150925150303.GA21567@ketchup.mtl.sfl>

Hi all,

On Sep. Friday 25 (39) 11:03 AM, Vivien Didelot wrote:
> On Sep. Thursday 24 (39) 10:55 PM, David Miller wrote:
> > From: Scott Feldman <sfeldma@gmail.com>
> > Date: Thu, 24 Sep 2015 22:29:43 -0700
> > 
> > > I'd rather keep 2-phase not optional, or at least make it some what of
> > > a pain for drivers to opt-out of 2-phase.  Forcing the driver to see
> > > both phases means the driver needs to put some code to skip phase 1
> > > (and hopefully has some persistent comment explaining why its being
> > > skipped).  Something like:
> > > 
> > > /* I'm skipping phase 1 prepare for this operation.  I have infinite hardware
> > >  * resources and I'm not setting any persistent state in the driver or device
> > >  * and I don't need any dynamic resources from the kernel, so its impossible
> > >  * for me to fail phase 2 commit.  Nothing to prepare, sorry.
> > >  */
> > 
> > I agree with Scott here.
> > 
> > If you can opt out of something, you can not think about it and thus
> > more likely get it wrong.
> > 
> > I can just see a driver not implementing prepare at all and then doing
> > stupid things in commit when they hit some resource limit or whatever,
> > rather than taking care of such issues in prepare.
> 
> OK, I have no experience with stacked devices nor what it actually looks
> like, but I understand that it is a redundant setup where it makes sense
> to ensure that an operation is feasible before programming the hardware.
> 
> I agree with both of you on imposing switchdev drivers such notion.
> 
> I was confused with the rtnl lock (from bridge netlink requests) which
> seemed to limit a lot the usage of this prepare phase.
> 
> I don't know the batch mode neither, but I can think about a potentially
> powerful usage of the prepare phase in Marvell switches (or any basic
> home router switches), please tell me if the following is feasible:
> 
> Every hardware VLANs I know of are programmed with all port membership
> in one shot. This is not feasible today with the bridge command. If I
> could bundle in one request the equivalent of ("VID 100: 0u 1u 5t"):
> 
>     bridge vlan add master dev swp0 vid 100 pvid untagged
>     bridge vlan add master dev swp1 vid 100 pvid untagged
>     bridge vlan add master dev swp5 vid 100 # cpu
> 
> In such case the prepare phase could be great to allocate and populate a
> VLAN entry structure (i.e. struct mv88e6xxx_vtu_stu_entry) before
> programming the hardware *just once*. Is that doable?

May I get answers for this? I'd need that in order to suggest a next
step for the prepare phase in DSA drivers.

Thanks,
-v

  reply	other threads:[~2015-09-30 18:56 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-24  8:02 [patch net-next v3 00/10] switchdev: transaction item queue and cleanup Jiri Pirko
2015-09-24  8:02 ` [patch net-next v3 01/10] switchdev: rename "trans" to "trans_ph" Jiri Pirko
2015-09-24  8:02 ` [patch net-next v3 02/10] switchdev: introduce transaction item queue for attr_set and obj_add Jiri Pirko
2015-09-25  4:36   ` Vivien Didelot
2015-09-25  5:29     ` Scott Feldman
2015-09-25  5:55       ` David Miller
2015-09-25 15:03         ` Vivien Didelot
2015-09-30 18:56           ` Vivien Didelot [this message]
2015-10-01  4:27             ` Scott Feldman
2015-10-01 15:15               ` Vivien Didelot
2015-10-01 15:28                 ` Andrew Lunn
2015-10-01 16:17                   ` Vivien Didelot
2015-10-01 16:26                     ` Andrew Lunn
2015-10-01 17:11                       ` Vivien Didelot
2015-09-24  8:02 ` [patch net-next v3 03/10] switchdev: move transaction phase enum under transaction structure Jiri Pirko
2015-09-24  8:02 ` [patch net-next v3 04/10] switchdev: add switchdev_trans_ph_prepare/commit helpers Jiri Pirko
2015-09-24  8:02 ` [patch net-next v3 05/10] rocker: push struct switchdev_trans down through rocker code Jiri Pirko
2015-09-24  8:02 ` [patch net-next v3 06/10] rocker: use switchdev transaction queue for allocated memory Jiri Pirko
2015-09-24  8:02 ` [patch net-next v3 07/10] switchdev: remove "NONE" transaction phase Jiri Pirko
2015-09-24  8:02 ` [patch net-next v3 08/10] switchdev: remove "ABORT" " Jiri Pirko
2015-09-24  8:02 ` [patch net-next v3 09/10] dsa: use prepare/commit switchdev transaction helpers Jiri Pirko
2015-09-24  8:02 ` [patch net-next v3 10/10] switchdev: reduce transaction phase enum down to a boolean Jiri Pirko
2015-09-25  3:05 ` [patch net-next v3 00/10] switchdev: transaction item queue and cleanup Scott Feldman
2015-09-25  6:00   ` David Miller
2015-09-25  6:04     ` Jiri Pirko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150930185652.GA2455@ketchup.lan \
    --to=vivien.didelot@savoirfairelinux.com \
    --cc=andrew@lunn.ch \
    --cc=eladr@mellanox.com \
    --cc=f.fainelli@gmail.com \
    --cc=gospo@cumulusnetworks.com \
    --cc=idosch@mellanox.com \
    --cc=jiri@mellanox.com \
    --cc=jiri@resnulli.us \
    --cc=linux@roeck-us.net \
    --cc=netdev@vger.kernel.org \
    --cc=pjonnala@broadcom.com \
    --cc=rami.rosen@intel.com \
    --cc=roopa@cumulusnetworks.com \
    --cc=sfeldma@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).