Re: [PATCH net-next 1/2] cxgb4/cxgb4vf: Add support for ndo_set_vf_vlan

[Hariprasad Shenai <hariprasad@chelsio.com>]

On Wednesday, August 08/24/16, 2016 at 08:31:58 +0000, Yuval Mintz wrote:
> > > > @@ -1202,6 +1202,10 @@ int t4vf_eth_xmit(struct sk_buff *skb, struct
> > > > net_device *dev)
> > > >  	BUG_ON(qidx >= pi->nqsets);
> > > >  	txq = &adapter->sge.ethtxq[pi->first_qset + qidx];
> > > >
> > > > +	if (pi->vlan_id && !skb_vlan_tag_present(skb))
> > > > +		__vlan_hwaccel_put_tag(skb, cpu_to_be16(ETH_P_8021Q),
> > > > +				       pi->vlan_id);
> > > > +
> > >
> > > So it's a purely SW implementation of the feature on the VF side?
> > > Does the HW enforces the configuration in any way on the VF?
> > Basically the PF driver passes the VLAN ID it got through ndo_set_vf_vlan to the
> > VF driver. And then the VF driver reads it and requests hardware to tag it.
> 
> Problem with SW implementations is mainly that they have no effect over
> Malicious VFs
>  I.e., if the purpose here is to add the VF to some vlan-tagged subnet
> Whereas the user is oblivious to it, a malicious user can easily modify
> the driver to ignore this restriction and get access to the entire network.
> 
> I think one of the problems with this ndo is that it's poorly documented
> and thus open for various interpretations - so it's debatable what's important
> and what's not. [If it is properly documented anywhere, please educate me]
I agree with you. Even I coudn't find a proper documentation for the same.
I never thought about security issuses, (i.e., user modifying the VF driver
to gain access over the network) while implementing this.

> > > Also, looks like an already tagged packet would be processed with the
> > > original vlan-id [instead of the one of PF has provided].
> > > Is that intentional?
> > No, this isn't intentional. I thought VST and VGT cannot co-exist.
> > What should be the behavior?
> 
> Are you preventing VGT configuration once VST is configured?
> If not, what to prevent VM user from configuring vlan interfaces
> on top of the VF, even if VST is configured?
Again this misses documentation, what if VLAN interface is already configured in
VM before VST is configured. 
Before there were callbacks to add/remove vlan interface, now that is removed how
to achieve it?
    OR 
am I missing something?