[dpdk-dev,2/2] virtio: allow running w/o vlan filtering

  Back on this old patch, it seems justified but nobody agreed.

> "Ouyang, Changchun" <changchun.ouyang@intel.com> wrote:
> > > From: Stephen Hemminger
> > > Vlan filtering is an option, and not a requirement.
> > > If host does not support filtering then it can be done in software.
> > 
> > The question is that guest only send command, no real action to do the vlan filter. 
> > So if both host and guest have no real action for vlan filter, who will do it? 
> 
> The virtio driver has features.
> Guest can not send commands to host where feature bit not enabled.
> Application can call filter_set and check if filter worked or not.
> 
> Our code already had to do MAC and VLAN validation of incoming packets
> therefore if hardware can't do vlan match, there is no problem.
> I would expect other applications would do the same thing.
> 
> Failing during configuration is bad. DPDK API should never force
> application to play "guess the working configuration" with the device
> driver or do string match on "which device is this anyway"
  

I have comments for that.
Pls see below.

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, July 29, 2015 8:57 PM
> To: Ouyang, Changchun
> Cc: dev@dpdk.org; Stephen Hemminger
> Subject: Re: [dpdk-dev] [PATCH 2/2] virtio: allow running w/o vlan filtering
> 
> Back on this old patch, it seems justified but nobody agreed.
> 
> --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> @@ -1288,7 +1288,6 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>             && !vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN)) {
>                 PMD_DRV_LOG(NOTICE,
>                             "vlan filtering not available on this host");
> -               return -ENOTSUP;
>         }
> 
> 2015-03-06 08:24, Stephen Hemminger:
> > "Ouyang, Changchun" <changchun.ouyang@intel.com> wrote:
> > > > From: Stephen Hemminger
> > > > Vlan filtering is an option, and not a requirement.
> > > > If host does not support filtering then it can be done in software.

Yes, vlan filter is an option, but currently virtio driver has no software solution for vlan filter.
So I would like to disable hw_vlan_filter in rxmode if the dev can't really support it rather than removing the return there.

> > >
> > > The question is that guest only send command, no real action to do the
> vlan filter.
> > > So if both host and guest have no real action for vlan filter, who will do it?
> >
> > The virtio driver has features.
> > Guest can not send commands to host where feature bit not enabled.
> > Application can call filter_set and check if filter worked or not.
> >
> > Our code already had to do MAC and VLAN validation of incoming packets

There is vlan strip, but have no vlan filter in the rx function.

> > therefore if hardware can't do vlan match, there is no problem.
> > I would expect other applications would do the same thing.
> >
> > Failing during configuration is bad. DPDK API should never force
> > application to play "guess the working configuration" with the device
> > driver or do string match on "which device is this anyway"
  

Thomas, Changchun,

On 29/07/2015 14:56, Thomas Monjalon wrote:
> Back on this old patch, it seems justified but nobody agreed.
>
> --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> @@ -1288,7 +1288,6 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>              && !vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN)) {
>                  PMD_DRV_LOG(NOTICE,
>                              "vlan filtering not available on this host");
> -               return -ENOTSUP;
>          }
>
> 2015-03-06 08:24, Stephen Hemminger:
>> "Ouyang, Changchun" <changchun.ouyang@intel.com> wrote:
>>>> From: Stephen Hemminger
>>>> Vlan filtering is an option, and not a requirement.
>>>> If host does not support filtering then it can be done in software.

+1 with Stephen, remove return -ENOTSUP;

applications must not fail, software stacks will handle it. We did 
experiment some issues when testpmd was failing while it was supposed to 
run. A notice would be good enough.


>>>
>>> The question is that guest only send command, no real action to do the vlan filter.
>>> So if both host and guest have no real action for vlan filter, who will do it?
>>
>> The virtio driver has features.
>> Guest can not send commands to host where feature bit not enabled.
>> Application can call filter_set and check if filter worked or not.
>>
>> Our code already had to do MAC and VLAN validation of incoming packets
>> therefore if hardware can't do vlan match, there is no problem.
>> I would expect other applications would do the same thing.
>>
>> Failing during configuration is bad. DPDK API should never force
>> application to play "guess the working configuration" with the device
>> driver or do string match on "which device is this anyway"

Agree, it is not a failure of a configuration, it is a failure of 
negotiation of virtio's capabilities.

Let's use another example: we do not expect a guest kernel to panic() 
because it is not properly negotiated? So why should a DPDK application 
fail and return -ENOTSUP?

Thank you,
   Vincent
  

Hi Vincent,

> -----Original Message-----
> From: Vincent JARDIN [mailto:vincent.jardin@6wind.com]
> Sent: Tuesday, August 4, 2015 8:52 PM
> To: Thomas Monjalon; Ouyang, Changchun
> Cc: dev@dpdk.org; Stephen Hemminger
> Subject: Re: [dpdk-dev] [PATCH 2/2] virtio: allow running w/o vlan filtering
> 
> Thomas, Changchun,
> 
> On 29/07/2015 14:56, Thomas Monjalon wrote:
> > Back on this old patch, it seems justified but nobody agreed.
> >
> > --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> > +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> > @@ -1288,7 +1288,6 @@ virtio_dev_configure(struct rte_eth_dev *dev)
> >              && !vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN)) {
> >                  PMD_DRV_LOG(NOTICE,
> >                              "vlan filtering not available on this host");
> > -               return -ENOTSUP;
> >          }
> >
> > 2015-03-06 08:24, Stephen Hemminger:
> >> "Ouyang, Changchun" <changchun.ouyang@intel.com> wrote:
> >>>> From: Stephen Hemminger
> >>>> Vlan filtering is an option, and not a requirement.
> >>>> If host does not support filtering then it can be done in software.
> 
> +1 with Stephen, remove return -ENOTSUP;
> 
> applications must not fail, software stacks will handle it. We did experiment
Do you mean handling it in software stack outside the virtio pmd?
AFAIK, inside virtio PMD, we have no codes to handle it currently.

> some issues when testpmd was failing while it was supposed to run. A notice
> would be good enough.
> 

Use '--disable-hw-vlan-filter'  in testpmd command line will allow it continue to work. 
You can have a try.

> 
> >>>
> >>> The question is that guest only send command, no real action to do the
> vlan filter.
> >>> So if both host and guest have no real action for vlan filter, who will do it?
> >>
> >> The virtio driver has features.
> >> Guest can not send commands to host where feature bit not enabled.
> >> Application can call filter_set and check if filter worked or not.
> >>
> >> Our code already had to do MAC and VLAN validation of incoming
> >> packets therefore if hardware can't do vlan match, there is no problem.
> >> I would expect other applications would do the same thing.
> >>
> >> Failing during configuration is bad. DPDK API should never force
> >> application to play "guess the working configuration" with the device
> >> driver or do string match on "which device is this anyway"
> 
> Agree, it is not a failure of a configuration, it is a failure of negotiation of
> virtio's capabilities.

I am not sure which one is better when app configures one feature but fail to negotiate it with host(which means has
no such capability to support this feature currently).
1)The driver cheat the app, and continue to do the rest of work(of course need some hints).
2)give hints and exit, then user re-run app with correct configuration.

> 
> Let's use another example: we do not expect a guest kernel to panic()
> because it is not properly negotiated? So why should a DPDK application fail
> and return -ENOTSUP?
I think user mode driver/app and kernel is different thing :-)

Changchun
  

In Linux and BSD, if driver does not support filtering, the kernel takes
care of the situation.
A DPDK application will need a device layer; maybe some part of the
port/pipeline would
be a good place for it.

On Tue, Aug 4, 2015 at 6:01 PM, Ouyang, Changchun <
changchun.ouyang@intel.com> wrote:

>
> Hi Vincent,
>
> > -----Original Message-----
> > From: Vincent JARDIN [mailto:vincent.jardin@6wind.com]
> > Sent: Tuesday, August 4, 2015 8:52 PM
> > To: Thomas Monjalon; Ouyang, Changchun
> > Cc: dev@dpdk.org; Stephen Hemminger
> > Subject: Re: [dpdk-dev] [PATCH 2/2] virtio: allow running w/o vlan
> filtering
> >
> > Thomas, Changchun,
> >
> > On 29/07/2015 14:56, Thomas Monjalon wrote:
> > > Back on this old patch, it seems justified but nobody agreed.
> > >
> > > --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> > > +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> > > @@ -1288,7 +1288,6 @@ virtio_dev_configure(struct rte_eth_dev *dev)
> > >              && !vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN)) {
> > >                  PMD_DRV_LOG(NOTICE,
> > >                              "vlan filtering not available on this
> host");
> > > -               return -ENOTSUP;
> > >          }
> > >
> > > 2015-03-06 08:24, Stephen Hemminger:
> > >> "Ouyang, Changchun" <changchun.ouyang@intel.com> wrote:
> > >>>> From: Stephen Hemminger
> > >>>> Vlan filtering is an option, and not a requirement.
> > >>>> If host does not support filtering then it can be done in software.
> >
> > +1 with Stephen, remove return -ENOTSUP;
> >
> > applications must not fail, software stacks will handle it. We did
> experiment
> Do you mean handling it in software stack outside the virtio pmd?
> AFAIK, inside virtio PMD, we have no codes to handle it currently.
>
> > some issues when testpmd was failing while it was supposed to run. A
> notice
> > would be good enough.
> >
>
> Use '--disable-hw-vlan-filter'  in testpmd command line will allow it
> continue to work.
> You can have a try.
>
> >
> > >>>
> > >>> The question is that guest only send command, no real action to do
> the
> > vlan filter.
> > >>> So if both host and guest have no real action for vlan filter, who
> will do it?
> > >>
> > >> The virtio driver has features.
> > >> Guest can not send commands to host where feature bit not enabled.
> > >> Application can call filter_set and check if filter worked or not.
> > >>
> > >> Our code already had to do MAC and VLAN validation of incoming
> > >> packets therefore if hardware can't do vlan match, there is no
> problem.
> > >> I would expect other applications would do the same thing.
> > >>
> > >> Failing during configuration is bad. DPDK API should never force
> > >> application to play "guess the working configuration" with the device
> > >> driver or do string match on "which device is this anyway"
> >
> > Agree, it is not a failure of a configuration, it is a failure of
> negotiation of
> > virtio's capabilities.
>
> I am not sure which one is better when app configures one feature but fail
> to negotiate it with host(which means has
> no such capability to support this feature currently).
> 1)The driver cheat the app, and continue to do the rest of work(of course
> need some hints).
> 2)give hints and exit, then user re-run app with correct configuration.
>
> >
> > Let's use another example: we do not expect a guest kernel to panic()
> > because it is not properly negotiated? So why should a DPDK application
> fail
> > and return -ENOTSUP?
> I think user mode driver/app and kernel is different thing :-)
>
> Changchun
>
>
  

> Use '--disable-hw-vlan-filter'  in testpmd command line will allow it continue to work.
> You can have a try.

Yes, but not using this flag should not imply to exit.

> I am not sure which one is better when app configures one feature but fail to negotiate it with host(which means has
> no such capability to support this feature currently).
> 1)The driver cheat the app, and continue to do the rest of work(of course need some hints).
> 2)give hints and exit, then user re-run app with correct configuration.

Same as Stephen:

3) give hints of capabilities, do not exit, then user app does whatever 
it wants (including exit if needed).

Thank you,
  

2015-08-05 12:49, Vincent JARDIN:
> > Use '--disable-hw-vlan-filter'  in testpmd command line will allow it continue to work.
> > You can have a try.
> 
> Yes, but not using this flag should not imply to exit.
> 
> > I am not sure which one is better when app configures one feature but fail to negotiate it with host(which means has
> > no such capability to support this feature currently).
> > 1)The driver cheat the app, and continue to do the rest of work(of course need some hints).
> > 2)give hints and exit, then user re-run app with correct configuration.
> 
> Same as Stephen:
> 
> 3) give hints of capabilities, do not exit, then user app does whatever 
> it wants (including exit if needed).

I would tend to apply this patch but as it was discussed we need some
clear acknowledgement.
Huawei?
  

This is a very old thread which has no conclusion.
Now that we have more experience with virtio, could we try to fix it properly?
	http://dpdk.org/patch/3897/


2015-10-21 15:58, Thomas Monjalon:
> 2015-08-05 12:49, Vincent JARDIN:
> > > Use '--disable-hw-vlan-filter'  in testpmd command line will allow it continue to work.
> > > You can have a try.
> > 
> > Yes, but not using this flag should not imply to exit.
> > 
> > > I am not sure which one is better when app configures one feature but fail to negotiate it with host(which means has
> > > no such capability to support this feature currently).
> > > 1)The driver cheat the app, and continue to do the rest of work(of course need some hints).
> > > 2)give hints and exit, then user re-run app with correct configuration.
> > 
> > Same as Stephen:
> > 
> > 3) give hints of capabilities, do not exit, then user app does whatever 
> > it wants (including exit if needed).
> 
> I would tend to apply this patch but as it was discussed we need some
> clear acknowledgement.
> Huawei?