Re: [dpdk-dev] [PATCH v3] net/tap: perform proto field update for tun only

["Varghese, Vipin" <vipin.varghese@intel.com>]

Hi Ophir,

Thanks for inputs, please find my answers inline to the mail

> -----Original Message-----
> From: Ophir Munk [mailto:ophirmu@mellanox.com]
> Sent: Tuesday, May 22, 2018 8:35 PM
> To: Varghese, Vipin <vipin.varghese@intel.com>; dev@dpdk.org; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Wiles, Keith <keith.wiles@intel.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; Olga Shern
> <olgas@mellanox.com>
> Subject: RE: [PATCH v3] net/tap: perform proto field update for tun only
> 
> Hi,
> Overall it looks good.
> Please note a few more comments below.
> 
> > -----Original Message-----
> > From: Vipin Varghese [mailto:vipin.varghese@intel.com]
> > Sent: Tuesday, May 22, 2018 1:47 PM
> > To: dev@dpdk.org; ferruh.yigit@intel.com; keith.wiles@intel.com; Ophir
> > Munk <ophirmu@mellanox.com>
> > Cc: Thomas Monjalon <thomas@monjalon.net>; Olga Shern
> > <olgas@mellanox.com>; Vipin Varghese <vipin.varghese@intel.com>
> > Subject: [PATCH v3] net/tap: perform proto field update for tun only
> >
> > The TX function is shared between TAP and TUN PMD. Checking TUN-TAP
> > type field will ensure the TAP PMD will always have protocol field as 0.
> >
> > Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> > Suggested-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> 
> Shouldn't it be a fix commit?

The following code base is not merged with stable, dev or next-net branch. Hence do we 'fix commit' this?

> If so - please update the commit title, add Fixes: ... , Cc: stable@dpdk.org...
> 
> > Changes in V3:
> > - remove type field from rx struct - Ophir Munk
> > - add space for comment in struct - Ophir Munk
> > - pass type field into eth_dev_tap_create - Ophir Munk
> > - replace with enum value for type - Ophir Munk
> > - return as 'not supported' for mac_set - Vipin Varghese
> >
> > Changes in V2:
> > - updated in comment wording - Keith Wiles
> > - remove debug print from tx code - Keith Wiles
> > ---
> >  drivers/net/tap/rte_eth_tap.c | 61
> > +++++++++++++++++++++++++-------------
> > -----
> >  drivers/net/tap/rte_eth_tap.h |  9 +++++++
> >  2 files changed, 45 insertions(+), 25 deletions(-)
> >

<Snipped>

> >
> > +	if (pmd->type == ETH_TUNTAP_TYPE_TUN)
> > +		return -ENOTSUP;
> 
> 
> Can you please add a TAP_LOG(ERR, ...) to log this error similar to other error
> cases in this function?

Yes, I will add the change in v4 and share the same.

> 
> > +

<Snipped>