Re: [net-next 8/8] net/mlx5: Add vf ACL access via tc flower

[Saeed Mahameed <saeedm@mellanox.com>]

On Wed, 2019-11-13 at 12:19 -0800, Jakub Kicinski wrote:
> On Tue, 12 Nov 2019 16:31:19 -0800, Saeed Mahameed wrote:
> > On Tue, Nov 12, 2019 at 3:41 PM Jakub Kicinski wrote:
> > > On Tue, 12 Nov 2019 17:13:53 +0000, Saeed Mahameed wrote:  
> > > > From: Ariel Levkovich <lariel@mellanox.com>
> > > > 
> > > > Implementing vf ACL access via tc flower api to allow
> > > > admins configure the allowed vlan ids on a vf interface.
> > > > 
> > > > To add a vlan id to a vf's ingress/egress ACL table while
> > > > in legacy sriov mode, the implementation intercepts tc flows
> > > > created on the pf device where the flower matching keys include
> > > > the vf's mac address as the src_mac (eswitch ingress) or the
> > > > dst_mac (eswitch egress) while the action is accept.
> > > > 
> > > > In such cases, the mlx5 driver interpets these flows as adding
> > > > a vlan id to the vf's ingress/egress ACL table and updates
> > > > the rules in that table using eswitch ACL configuration api
> > > > that is introduced in a previous patch.  
> > > 
> > > Nack, the magic interpretation of rules installed on the PF is a
> > > no go.  
> > 
> > PF is the eswitch manager it is legit for the PF to forward rules
> > to
> > the eswitch FDB,
> > we do it all over the place, this is how ALL legacy ndos work, why
> > this should be treated differently ?
> 
> It's not a legacy NDO, there's little precedent for it, and you're
> inventing a new meaning for an operation.
> 
> > Anyway just for the record, I don't think you are being fair here,
> > you
> > just come up with rules on the go just to block anything related to
> > legacy mode.
> 
> I tried to block everything related to legacy NDOs for a while now,
> and
> I'm not the only one (/me remembers Or in netdevconf 1.1). I'm sorry
> but
> I won't go and dig out the links now, it's a waste of time.
> 
> Maybe we differ on the definition of fairness. I'm against this
> exactly
> _because_ I'm fair, nobody gets a free pass, no matter how much we
> otherwise appreciate given company contributing to the kernel...

I wasn't looking for free passes, we just disagree on how pf driver
should interpret TC flower in case of legacy sriov, which  was never
defined and no one really cared about it until this patch.

My only concern here is that people will make up their own
rules/interpretation on the go as they see fit to promote their own
agenda, this applies to both of us, this what makes it unfair, we must
go with your rules and interpretations at the end of the day.

Anyway message received, we don't like legacy sriov and every thing
related to it will be handled with an iron fist, I will drop this
patch.