Date:	Sat, 16 May 2015 04:56:15 -0700
From:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
To:	Hiroshi Shimamoto <h-shimamoto@...jp.nec.com>
Cc:	"Skidmore, Donald C" <donald.c.skidmore@...el.com>,
	Or Gerlitz <gerlitz.or@...il.com>,
	David Miller <davem@...emloft.net>,
	Linux Netdev List <netdev@...r.kernel.org>,
	"nhorman@...hat.com" <nhorman@...hat.com>,
	"sassmann@...hat.com" <sassmann@...hat.com>,
	"jogreene@...hat.com" <jogreene@...hat.com>,
	"Choi, Sy Jong" <sy.jong.choi@...el.com>,
	Edward Cree <ecree@...arflare.com>,
	Rony Efraim <ronye@...lanox.com>
Subject: Re: [net-next 07/11] if_link: Add VF multicast promiscuous control

On Tue, 2015-05-12 at 00:33 +0000, Hiroshi Shimamoto wrote:
> > > -----Original Message-----
> > > From: Hiroshi Shimamoto [mailto:h-shimamoto@...jp.nec.com]
> > > Sent: Monday, May 11, 2015 4:55 PM
> > > To: Skidmore, Donald C; Or Gerlitz; Kirsher, Jeffrey T
> > > Cc: David Miller; Linux Netdev List; nhorman@...hat.com;
> > > sassmann@...hat.com; jogreene@...hat.com; Choi, Sy Jong; Edward
> Cree;
> > > Rony Efraim
> > > Subject: RE: [net-next 07/11] if_link: Add VF multicast
> promiscuous control
> > >
> > > > > > -----Original Message-----
> > > > > > From: Hiroshi Shimamoto [mailto:h-shimamoto@...jp.nec.com]
> > > > > > Sent: Wednesday, May 06, 2015 10:55 PM
> > > > > > To: Skidmore, Donald C; Or Gerlitz; Kirsher, Jeffrey T
> > > > > > Cc: David Miller; Linux Netdev List; nhorman@...hat.com;
> > > > > > sassmann@...hat.com; jogreene@...hat.com; Choi, Sy Jong;
> Edward
> > > > > > Cree; Rony Efraim
> > > > > > Subject: RE: [net-next 07/11] if_link: Add VF multicast
> > > > > > promiscuous control
> > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Or Gerlitz [mailto:gerlitz.or@...il.com]
> > > > > > > > Sent: Sunday, May 03, 2015 7:16 AM
> > > > > > > > To: Kirsher, Jeffrey T
> > > > > > > > Cc: David Miller; Hiroshi Shimamoto; Linux Netdev List;
> > > > > > > > nhorman@...hat.com; sassmann@...hat.com;
> > > jogreene@...hat.com;
> > > > > > Choi,
> > > > > > > > Sy Jong; Edward Cree; Skidmore, Donald C; Rony Efraim
> > > > > > > > Subject: Re: [net-next 07/11] if_link: Add VF multicast
> > > > > > > > promiscuous control
> > > > > > > >
> > > > > > > > On Sat, May 2, 2015 at 1:42 PM, Jeff Kirsher
> > > > > > > > <jeffrey.t.kirsher@...el.com>
> > > > > > > > wrote:
> > > > > > > > > From: Hiroshi Shimamoto <h-shimamoto@...jp.nec.com>
> > > > > > > > >
> > > > > > > > > Add netlink directives and ndo entry to allow VF
> multicast
> > > > > > > > > promiscuous
> > > > > > > > mode.
> > > > > > > > >
> > > > > > > > > This controls the permission to enter VF multicast
> promiscuous
> > > mode.
> > > > > > > > > The administrator will dedicatedly allow multicast
> promiscuous per
> > > VF.
> > > > > > > > >
> > > > > > > > > When the VF is under multicast promiscuous mode, all
> > > > > > > > > multicast packets are sent to the VF.
> > > > > > > > >
> > > > > > > > > Don't allow VF multicast promiscuous if the VM isn't
> fully trusted.
> > > > > > > >
> > > > > > > > Guys,
> > > > > > > >
> > > > > > > > I don't think the discussion we held in the past [1] on
> the
> > > > > > > > matter actually converged. Few open points that came up
> while
> > > > > > > > debating it internally with
> > > > > > > > Rony:
> > > > > > > >
> > > > > > > > 1. maybe what we we actually want here an API that
> states a VF
> > > > > > > > to be privileged/trusted and then we can over load the
> feature
> > > > > > > > set of being
> > > > > > such?
> > > > > > >
> > > > > > > I suggested this originally, but there was push back as it
> was
> > > > > > > thought too
> > > > > > generic as the definition of what being a "trusted"
> > > > > > > vendor would differ from driver to driver.  Personally I
> still
> > > > > > > like the idea of
> > > > > > having one mode saying that we "trust"
> > > > > > > a given VF.  Then that VF can request whatever it support
> it
> > > > > > > wants from the PF regardless of possible negative impact
> on other
> > > VF's.
> > > > > > > What is possible to support would then be left to the
> interface
> > > > > > > between the VF and PF.  This of course would be dependent
> on
> > > > > > > what the
> > > > > > given HW could support and would mean this mode would mean
> > > > > > different things for different adapters and I do see why
> some
> > > > > > might see this as a concern.
> > > > > >
> > > > > > The point is granularity, right?
> > > > > > Allow everything or allow subset of features.
> > > > >
> > > > > Nice way to sum it up.  The trick being with the subset of
> features path is
> > > not all hardware can/will support everything.
> > > > > Also I worry about worry about the feature list growing
> requiring
> > > > > more and more nobs on the PF to allow/disallow granular
> behavior
> > > > > that could brake VF isolation.  With a simple hint to the PF
> that a given VF
> > > is "trusted" would allow all that complexity to be contained in
> the mailbox
> > > protocol between the PF/VF.
> > > > >
> > > > > All that said I realize others are concerned with the
> ambiguousness
> > > > > of such a field and can certainly live with your
> implementation.
> > > >
> > > > I see, it seems better to have a single knob which indicates
> "trust
> > > > this VF" and PF will allow requests which might hurt performance
> or
> > > > security from trusted VF, instead of creating a knob for
> multicast
> > > promiscuous, a knob for feature X and so on.
> > > >
> > > > I will make a patch to implement that "trusted knob" instead of
> allowing
> > > MC promiscuous.
> > > > Is there any comment?
> > >
> > > Any comments?
> > > Is that the way to go ahead this series?
> > >
> > > thanks,
> > > Hiroshi
> > 
> > Clearly I am ok with the idea.  :)
> > 
> > If the change isn't too difficult to implement, maybe just submit
> the path.  That will most likely get more attention.
> 
> okay, I'll submit a new patches.
> BTW, which tree should I make patches against for?
> Because Jeff's tree still have previous patches.

So sorry for the delay, been dealing with some health issues.
I have removed your previous series of patches from my next-queue tree,
so you can re-spin your patches based on the dev-queue branch.

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)

Powered by blists - more mailing lists