[dpdk-dev] [PATCH 1/2] net/tap: add tun support

Varghese, Vipin vipin.varghese at intel.com
Mon Apr 23 14:58:47 CEST 2018


Hi Ophir,

Can you help me with the investigation with the following information?
1) The kernel or distro in which the TAP proto flag set breaks the logic?
2) Is the above still valid even after applying the patch ' https://dpdk.org/dev/patchwork/patch/37986/'?

Note: I am testing with 3.13.0, 4.4.0 and 4.13.0.

Thanks
Vipin Varghese

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Varghese, Vipin
> Sent: Saturday, April 21, 2018 8:40 PM
> To: Ophir Munk <ophirmu at mellanox.com>; dev at dpdk.org;
> pascal.mazon at 6wind.com; Yigit, Ferruh <ferruh.yigit at intel.com>; Thomas
> Monjalon <thomas at monjalon.net>; Olga Shern <olgas at mellanox.com>;
> Shahaf Shuler <shahafs at mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH 1/2] net/tap: add tun support
> 
> Hi Ophir,
> 
> <Snip>
> 
> > Hi Vipin,
> > I missed your point:
> > You claim that TAP should work regardless of any pi.proto values.
> > Can you confirm that for ALL kernels versions (past and future)?
> 
> I have tested with 3.13.0 , 4.4.0 with patch fix.
> 
> >
> > > -----Original Message-----
> > > From: Ophir Munk
> > > Sent: Saturday, April 21, 2018 12:49 AM
> > > To: Varghese, Vipin <vipin.varghese at intel.com>; dev at dpdk.org;
> > > pascal.mazon at 6wind.com; Yigit, Ferruh <ferruh.yigit at intel.com>;
> > > Thomas Monjalon <thomas at monjalon.net>; Olga Shern
> > > <olgas at mellanox.com>; Shahaf Shuler <shahafs at mellanox.com>
> > > Subject: RE: [dpdk-dev] [PATCH 1/2] net/tap: add tun support
> > >
> > > Hi Vipin,
> > >
> > > Please find comments inline.
> > >
> > > > -----Original Message-----
> > > > From: Varghese, Vipin [mailto:vipin.varghese at intel.com]
> > > > Sent: Friday, April 13, 2018 6:18 AM
> > > > To: Ophir Munk <ophirmu at mellanox.com>; dev at dpdk.org;
> > > > pascal.mazon at 6wind.com; Yigit, Ferruh <ferruh.yigit at intel.com>;
> > > > Thomas Monjalon <thomas at monjalon.net>; Olga Shern
> > > > <olgas at mellanox.com>; Shahaf Shuler <shahafs at mellanox.com>
> > > > Subject: RE: [dpdk-dev] [PATCH 1/2] net/tap: add tun support
> > > >
> 
> <Snip>
> 
> > > > > 1. Accessing the first byte here assumes it is the first IP
> > > > > header byte (layer 3) which is correct for TUN.
> > > > > For TAP however the first byte belongs to Ethernet destination
> > > > > address (layer 2).
> > > > > Please explain how this logic will work for TAP.
> > > >
> > > > Based on linux code base '/driver/net/tap.c' and '/driver/net/tun.c'
> > > > from 3.13. to  4.16,
> > > >
> > > > Please find my observation below
> > > > 1. File: tun.c, function: tun_get_user, check for 'tun->flags &
> > > > TUN_TYPE_MASK' is done and if non ip is taken counter 'rx_dropped'
> > > > is updated.
> > > > 2. File: tap.c, there are no checks for 'tap->flags' for IFF_NO_PI
> > > > in rx data path. Counter 'rx_dropped' is updated in 'tap_handle_frame'.
> > > >
> > >
> > > I understand that in kernel implementation there is no check for
> > > tap->flags in file tap.c, however I think there is a bug in dpdk
> > > tap->rte_eth_tap.c
> > file.
> > > Please find below an example which demonstrates this claim.
> > >
> > > > Please find my reasoning below
> > > > 1. First approach was to have separate function for tap and tun TX and
> RX.
> > > > But this will introduce code duplication, hence reworked the code
> > > > as
> > > above.
> > >
> > > I agree. Avoiding code duplication is a good approach.
> > >
> > > > 2. During my internal testing assigning dummy value for protocol
> > > > field in TAP packets, did not show a difference in behaviour. May
> > > > be there are some specific cases this failing.
> > > >
> > > > If there difference in behaviour, can please share the same?
> > > >
> > >
> > > Please consider the following example:
> > > I am running testpmd with a TAP device, --forward-mode=csum.
> > > I am injecting a TCP packet, which is forwarded back (mac addresses
> > > swapped) to the sender.
> > > Using gdb I set a breakpoint at pmd_tx_burst() in file rte_eth_tap.c
> > >
> > > Looking at the following code inside pmd_tx_burst():
> > >
> > > 527                 char *buff_data = rte_pktmbuf_mtod(seg, void *);
> > > 528                 j = (*buff_data & 0xf0);
> > > 529                 pi.proto = (j == 0x40) ? 0x0008 :
> > > 530                                 (j == 0x60) ? 0xdd86 : 0x00;
> > >
> > > I am printing the first 20 bytes of buff_data in line 527:
> > >
> > > (gdb) p/x *(unsigned char *)buff_data at 20
> > > $3 = {0x0, 0x25, 0x88, 0x10, 0x66, 0x2, 0xf4, 0x52, 0x14, 0x7a,
> > > 0x59, 0x81, 0x8, 0x0, 0x45, 0x0, 0x4, 0xdf, 0x0, 0x1}
> > >
> > > The gdb printout refers to:
> > > 6 bytes of destination MAC address: 0x0, 0x25, 0x88, 0x10, 0x66, 0x2
> > > 6 bytes of source MAC address: 0xf4, 0x52, 0x14, 0x7a, 0x59, 0x81
> > > 2 bytes of Ethernet type: 0x8, 0x0 - (IPv4) IP header starting with 0x45, ...
> > > which is the byte (0x45) that "j" should have looked at
> > >
> > > In the case of TAP - buff_data starts with the destination MAC
> > > address of the sender (0x0, ...).
> > > The code in line 528 expects that buff_data would start with an IP
> > > header protocol (e.g. 0x45), but it is not the case for TAP.
> > > In my case j=0x0 (line 528) which is harmless (as it ends up with
> > > setting pi.proto=0x00, which is correct for TAP).
> > > However, if the sender had an Intel NIC - the destination MAC
> > > address could have started with:
> > > $3 = {0x40, 0x25, 0xC2, ...
> > > Or-
> > > $3 = {0x64, 0xD4, 0xDA, ...
> > >
> > > as 4025C2 and 64D4DA are reserved prefixes for Intel Ethernet MAC
> > > addresses, see: http://www.coffer.com/mac_find/?string=intel
> > >
> > > In this case pi.proto could end up with 0x0008 or 0xdd86 instead of
> > > 0x0 as expected for TAP.
> > >
> > > I hope that this example clarifies the bug I am referring to.
> > >
> 
> Thanks for sharing detailed example overview. But as you mentioned this will
> break ' 4025C2' and ' 64D4DA', This will not solve for the correction patch  '
> https://dpdk.org/dev/patchwork/patch/37986/'.
> 
> Only choice left is separate tx_burst for TAP and TUN PMD, as we do not
> want to check PMD type on each call.
> 
> Questions:
> 1) Is this ok to split tx_burst and have redundant code?
> 2) Does applications transparently send packets coming from Physical NIC to
> TAP interface? Does not the application Modifies the DEST MAC addr to TAP
> interface?
> 
> > > > >
> > > > > 2. If the first TUN byte contains 0x2X (which is neither IPv4
> > > > > nor
> > > > > IPv6) it will end up by setting ip.proto as 0xdd86.
> > > > > Please explain how this logic will work for non-IP packets in
> > > > > TUN
> > > >
> > > > I see your point. You are correct about this. Thanks for pointing
> > > > out, may I send correction for this as
> > > >
> > > > """
> > > > -		if (j & (0x40 | 0x60))
> > > > -			pi.proto = (j == 0x40) ? 0x0008 : 0xdd86;
> > > > +		pi.proto = (j == 0x40) ? 0x0008 :
> > > > +					(j == 0x60) ? 0xdd86 :
> > > > +					0x00;
> > > > """


More information about the dev mailing list