DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 03/12] virtio: reinitialize the device in configure callback
From: Olivier MATZ @ 2016-10-13 13:57 UTC (permalink / raw)
  To: Yuanhan Liu
  Cc: dev, konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang
In-Reply-To: <20161013075415.GS16751@yliu-dev.sh.intel.com>



On 10/13/2016 09:54 AM, Yuanhan Liu wrote:
> On Wed, Oct 12, 2016 at 06:01:25PM +0200, Olivier MATZ wrote:
>> Hello Yuanhan,
>>
>> On 10/12/2016 04:41 PM, Yuanhan Liu wrote:
>>> On Mon, Oct 03, 2016 at 11:00:14AM +0200, Olivier Matz wrote:
>>>> @@ -1344,6 +1347,7 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>>>>   {
>>>>   	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
>>>>   	struct virtio_hw *hw = dev->data->dev_private;
>>>> +	uint64_t req_features;
>>>>   	int ret;
>>>>
>>>>   	PMD_INIT_LOG(DEBUG, "configure");
>>>> @@ -1353,6 +1357,14 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>>>>   		return -EINVAL;
>>>>   	}
>>>>
>>>> +	req_features = VIRTIO_PMD_GUEST_FEATURES;
>>>> +	/* if request features changed, reinit the device */
>>>> +	if (req_features != hw->req_guest_features) {
>>>> +		ret = virtio_init_device(dev, req_features);
>>>> +		if (ret < 0)
>>>> +			return ret;
>>>> +	}
>>>
>>> Why do you have to reset virtio here? This doesn't make too much sense
>>> to me.
>>>
>>> IIUC, you want to make sure those TSO related features being unset at
>>> init time, and enable it (by doing reset) when it's asked to be enabled
>>> (by rte_eth_dev_configure)?
>>>
>>> Why not always setting those features? We could do the actual offloads
>>> when:
>>>
>>> - those features have been negoiated
>>>
>>> - they are enabled through rte_eth_dev_configure
>>>
>>> With that, I think we could avoid the reset here?
>>
>> It would work for TX, since you decide to use or not the feature. But I
>> think this won't work for RX: if you negociate LRO at init, the host may
>> send you large packets, even if LRO is disabled in dev_configure.
>
> I see. Thanks.
>
> Besides, I think you should return error when LRO is not negoiated
> after the reset (say, when it's disabled through qemu command line)?

Good idea, I now return an error if offload cannot be negotiated.

Olivier

^ permalink raw reply

* Re: [PATCH v2 10/12] virtio: add Tx checksum offload support
From: Olivier MATZ @ 2016-10-13 13:58 UTC (permalink / raw)
  To: Yuanhan Liu
  Cc: dev, konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang
In-Reply-To: <20161013083821.GU16751@yliu-dev.sh.intel.com>



On 10/13/2016 10:38 AM, Yuanhan Liu wrote:
> On Mon, Oct 03, 2016 at 11:00:21AM +0200, Olivier Matz wrote:
>> +	/* Checksum Offload */
>> +	switch (cookie->ol_flags & PKT_TX_L4_MASK) {
>> +	case PKT_TX_UDP_CKSUM:
>> +		hdr->csum_start = cookie->l2_len + cookie->l3_len;
>> +		hdr->csum_offset = 6;
>> +		hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
>> +		break;
>> +
>> +	case PKT_TX_TCP_CKSUM:
>> +		hdr->csum_start = cookie->l2_len + cookie->l3_len;
>> +		hdr->csum_offset = 16;
>
> I would suggest to use "offsetof(...)" here, instead of some magic
> number like 16.

Will do, it's actually clearer.

Olivier

^ permalink raw reply

* Re: [PATCH] testpmd: fix tso with csum engine
From: Thomas Monjalon @ 2016-10-13 14:01 UTC (permalink / raw)
  To: Olivier Matz; +Cc: dev, pablo.de.lara.guarch
In-Reply-To: <1476366030-28321-1-git-send-email-olivier.matz@6wind.com>

2016-10-13 15:40, Olivier Matz:
> The commit that disabled tso for small packets was broken during the
> rebase. The problem is the IP checksum is not calculated in software if:
> - TX IP checksum is disabled
> - TSO is enabled
> - the current packet is smaller than tso segment size
> 
> When checking if the PKT_TX_IP_CKSUM flag should be set (in case
> of tso), use the local tso_segsz variable, which is set to 0 when the
> packet is too small to require tso. Therefore the IP checksum will be
> correctly calculated in software.
> 
> Moreover, we should not use tunnel segment size for non-tunnel tso, else
> TSO will stay disabled for all packets.
> 
> Fixes: 97c21329d42b ("app/testpmd: do not use TSO for small packets")
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

Applied, thanks

^ permalink raw reply

* Re: [PATCH v2 12/12] virtio: add Tso support
From: Olivier MATZ @ 2016-10-13 14:02 UTC (permalink / raw)
  To: Yuanhan Liu
  Cc: dev, konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang
In-Reply-To: <20161013081839.GT16751@yliu-dev.sh.intel.com>



On 10/13/2016 10:18 AM, Yuanhan Liu wrote:
> On Mon, Oct 03, 2016 at 11:00:23AM +0200, Olivier Matz wrote:
>> +/* When doing TSO, the IP length is not included in the pseudo header
>> + * checksum of the packet given to the PMD, but for virtio it is
>> + * expected.
>> + */
>> +static void
>> +virtio_tso_fix_cksum(struct rte_mbuf *m)
>> +{
>> +	/* common case: header is not fragmented */
>> +	if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len +
>> +			m->l4_len)) {
> ...
>> +		/* replace it in the packet */
>> +		th->cksum = new_cksum;
>> +	} else {
> ...
>> +		/* replace it in the packet */
>> +		*rte_pktmbuf_mtod_offset(m, uint8_t *,
>> +			m->l2_len + m->l3_len + 16) = new_cksum.u8[0];
>> +		*rte_pktmbuf_mtod_offset(m, uint8_t *,
>> +			m->l2_len + m->l3_len + 17) = new_cksum.u8[1];
>> +	}
>
> The tcp header will always be in the mbuf, right? Otherwise, you can't
> update the cksum field here. What's the point of introducing the "else
> clause" then?

Sorry, I don't see the problem you're pointing out here.

What I want to solve here is to support the cases where the mbuf is 
segmented in the middle of the network header (which is probably a rare 
case).

In the "else" part, I only access the mbuf byte by byte using the 
rte_pktmbuf_mtod_offset() accessor. An alternative would have been to 
copy the header in a linear buffer, fix the checksum, then copy it again 
in the packet, but there is no mbuf helpers to do these copies for now.

Regards,
Olivier

^ permalink raw reply

* Re: [PATCH v2] app/testpmd: fix RSS-hash-key size
From: Thomas Monjalon @ 2016-10-13 14:08 UTC (permalink / raw)
  To: Awal, Mohammad Abdul; +Cc: dev, Ananyev, Konstantin
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0BBE4D@irsmsx105.ger.corp.intel.com>

> > RSS hash-key-size is retrieved from device configuration instead of
> > using a fixed size of 40 bytes.
> > 
> > Fixes: f79959ea1504 ("app/testpmd: allow to configure RSS hash key")
> > 
> > Signed-off-by: Mohammad Abdul Awal <mohammad.abdul.awal@intel.com>
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Applied, thanks

^ permalink raw reply

* [PATCH v3 00/12] net/virtio: add offload support
From: Olivier Matz @ 2016-10-13 14:15 UTC (permalink / raw)
  To: dev, yuanhan.liu
  Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang, maxime.coquelin
In-Reply-To: <1469088510-7552-1-git-send-email-olivier.matz@6wind.com>

This patchset, targetted for 16.11, introduces the support of rx and tx
offload in virtio pmd.  To achieve this, some new mbuf flags must be
introduced, as discussed in [1].

It applies on master + a patch fixing the testpmd csum engine:
http://dpdk.org/dev/patchwork/patch/16538/

The new mbuf checksum flags are backward compatible for current
applications that assume that unknown_csum = good_cum (since there
was only a bad_csum flag). But it the patchset is integrated, we
should consider updating the PMDs to match the new API for 16.11.

[1] http://dpdk.org/ml/archives/dev/2016-May/039920.html

changes v2 -> v3

- fix typo in release note
- add unlikely() in cksum calculation error case
- add likely() in virtio rx function when cksum != 0xffff
- return an error code instead of the cksum in rte_raw_cksum_mbuf()
- do not access to the virtio header if no offload is negotiated (rx and tx)
- return an error if offload cannot be negotiated
- use offsetof() instead of magic hardcoded values for cksum offsets
- changefix some commit titles

changes v1 -> v2
- change mbuf checksum calculation static inline
- fix checksum calculation for protocol where csum=0 means no csum
- move mbuf checksum calculation in librte_net
- use RTE_MIN() to set max rx/tx queue
- rebase on top of head

Olivier Matz (12):
  virtio: move device initialization in a function
  virtio: setup and start cq in configure callback
  virtio: reinitialize the device in configure callback
  net: add function to calculate a checksum in a mbuf
  mbuf: add new Rx checksum mbuf flags
  app/testpmd: fix checksum stats in csum engine
  mbuf: new flag for LRO
  app/testpmd: display lro segment size
  virtio: add Rx checksum offload support
  virtio: add Tx checksum offload support
  virtio: add Lro support
  virtio: add Tso support

 app/test-pmd/csumonly.c                |   8 +-
 doc/guides/rel_notes/release_16_11.rst |  16 ++
 drivers/net/virtio/virtio_ethdev.c     | 197 ++++++++++++++--------
 drivers/net/virtio/virtio_ethdev.h     |  18 +-
 drivers/net/virtio/virtio_pci.h        |   4 +-
 drivers/net/virtio/virtio_rxtx.c       | 298 ++++++++++++++++++++++++++++++---
 drivers/net/virtio/virtqueue.h         |   1 +
 lib/librte_mbuf/rte_mbuf.c             |  18 +-
 lib/librte_mbuf/rte_mbuf.h             |  58 ++++++-
 lib/librte_net/rte_ip.h                |  71 ++++++++
 10 files changed, 580 insertions(+), 109 deletions(-)

Test plan
=========

(replayed on v3)

Platform description
--------------------

  guest (dpdk)
  +----------------+
  |                |
  |                |
  |         port0  +-----<---+
  |       ixgbe /  |         |
  |       directio |         |
  |                |         |
  |    port1       |         ^ flow1
  +----------------+         | (flow2 is the reverse)
         |                   |
         | virtio            |
         v                   |
  +----------------+         |
  |     tap0   /   |         |
  |1.1.1.1   /     |         |
  |ns-tap  /       |         |
  |      /         |         |
  |    /   ixgbe2  +------>--+
  |  /    1.1.1.2  |
  |/      ns-ixgbe |
  +----------------+
  host (linux, vhost-net)


flow1:
  host -(ixgbe)-> guest -(virtio)-> host
  1.1.1.2 -> 1.1.1.1

flow2:
  host -(virtio)-> guest -(ixgbe)-> host
  1.1.1.2 -> 1.1.1.1

Host configuration
------------------

Start qemu with:

- a ne2k management interface to avoi any conflict with dpdk
- 2 ixgbe interfaces given to with vm through vfio
- a virtio net device, connected to a tap interface through vhost-net

  /usr/bin/qemu-system-x86_64 -k fr -daemonize --enable-kvm -m 1G -cpu host \
    -smp 3 -serial telnet::40564,server,nowait -serial null \
    -qmp tcp::44340,server,nowait -monitor telnet::49229,server,nowait \
    -device ne2k_pci,mac=de:ad:de:01:02:03,netdev=user.0,addr=03 \
    -netdev user,id=user.0,hostfwd=tcp::34965-:22 \
    -device vfio-pci,host=0000:04:00.0 -device vfio-pci,host=0000:04:00.1 \
    -netdev type=tap,id=vhostnet0,script=no,vhost=on,queues=8 \
    -device virtio-net-pci,netdev=vhostnet0,ioeventfd=on,mq=on,vectors=17 \
    -hda "/path/to/ubuntu-14.04-template.qcow2" \
    -snapshot -vga none -display none

Move the tap interface in a netns, and configure it:

  ip netns add ns-tap
  ip netns exec ns-tap ip l set lo up
  ip link set tap0 netns ns-tap
  ip netns exec ns-tap ip l set tap0 down
  ip netns exec ns-tap ip l set addr 02:00:00:00:00:01 dev tap0
  ip netns exec ns-tap ip l set tap0 up
  ip netns exec ns-tap ip a a 1.1.1.1/24 dev tap0
  ip netns exec ns-tap arp -s 1.1.1.2 02:00:00:00:00:00
  ip netns exec ns-tap ip a

Move the ixgbe interface in a netns, and configure it:

  IXGBE=ixgbe2
  ip netns add ns-ixgbe
  ip netns exec ns-ixgbe ip l set lo up
  ip link set ${IXGBE} netns ns-ixgbe
  ip netns exec ns-ixgbe ip l set ${IXGBE} down
  ip netns exec ns-ixgbe ip l set addr 02:00:00:00:00:00 dev ${IXGBE}
  ip netns exec ns-ixgbe ip l set ${IXGBE} up
  ip netns exec ns-ixgbe ip a a 1.1.1.2/24 dev ${IXGBE}
  ip netns exec ns-ixgbe arp -s 1.1.1.1 02:00:00:00:00:01
  ip netns exec ns-ixgbe ip a

Guest configuration
-------------------

List of pci devices:

  00:02.0 Ethernet controller [0200]: Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network Connection [8086:10fb] (rev 01)
  00:03.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd. RTL-8029(AS) [10ec:8029]
  00:04.0 Ethernet controller [0200]: Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network Connection [8086:10fb] (rev 01)
  00:05.0 Ethernet controller [0200]: Red Hat, Inc Virtio network device [1af4:1000]

Compile dpdk:

  cd dpdk.org
  make config T=x86_64-native-linuxapp-gcc
  make -j4

Prepare environment:

  mkdir -p /mnt/huge
  mount -t hugetlbfs nodev /mnt/huge
  echo 256 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
  modprobe uio_pci_generic
  python tools/dpdk_nic_bind.py -b uio_pci_generic 0000:00:02.0
  python tools/dpdk_nic_bind.py -b uio_pci_generic 0000:00:05.0

Run test
========

The test uses iperf to validate connectivity between the 2 netns of the
host and trough the guest.

Iperf is run with:

  # flow1: host -(ixgbe)-> guest -(virtio)-> host
  ip netns exec ns-tap iperf -s
  ip netns exec ns-ixgbe iperf -c 1.1.1.1 -t 10

  # flow2: host -(virtio)-> guest -(ixgbe)-> host
  ip netns exec ns-ixgbe iperf -s
  ip netns exec ns-tap iperf -c 1.1.1.2 -t 10

The guest runs testpmd with csum forward engine, its configuration
depends on the test case.

test1: large packets (lro/tso)
------------------------------

Configuration of testpmd:

  ./build/app/testpmd -l 0,1 --log-level 8 -- --total-num-mbufs=16384 \
    -i --port-topology=chained --disable-hw-vlan-filter \
    --disable-hw-vlan-strip --enable-rx-cksum --enable-lro \
    --crc-strip --txqflags=0

  set fwd csum
  tso set 1440 0
  csum set ip hw 0
  csum set tcp hw 0
  tso set 1440 1
  #csum set ip hw 1 # not supported by virtio
  csum set tcp hw 1
  start

Iperf log:

  root@ubuntu1404:~# ip netns exec ns-ixgbe iperf -c 1.1.1.1 -t 10
  ------------------------------------------------------------
  Client connecting to 1.1.1.1, TCP port 5001
  TCP window size: 85.0 KByte (default)
  ------------------------------------------------------------
  [  3] local 1.1.1.2 port 54460 connected with 1.1.1.1 port 5001
  [ ID] Interval       Transfer     Bandwidth
  [  3]  0.0-10.0 sec  6.14 GBytes  5.27 Gbits/sec
  root@ubuntu1404:~# ip netns exec ns-tap iperf -c 1.1.1.2 -t 10
  ------------------------------------------------------------
  Client connecting to 1.1.1.2, TCP port 5001
  TCP window size: 85.0 KByte (default)
  ------------------------------------------------------------
  [  3] local 1.1.1.1 port 58312 connected with 1.1.1.2 port 5001
  [ ID] Interval       Transfer     Bandwidth
  [  3]  0.0-10.0 sec  6.70 GBytes  5.76 Gbits/sec

Example of what we see with "set verbose 1" in testpmd:

  -- flow1: ixgbe2 -> port0 (ixgbe) -> testpmd -> port1 (virtio) <-> tap0
  port=0, mbuf=0x7f968ad9fdc0, pkt_len=24682, nb_segs=13:
  rx: l2_len=14 ethertype=800 l3_len=20 l4_proto=6 l4_len=32 flags=PKT_RX_L4_CKSUM_UNKNOWN PKT_RX_IP_CKSUM_UNKNOWN
  tx: m->l2_len=14 m->l3_len=20 m->l4_len=32
  tx: m->tso_segsz=1440
  tx: flags=PKT_TX_IP_CKSUM PKT_TX_L4_NO_CKSUM PKT_TX_TCP_SEG PKT_TX_IPV4

  -- flow2: tap0 -> port1 (virtio)-> testpmd -> port0 (ixgbe) -> ixgbe2
  port=1, mbuf=0x7f968acc9f40, pkt_len=42058, nb_segs=21:
  rx: l2_len=14 ethertype=800 l3_len=20 l4_proto=6 l4_len=32 flags=PKT_RX_L4_CKSUM_NONE PKT_RX_IP_CKSUM_UNKNOWN PKT_RX_LRO
  rx: m->lro_segsz=1440
  tx: m->l2_len=14 m->l3_len=20 m->l4_len=32
  tx: m->tso_segsz=1440
  tx: flags=PKT_TX_IP_CKSUM PKT_TX_L4_NO_CKSUM PKT_TX_TCP_SEG PKT_TX_IPV4

test2: hardware checksum only
-----------------------------

Configuration of testpmd:

  ./build/app/testpmd -l 0,1 --log-level 8 -- --total-num-mbufs=16384 \
    -i --port-topology=chained --disable-hw-vlan-filter \
    --disable-hw-vlan-strip --enable-rx-cksum --crc-strip --txqflags=0

  set fwd csum
  csum set ip hw 0
  csum set tcp hw 0
  csum set tcp hw 1
  start

Iperf log:

  root@ubuntu1404:~# ip netns exec ns-ixgbe iperf -c 1.1.1.1 -t 10
  ------------------------------------------------------------
  Client connecting to 1.1.1.1, TCP port 5001
  TCP window size: 85.0 KByte (default)
  ------------------------------------------------------------
  [  3] local 1.1.1.2 port 54462 connected with 1.1.1.1 port 5001
  [ ID] Interval       Transfer     Bandwidth
  [  3]  0.0-10.0 sec  4.49 GBytes  3.86 Gbits/sec
  root@ubuntu1404:~# ip netns exec ns-tap iperf -c 1.1.1.2 -t 10
  ------------------------------------------------------------
  Client connecting to 1.1.1.2, TCP port 5001
  TCP window size: 85.0 KByte (default)
  ------------------------------------------------------------
  [  3] local 1.1.1.1 port 58314 connected with 1.1.1.2 port 5001
  [ ID] Interval       Transfer     Bandwidth
  [  3]  0.0-10.0 sec  6.66 GBytes  5.72 Gbits/sec

Example of what we see with "set verbose 1" in testpmd:

  -- flow1: ixgbe2 -> port0 (ixgbe) -> testpmd -> port1 (virtio) <-> tap0
  port=0, mbuf=0x7f0adca89b40, pkt_len=1514, nb_segs=1:
  rx: l2_len=14 ethertype=800 l3_len=20 l4_proto=6 l4_len=32 flags=PKT_RX_L4_CKSUM_UNKNOWN PKT_RX_IP_CKSUM_UNKNOWN
  tx: m->l2_len=14 m->l3_len=20 m->l4_len=32
  tx: flags=PKT_TX_TCP_CKSUM PKT_TX_IPV4

  -- flow2: tap0 -> port1 (virtio)-> testpmd -> port0 (ixgbe) -> ixgbe2
  port=1, mbuf=0x7f0adcb98d80, pkt_len=1514, nb_segs=1:
  rx: l2_len=14 ethertype=800 l3_len=20 l4_proto=6 l4_len=32 flags=PKT_RX_L4_CKSUM_NONE PKT_RX_IP_CKSUM_UNKNOWN
  tx: m->l2_len=14 m->l3_len=20 m->l4_len=32
  tx: flags=PKT_TX_IP_CKSUM PKT_TX_TCP_CKSUM PKT_TX_IPV4

test3: no offload
-----------------

Configuration of testpmd:

  ./build/app/testpmd -l 0,1 --log-level 8 -- --total-num-mbufs=16384 \
    -i --port-topology=chained --disable-hw-vlan-filter --disable-hw-vlan-strip

  set fwd csum
  start

Iperf log:

  root@ubuntu1404:~# ip netns exec ns-ixgbe iperf -c 1.1.1.1 -t 10
  ------------------------------------------------------------
  Client connecting to 1.1.1.1, TCP port 5001
  TCP window size: 85.0 KByte (default)
  ------------------------------------------------------------
  [  3] local 1.1.1.2 port 54466 connected with 1.1.1.1 port 5001
  [ ID] Interval       Transfer     Bandwidth
  [  3]  0.0-10.0 sec  4.29 GBytes  3.68 Gbits/sec
  root@ubuntu1404:~# ip netns exec ns-tap iperf -c 1.1.1.2 -t 10
  ------------------------------------------------------------
  Client connecting to 1.1.1.2, TCP port 5001
  TCP window size: 85.0 KByte (default)
  ------------------------------------------------------------
  [  3] local 1.1.1.1 port 58316 connected with 1.1.1.2 port 5001
  [ ID] Interval       Transfer     Bandwidth
  [  3]  0.0-10.0 sec  6.66 GBytes  5.72 Gbits/sec

Example of what we see with "set verbose 1" in testpmd:

  -- flow1: ixgbe2 -> port0 (ixgbe) -> testpmd -> port1 (virtio) <-> tap0
  port=0, mbuf=0x7faf38b3e700, pkt_len=1514, nb_segs=1:
  rx: l2_len=14 ethertype=800 l3_len=20 l4_proto=6 l4_len=32 flags=PKT_RX_L4_CKSUM_UNKNOWN PKT_RX_IP_CKSUM_UNKNOWN
  tx: flags=PKT_TX_L4_NO_CKSUM PKT_TX_IPV4

  -- flow2: tap0 -> port1 (virtio)-> testpmd -> port0 (ixgbe) -> ixgbe2
  port=1, mbuf=0x7faf38b71500, pkt_len=1514, nb_segs=1:
  rx: l2_len=14 ethertype=800 l3_len=20 l4_proto=6 l4_len=32 flags=PKT_RX_L4_CKSUM_UNKNOWN PKT_RX_IP_CKSUM_UNKNOWN
  tx: flags=PKT_TX_L4_NO_CKSUM PKT_TX_IPV4

-- 
2.8.1

^ permalink raw reply

* [PATCH v3 01/12] net/virtio: move device initialization in a function
From: Olivier Matz @ 2016-10-13 14:16 UTC (permalink / raw)
  To: dev, yuanhan.liu
  Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang, maxime.coquelin
In-Reply-To: <1476368171-18176-1-git-send-email-olivier.matz@6wind.com>

Move all code related to device initialization in a new function
virtio_init_device().

This commit brings no functional change, it prepares the next commits
that will add the offload support. For that, it will be needed to
reinitialize the device from ethdev->configure(), using this new
function.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 99 ++++++++++++++++++++++----------------
 1 file changed, 58 insertions(+), 41 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index b4dfc0a..77ca569 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1118,46 +1118,13 @@ rx_func_get(struct rte_eth_dev *eth_dev)
 		eth_dev->rx_pkt_burst = &virtio_recv_pkts;
 }
 
-/*
- * This function is based on probe() function in virtio_pci.c
- * It returns 0 on success.
- */
-int
-eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
+static int
+virtio_init_device(struct rte_eth_dev *eth_dev)
 {
 	struct virtio_hw *hw = eth_dev->data->dev_private;
 	struct virtio_net_config *config;
 	struct virtio_net_config local_config;
-	struct rte_pci_device *pci_dev;
-	uint32_t dev_flags = RTE_ETH_DEV_DETACHABLE;
-	int ret;
-
-	RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr_mrg_rxbuf));
-
-	eth_dev->dev_ops = &virtio_eth_dev_ops;
-	eth_dev->tx_pkt_burst = &virtio_xmit_pkts;
-
-	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
-		rx_func_get(eth_dev);
-		return 0;
-	}
-
-	/* Allocate memory for storing MAC addresses */
-	eth_dev->data->mac_addrs = rte_zmalloc("virtio", VIRTIO_MAX_MAC_ADDRS * ETHER_ADDR_LEN, 0);
-	if (eth_dev->data->mac_addrs == NULL) {
-		PMD_INIT_LOG(ERR,
-			"Failed to allocate %d bytes needed to store MAC addresses",
-			VIRTIO_MAX_MAC_ADDRS * ETHER_ADDR_LEN);
-		return -ENOMEM;
-	}
-
-	pci_dev = eth_dev->pci_dev;
-
-	if (pci_dev) {
-		ret = vtpci_init(pci_dev, hw, &dev_flags);
-		if (ret)
-			return ret;
-	}
+	struct rte_pci_device *pci_dev = eth_dev->pci_dev;
 
 	/* Reset the device although not necessary at startup */
 	vtpci_reset(hw);
@@ -1172,10 +1139,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 
 	/* If host does not support status then disable LSC */
 	if (!vtpci_with_feature(hw, VIRTIO_NET_F_STATUS))
-		dev_flags &= ~RTE_ETH_DEV_INTR_LSC;
+		eth_dev->data->dev_flags &= ~RTE_ETH_DEV_INTR_LSC;
+	else
+		eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
-	eth_dev->data->dev_flags = dev_flags;
 
 	rx_func_get(eth_dev);
 
@@ -1254,12 +1222,61 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 			eth_dev->data->port_id, pci_dev->id.vendor_id,
 			pci_dev->id.device_id);
 
+	virtio_dev_cq_start(eth_dev);
+
+	return 0;
+}
+
+/*
+ * This function is based on probe() function in virtio_pci.c
+ * It returns 0 on success.
+ */
+int
+eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
+{
+	struct virtio_hw *hw = eth_dev->data->dev_private;
+	struct rte_pci_device *pci_dev;
+	uint32_t dev_flags = RTE_ETH_DEV_DETACHABLE;
+	int ret;
+
+	RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr_mrg_rxbuf));
+
+	eth_dev->dev_ops = &virtio_eth_dev_ops;
+	eth_dev->tx_pkt_burst = &virtio_xmit_pkts;
+
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		rx_func_get(eth_dev);
+		return 0;
+	}
+
+	/* Allocate memory for storing MAC addresses */
+	eth_dev->data->mac_addrs = rte_zmalloc("virtio", VIRTIO_MAX_MAC_ADDRS * ETHER_ADDR_LEN, 0);
+	if (eth_dev->data->mac_addrs == NULL) {
+		PMD_INIT_LOG(ERR,
+			"Failed to allocate %d bytes needed to store MAC addresses",
+			VIRTIO_MAX_MAC_ADDRS * ETHER_ADDR_LEN);
+		return -ENOMEM;
+	}
+
+	pci_dev = eth_dev->pci_dev;
+
+	if (pci_dev) {
+		ret = vtpci_init(pci_dev, hw, &dev_flags);
+		if (ret)
+			return ret;
+	}
+
+	eth_dev->data->dev_flags = dev_flags;
+
+	/* reset device and negotiate features */
+	ret = virtio_init_device(eth_dev);
+	if (ret < 0)
+		return ret;
+
 	/* Setup interrupt callback  */
 	if (eth_dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC)
 		rte_intr_callback_register(&pci_dev->intr_handle,
-				   virtio_interrupt_handler, eth_dev);
-
-	virtio_dev_cq_start(eth_dev);
+			virtio_interrupt_handler, eth_dev);
 
 	return 0;
 }
-- 
2.8.1

^ permalink raw reply related

* [PATCH v3 02/12] net/virtio: setup and start cq in configure callback
From: Olivier Matz @ 2016-10-13 14:16 UTC (permalink / raw)
  To: dev, yuanhan.liu
  Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang, maxime.coquelin
In-Reply-To: <1476368171-18176-1-git-send-email-olivier.matz@6wind.com>

Move the configuration of control queue in the configure callback.
This is needed by next commit, which introduces the reinitialization
of the device in the configure callback to change the feature flags.
Therefore, the control queue will have to be restarted at the same
place.

As virtio_dev_cq_queue_setup() is called from a place where
config->max_virtqueue_pairs is not available, we need to store this in
the private structure. It replaces max_rx_queues and max_tx_queues which
have the same value. The log showing the value of max_rx_queues and
max_tx_queues is also removed since config->max_virtqueue_pairs is
already displayed above.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 43 +++++++++++++++++++-------------------
 drivers/net/virtio/virtio_ethdev.h |  4 ++--
 drivers/net/virtio/virtio_pci.h    |  3 +--
 3 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 77ca569..f3921ac 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -552,6 +552,9 @@ virtio_dev_close(struct rte_eth_dev *dev)
 	if (hw->started == 1)
 		virtio_dev_stop(dev);
 
+	if (hw->cvq)
+		virtio_dev_queue_release(hw->cvq->vq);
+
 	/* reset the NIC */
 	if (dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC)
 		vtpci_irq_config(hw, VIRTIO_MSI_NO_VECTOR);
@@ -1191,16 +1194,7 @@ virtio_init_device(struct rte_eth_dev *eth_dev)
 			config->max_virtqueue_pairs = 1;
 		}
 
-		hw->max_rx_queues =
-			(VIRTIO_MAX_RX_QUEUES < config->max_virtqueue_pairs) ?
-			VIRTIO_MAX_RX_QUEUES : config->max_virtqueue_pairs;
-		hw->max_tx_queues =
-			(VIRTIO_MAX_TX_QUEUES < config->max_virtqueue_pairs) ?
-			VIRTIO_MAX_TX_QUEUES : config->max_virtqueue_pairs;
-
-		virtio_dev_cq_queue_setup(eth_dev,
-					config->max_virtqueue_pairs * 2,
-					SOCKET_ID_ANY);
+		hw->max_queue_pairs = config->max_virtqueue_pairs;
 
 		PMD_INIT_LOG(DEBUG, "config->max_virtqueue_pairs=%d",
 				config->max_virtqueue_pairs);
@@ -1211,19 +1205,15 @@ virtio_init_device(struct rte_eth_dev *eth_dev)
 				config->mac[2], config->mac[3],
 				config->mac[4], config->mac[5]);
 	} else {
-		hw->max_rx_queues = 1;
-		hw->max_tx_queues = 1;
+		PMD_INIT_LOG(DEBUG, "config->max_virtqueue_pairs=1");
+		hw->max_queue_pairs = 1;
 	}
 
-	PMD_INIT_LOG(DEBUG, "hw->max_rx_queues=%d   hw->max_tx_queues=%d",
-			hw->max_rx_queues, hw->max_tx_queues);
 	if (pci_dev)
 		PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x",
 			eth_dev->data->port_id, pci_dev->id.vendor_id,
 			pci_dev->id.device_id);
 
-	virtio_dev_cq_start(eth_dev);
-
 	return 0;
 }
 
@@ -1285,7 +1275,6 @@ static int
 eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
 {
 	struct rte_pci_device *pci_dev;
-	struct virtio_hw *hw = eth_dev->data->dev_private;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -1301,9 +1290,6 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
 	eth_dev->tx_pkt_burst = NULL;
 	eth_dev->rx_pkt_burst = NULL;
 
-	if (hw->cvq)
-		virtio_dev_queue_release(hw->cvq->vq);
-
 	rte_free(eth_dev->data->mac_addrs);
 	eth_dev->data->mac_addrs = NULL;
 
@@ -1352,6 +1338,7 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 {
 	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 	struct virtio_hw *hw = dev->data->dev_private;
+	int ret;
 
 	PMD_INIT_LOG(DEBUG, "configure");
 
@@ -1360,6 +1347,16 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
+	/* Setup and start control queue */
+	if (vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VQ)) {
+		ret = virtio_dev_cq_queue_setup(dev,
+			hw->max_queue_pairs * 2,
+			SOCKET_ID_ANY);
+		if (ret < 0)
+			return ret;
+		virtio_dev_cq_start(dev);
+	}
+
 	hw->vlan_strip = rxmode->hw_vlan_strip;
 
 	if (rxmode->hw_vlan_filter
@@ -1553,8 +1550,10 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		dev_info->driver_name = dev->driver->pci_drv.driver.name;
 	else
 		dev_info->driver_name = "virtio_user PMD";
-	dev_info->max_rx_queues = (uint16_t)hw->max_rx_queues;
-	dev_info->max_tx_queues = (uint16_t)hw->max_tx_queues;
+	dev_info->max_rx_queues =
+		RTE_MIN(hw->max_queue_pairs, VIRTIO_MAX_RX_QUEUES);
+	dev_info->max_tx_queues =
+		RTE_MIN(hw->max_queue_pairs, VIRTIO_MAX_TX_QUEUES);
 	dev_info->min_rx_bufsize = VIRTIO_MIN_RX_BUFSIZE;
 	dev_info->max_rx_pktlen = VIRTIO_MAX_RX_PKTLEN;
 	dev_info->max_mac_addrs = VIRTIO_MAX_MAC_ADDRS;
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index 04d626b..dc18341 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -47,8 +47,8 @@
 #define PAGE_SIZE 4096
 #endif
 
-#define VIRTIO_MAX_RX_QUEUES 128
-#define VIRTIO_MAX_TX_QUEUES 128
+#define VIRTIO_MAX_RX_QUEUES 128U
+#define VIRTIO_MAX_TX_QUEUES 128U
 #define VIRTIO_MAX_MAC_ADDRS 64
 #define VIRTIO_MIN_RX_BUFSIZE 64
 #define VIRTIO_MAX_RX_PKTLEN  9728
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index b8295a7..6930cd6 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -246,8 +246,7 @@ struct virtio_hw {
 	struct virtnet_ctl *cvq;
 	struct rte_pci_ioport io;
 	uint64_t    guest_features;
-	uint32_t    max_tx_queues;
-	uint32_t    max_rx_queues;
+	uint32_t    max_queue_pairs;
 	uint16_t    vtnet_hdr_size;
 	uint8_t	    vlan_strip;
 	uint8_t	    use_msix;
-- 
2.8.1

^ permalink raw reply related

* [PATCH v3 03/12] net/virtio: reinitialize the device in configure callback
From: Olivier Matz @ 2016-10-13 14:16 UTC (permalink / raw)
  To: dev, yuanhan.liu
  Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang, maxime.coquelin
In-Reply-To: <1476368171-18176-1-git-send-email-olivier.matz@6wind.com>

Add the ability to reset the virtio device in the configure callback
if the features flag changed since previous reset. This will be possible
with the introduction of offload support in next commits.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 26 +++++++++++++++++++-------
 drivers/net/virtio/virtio_pci.h    |  1 +
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index f3921ac..b5bc0ee 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1045,14 +1045,13 @@ virtio_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 }
 
 static int
-virtio_negotiate_features(struct virtio_hw *hw)
+virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
 {
 	uint64_t host_features;
 
 	/* Prepare guest_features: feature that driver wants to support */
-	hw->guest_features = VIRTIO_PMD_GUEST_FEATURES;
 	PMD_INIT_LOG(DEBUG, "guest_features before negotiate = %" PRIx64,
-		hw->guest_features);
+		req_features);
 
 	/* Read device(host) feature bits */
 	host_features = hw->vtpci_ops->get_features(hw);
@@ -1063,6 +1062,7 @@ virtio_negotiate_features(struct virtio_hw *hw)
 	 * Negotiate features: Subset of device feature bits are written back
 	 * guest feature bits.
 	 */
+	hw->guest_features = req_features;
 	hw->guest_features = vtpci_negotiate_features(hw, host_features);
 	PMD_INIT_LOG(DEBUG, "features after negotiate = %" PRIx64,
 		hw->guest_features);
@@ -1081,6 +1081,8 @@ virtio_negotiate_features(struct virtio_hw *hw)
 		}
 	}
 
+	hw->req_guest_features = req_features;
+
 	return 0;
 }
 
@@ -1121,8 +1123,9 @@ rx_func_get(struct rte_eth_dev *eth_dev)
 		eth_dev->rx_pkt_burst = &virtio_recv_pkts;
 }
 
+/* reset device and renegotiate features if needed */
 static int
-virtio_init_device(struct rte_eth_dev *eth_dev)
+virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 {
 	struct virtio_hw *hw = eth_dev->data->dev_private;
 	struct virtio_net_config *config;
@@ -1137,7 +1140,7 @@ virtio_init_device(struct rte_eth_dev *eth_dev)
 
 	/* Tell the host we've known how to drive the device. */
 	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_DRIVER);
-	if (virtio_negotiate_features(hw) < 0)
+	if (virtio_negotiate_features(hw, req_features) < 0)
 		return -1;
 
 	/* If host does not support status then disable LSC */
@@ -1258,8 +1261,8 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 
 	eth_dev->data->dev_flags = dev_flags;
 
-	/* reset device and negotiate features */
-	ret = virtio_init_device(eth_dev);
+	/* reset device and negotiate default features */
+	ret = virtio_init_device(eth_dev, VIRTIO_PMD_GUEST_FEATURES);
 	if (ret < 0)
 		return ret;
 
@@ -1338,6 +1341,7 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 {
 	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 	struct virtio_hw *hw = dev->data->dev_private;
+	uint64_t req_features;
 	int ret;
 
 	PMD_INIT_LOG(DEBUG, "configure");
@@ -1347,6 +1351,14 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
+	req_features = VIRTIO_PMD_GUEST_FEATURES;
+	/* if request features changed, reinit the device */
+	if (req_features != hw->req_guest_features) {
+		ret = virtio_init_device(dev, req_features);
+		if (ret < 0)
+			return ret;
+	}
+
 	/* Setup and start control queue */
 	if (vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VQ)) {
 		ret = virtio_dev_cq_queue_setup(dev,
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 6930cd6..bbf06ec 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -245,6 +245,7 @@ struct virtio_net_config;
 struct virtio_hw {
 	struct virtnet_ctl *cvq;
 	struct rte_pci_ioport io;
+	uint64_t    req_guest_features;
 	uint64_t    guest_features;
 	uint32_t    max_queue_pairs;
 	uint16_t    vtnet_hdr_size;
-- 
2.8.1

^ permalink raw reply related

* [PATCH v3 04/12] net: add function to calculate a checksum in a mbuf
From: Olivier Matz @ 2016-10-13 14:16 UTC (permalink / raw)
  To: dev, yuanhan.liu
  Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang, maxime.coquelin
In-Reply-To: <1476368171-18176-1-git-send-email-olivier.matz@6wind.com>

This function can be used to calculate the checksum of data embedded in
mbuf, that can be composed of several segments.

This function will be used by the virtio pmd in next commits to calculate
the checksum in software in case the protocol is not recognized.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 doc/guides/rel_notes/release_16_11.rst |  5 +++
 lib/librte_net/rte_ip.h                | 71 ++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/doc/guides/rel_notes/release_16_11.rst b/doc/guides/rel_notes/release_16_11.rst
index 51fc707..fbc0cbd 100644
--- a/doc/guides/rel_notes/release_16_11.rst
+++ b/doc/guides/rel_notes/release_16_11.rst
@@ -104,6 +104,11 @@ New Features
   The config option ``RTE_MACHINE`` can be used to pass code names to the compiler as ``-march`` flag.
 
 
+* **Added a functions to calculate the checksum of data in a mbuf.**
+
+  Added a new function ``rte_raw_cksum_mbuf()`` to process the checksum of
+  data embedded in an mbuf chain.
+
 Resolved Issues
 ---------------
 
diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index 5b7554a..4491b86 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -230,6 +230,77 @@ rte_raw_cksum(const void *buf, size_t len)
 }
 
 /**
+ * Compute the raw (non complemented) checksum of a packet.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param off
+ *   The offset in bytes to start the checksum.
+ * @param len
+ *   The length in bytes of the data to ckecksum.
+ * @param cksum
+ *   A pointer to the checksum, filled on success.
+ * @return
+ *   0 on success, -1 on error (bad length or offset).
+ */
+static inline int
+rte_raw_cksum_mbuf(const struct rte_mbuf *m, uint32_t off, uint32_t len,
+	uint16_t *cksum)
+{
+	const struct rte_mbuf *seg;
+	const char *buf;
+	uint32_t sum, tmp;
+	uint32_t seglen, done;
+
+	/* easy case: all data in the first segment */
+	if (off + len <= rte_pktmbuf_data_len(m)) {
+		*cksum = rte_raw_cksum(rte_pktmbuf_mtod_offset(m,
+				const char *, off), len);
+		return 0;
+	}
+
+	if (unlikely(off + len > rte_pktmbuf_pkt_len(m)))
+		return -1; /* invalid params, return a dummy value */
+
+	/* else browse the segment to find offset */
+	seglen = 0;
+	for (seg = m; seg != NULL; seg = seg->next) {
+		seglen = rte_pktmbuf_data_len(seg);
+		if (off < seglen)
+			break;
+		off -= seglen;
+	}
+	seglen -= off;
+	buf = rte_pktmbuf_mtod_offset(seg, const char *, off);
+	if (seglen >= len) {
+		/* all in one segment */
+		*cksum = rte_raw_cksum(buf, len);
+		return 0;
+	}
+
+	/* hard case: process checksum of several segments */
+	sum = 0;
+	done = 0;
+	for (;;) {
+		tmp = __rte_raw_cksum(buf, seglen, 0);
+		if (done & 1)
+			tmp = rte_bswap16(tmp);
+		sum += tmp;
+		done += seglen;
+		if (done == len)
+			break;
+		seg = seg->next;
+		buf = rte_pktmbuf_mtod(seg, const char *);
+		seglen = rte_pktmbuf_data_len(seg);
+		if (seglen > len - done)
+			seglen = len - done;
+	}
+
+	*cksum = __rte_raw_cksum_reduce(sum);
+	return 0;
+}
+
+/**
  * Process the IPv4 checksum of an IPv4 header.
  *
  * The checksum field must be set to 0 by the caller.
-- 
2.8.1

^ permalink raw reply related

* [PATCH v3 05/12] mbuf: add new Rx checksum mbuf flags
From: Olivier Matz @ 2016-10-13 14:16 UTC (permalink / raw)
  To: dev, yuanhan.liu
  Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang, maxime.coquelin
In-Reply-To: <1476368171-18176-1-git-send-email-olivier.matz@6wind.com>

Following discussions in [1] and [2], introduce a new bit to
describe the Rx checksum status in mbuf.

Before this patch, only one flag was available:
  PKT_RX_L4_CKSUM_BAD: L4 cksum of RX pkt. is not OK.

And same for L3:
  PKT_RX_IP_CKSUM_BAD: IP cksum of RX pkt. is not OK.

This had 2 issues:
- it was not possible to differentiate "checksum good" from
  "checksum unknown".
- it was not possible for a virtual driver to say "the checksum
  in packet may be wrong, but data integrity is valid".

This patch tries to solve this issue by having 4 states (2 bits)
for the IP and L4 Rx checksums. New values are:

 - PKT_RX_L4_CKSUM_UNKNOWN: no information about the RX L4 checksum
   -> the application should verify the checksum by sw
 - PKT_RX_L4_CKSUM_BAD: the L4 checksum in the packet is wrong
   -> the application can drop the packet without additional check
 - PKT_RX_L4_CKSUM_GOOD: the L4 checksum in the packet is valid
   -> the application can accept the packet without verifying the
      checksum by sw
 - PKT_RX_L4_CKSUM_NONE: the L4 checksum is not correct in the packet
   data, but the integrity of the L4 data is verified.
   -> the application can process the packet but must not verify the
      checksum by sw. It has to take care to recalculate the cksum
      if the packet is transmitted (either by sw or using tx offload)

  And same for L3 (replace L4 by IP in description above).

This commit tries to be compatible with existing applications that
only check the existing flag (CKSUM_BAD).

[1] http://dpdk.org/ml/archives/dev/2016-May/039920.html
[2] http://dpdk.org/ml/archives/dev/2016-June/040007.html

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 doc/guides/rel_notes/release_16_11.rst |  6 ++++
 lib/librte_mbuf/rte_mbuf.c             | 16 +++++++++--
 lib/librte_mbuf/rte_mbuf.h             | 51 ++++++++++++++++++++++++++++++++--
 3 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/doc/guides/rel_notes/release_16_11.rst b/doc/guides/rel_notes/release_16_11.rst
index fbc0cbd..2ec63b2 100644
--- a/doc/guides/rel_notes/release_16_11.rst
+++ b/doc/guides/rel_notes/release_16_11.rst
@@ -109,6 +109,12 @@ New Features
   Added a new function ``rte_raw_cksum_mbuf()`` to process the checksum of
   data embedded in an mbuf chain.
 
+* **Added new Rx checksum mbuf flags.**
+
+  Added new Rx checksum flags in mbufs to describe more states: unknown,
+  good, bad, or not present (useful for virtual drivers). This modification
+  was done for IP and L4.
+
 Resolved Issues
 ---------------
 
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 4e1fdd1..8d9b875 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -309,7 +309,11 @@ const char *rte_get_rx_ol_flag_name(uint64_t mask)
 	case PKT_RX_RSS_HASH: return "PKT_RX_RSS_HASH";
 	case PKT_RX_FDIR: return "PKT_RX_FDIR";
 	case PKT_RX_L4_CKSUM_BAD: return "PKT_RX_L4_CKSUM_BAD";
+	case PKT_RX_L4_CKSUM_GOOD: return "PKT_RX_L4_CKSUM_GOOD";
+	case PKT_RX_L4_CKSUM_NONE: return "PKT_RX_L4_CKSUM_NONE";
 	case PKT_RX_IP_CKSUM_BAD: return "PKT_RX_IP_CKSUM_BAD";
+	case PKT_RX_IP_CKSUM_GOOD: return "PKT_RX_IP_CKSUM_GOOD";
+	case PKT_RX_IP_CKSUM_NONE: return "PKT_RX_IP_CKSUM_NONE";
 	case PKT_RX_EIP_CKSUM_BAD: return "PKT_RX_EIP_CKSUM_BAD";
 	case PKT_RX_VLAN_STRIPPED: return "PKT_RX_VLAN_STRIPPED";
 	case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP";
@@ -333,8 +337,16 @@ rte_get_rx_ol_flag_list(uint64_t mask, char *buf, size_t buflen)
 		{ PKT_RX_VLAN_PKT, PKT_RX_VLAN_PKT, NULL },
 		{ PKT_RX_RSS_HASH, PKT_RX_RSS_HASH, NULL },
 		{ PKT_RX_FDIR, PKT_RX_FDIR, NULL },
-		{ PKT_RX_L4_CKSUM_BAD, PKT_RX_L4_CKSUM_BAD, NULL },
-		{ PKT_RX_IP_CKSUM_BAD, PKT_RX_IP_CKSUM_BAD, NULL },
+		{ PKT_RX_L4_CKSUM_BAD, PKT_RX_L4_CKSUM_MASK, NULL },
+		{ PKT_RX_L4_CKSUM_GOOD, PKT_RX_L4_CKSUM_MASK, NULL },
+		{ PKT_RX_L4_CKSUM_NONE, PKT_RX_L4_CKSUM_MASK, NULL },
+		{ PKT_RX_L4_CKSUM_UNKNOWN, PKT_RX_L4_CKSUM_MASK,
+		  "PKT_RX_L4_CKSUM_UNKNOWN" },
+		{ PKT_RX_IP_CKSUM_BAD, PKT_RX_IP_CKSUM_MASK, NULL },
+		{ PKT_RX_IP_CKSUM_GOOD, PKT_RX_IP_CKSUM_MASK, NULL },
+		{ PKT_RX_IP_CKSUM_NONE, PKT_RX_IP_CKSUM_MASK, NULL },
+		{ PKT_RX_IP_CKSUM_UNKNOWN, PKT_RX_IP_CKSUM_MASK,
+		  "PKT_RX_IP_CKSUM_UNKNOWN" },
 		{ PKT_RX_EIP_CKSUM_BAD, PKT_RX_EIP_CKSUM_BAD, NULL },
 		{ PKT_RX_VLAN_STRIPPED, PKT_RX_VLAN_STRIPPED, NULL },
 		{ PKT_RX_IEEE1588_PTP, PKT_RX_IEEE1588_PTP, NULL },
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 7541070..38022a3 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -91,8 +91,25 @@ extern "C" {
 
 #define PKT_RX_RSS_HASH      (1ULL << 1)  /**< RX packet with RSS hash result. */
 #define PKT_RX_FDIR          (1ULL << 2)  /**< RX packet with FDIR match indicate. */
-#define PKT_RX_L4_CKSUM_BAD  (1ULL << 3)  /**< L4 cksum of RX pkt. is not OK. */
-#define PKT_RX_IP_CKSUM_BAD  (1ULL << 4)  /**< IP cksum of RX pkt. is not OK. */
+
+/**
+ * Deprecated.
+ * Checking this flag alone is deprecated: check the 2 bits of
+ * PKT_RX_L4_CKSUM_MASK.
+ * This flag was set when the L4 checksum of a packet was detected as
+ * wrong by the hardware.
+ */
+#define PKT_RX_L4_CKSUM_BAD  (1ULL << 3)
+
+/**
+ * Deprecated.
+ * Checking this flag alone is deprecated: check the 2 bits of
+ * PKT_RX_IP_CKSUM_MASK.
+ * This flag was set when the IP checksum of a packet was detected as
+ * wrong by the hardware.
+ */
+#define PKT_RX_IP_CKSUM_BAD  (1ULL << 4)
+
 #define PKT_RX_EIP_CKSUM_BAD (1ULL << 5)  /**< External IP header checksum error. */
 
 /**
@@ -102,7 +119,35 @@ extern "C" {
  */
 #define PKT_RX_VLAN_STRIPPED (1ULL << 6)
 
-/* hole, some bits can be reused here  */
+/**
+ * Mask of bits used to determine the status of RX IP checksum.
+ * - PKT_RX_IP_CKSUM_UNKNOWN: no information about the RX IP checksum
+ * - PKT_RX_IP_CKSUM_BAD: the IP checksum in the packet is wrong
+ * - PKT_RX_IP_CKSUM_GOOD: the IP checksum in the packet is valid
+ * - PKT_RX_IP_CKSUM_NONE: the IP checksum is not correct in the packet
+ *   data, but the integrity of the IP header is verified.
+ */
+#define PKT_RX_IP_CKSUM_MASK ((1ULL << 4) | (1ULL << 7))
+
+#define PKT_RX_IP_CKSUM_UNKNOWN 0
+#define PKT_RX_IP_CKSUM_BAD     (1ULL << 4)
+#define PKT_RX_IP_CKSUM_GOOD    (1ULL << 7)
+#define PKT_RX_IP_CKSUM_NONE    ((1ULL << 4) | (1ULL << 7))
+
+/**
+ * Mask of bits used to determine the status of RX L4 checksum.
+ * - PKT_RX_L4_CKSUM_UNKNOWN: no information about the RX L4 checksum
+ * - PKT_RX_L4_CKSUM_BAD: the L4 checksum in the packet is wrong
+ * - PKT_RX_L4_CKSUM_GOOD: the L4 checksum in the packet is valid
+ * - PKT_RX_L4_CKSUM_NONE: the L4 checksum is not correct in the packet
+ *   data, but the integrity of the L4 data is verified.
+ */
+#define PKT_RX_L4_CKSUM_MASK ((1ULL << 3) | (1ULL << 8))
+
+#define PKT_RX_L4_CKSUM_UNKNOWN 0
+#define PKT_RX_L4_CKSUM_BAD     (1ULL << 3)
+#define PKT_RX_L4_CKSUM_GOOD    (1ULL << 8)
+#define PKT_RX_L4_CKSUM_NONE    ((1ULL << 3) | (1ULL << 8))
 
 #define PKT_RX_IEEE1588_PTP  (1ULL << 9)  /**< RX IEEE1588 L2 Ethernet PT Packet. */
 #define PKT_RX_IEEE1588_TMST (1ULL << 10) /**< RX IEEE1588 L2/L4 timestamped packet.*/
-- 
2.8.1

^ permalink raw reply related

* [PATCH v3 07/12] mbuf: new flag for LRO
From: Olivier Matz @ 2016-10-13 14:16 UTC (permalink / raw)
  To: dev, yuanhan.liu
  Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang, maxime.coquelin
In-Reply-To: <1476368171-18176-1-git-send-email-olivier.matz@6wind.com>

When receiving coalesced packets in virtio, the original size of the
segments is provided. This is a useful information because it allows to
resegment with the same size.

Add a RX new flag in mbuf, that can be set when packets are coalesced by
a hardware or virtual driver when the m->tso_segsz field is valid and is
set to the segment size of original packets.

This flag is used in next commits in the virtio pmd.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 doc/guides/rel_notes/release_16_11.rst | 5 +++++
 lib/librte_mbuf/rte_mbuf.c             | 2 ++
 lib/librte_mbuf/rte_mbuf.h             | 7 +++++++
 3 files changed, 14 insertions(+)

diff --git a/doc/guides/rel_notes/release_16_11.rst b/doc/guides/rel_notes/release_16_11.rst
index 2ec63b2..c9fcfb9 100644
--- a/doc/guides/rel_notes/release_16_11.rst
+++ b/doc/guides/rel_notes/release_16_11.rst
@@ -115,6 +115,11 @@ New Features
   good, bad, or not present (useful for virtual drivers). This modification
   was done for IP and L4.
 
+* **Added a LRO mbuf flag.**
+
+  Added a new RX LRO mbuf flag, used when packets are coalesced. This
+  flag indicates that the segment size of original packets is known.
+
 Resolved Issues
 ---------------
 
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 8d9b875..63f43c8 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -319,6 +319,7 @@ const char *rte_get_rx_ol_flag_name(uint64_t mask)
 	case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP";
 	case PKT_RX_IEEE1588_TMST: return "PKT_RX_IEEE1588_TMST";
 	case PKT_RX_QINQ_STRIPPED: return "PKT_RX_QINQ_STRIPPED";
+	case PKT_RX_LRO: return "PKT_RX_LRO";
 	default: return NULL;
 	}
 }
@@ -352,6 +353,7 @@ rte_get_rx_ol_flag_list(uint64_t mask, char *buf, size_t buflen)
 		{ PKT_RX_IEEE1588_PTP, PKT_RX_IEEE1588_PTP, NULL },
 		{ PKT_RX_IEEE1588_TMST, PKT_RX_IEEE1588_TMST, NULL },
 		{ PKT_RX_QINQ_STRIPPED, PKT_RX_QINQ_STRIPPED, NULL },
+		{ PKT_RX_LRO, PKT_RX_LRO, NULL },
 	};
 	const char *name;
 	unsigned int i;
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 38022a3..f5eedda 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -170,6 +170,13 @@ extern "C" {
  */
 #define PKT_RX_QINQ_PKT      PKT_RX_QINQ_STRIPPED
 
+/**
+ * When packets are coalesced by a hardware or virtual driver, this flag
+ * can be set in the RX mbuf, meaning that the m->tso_segsz field is
+ * valid and is set to the segment size of original packets.
+ */
+#define PKT_RX_LRO           (1ULL << 16)
+
 /* add new RX flags here */
 
 /* add new TX flags here */
-- 
2.8.1

^ permalink raw reply related

* [PATCH v3 08/12] app/testpmd: display lro segment size
From: Olivier Matz @ 2016-10-13 14:16 UTC (permalink / raw)
  To: dev, yuanhan.liu
  Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang, maxime.coquelin
In-Reply-To: <1476368171-18176-1-git-send-email-olivier.matz@6wind.com>

In csumonly engine, display the value of LRO segment if the
LRO flag is set.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 app/test-pmd/csumonly.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index da15185..57e6ae2 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -822,6 +822,8 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 				"l4_proto=%d l4_len=%d flags=%s\n",
 				info.l2_len, rte_be_to_cpu_16(info.ethertype),
 				info.l3_len, info.l4_proto, info.l4_len, buf);
+			if (rx_ol_flags & PKT_RX_LRO)
+				printf("rx: m->lro_segsz=%u\n", m->tso_segsz);
 			if (info.is_tunnel == 1)
 				printf("rx: outer_l2_len=%d outer_ethertype=%x "
 					"outer_l3_len=%d\n", info.outer_l2_len,
-- 
2.8.1

^ permalink raw reply related

* [PATCH v3 09/12] net/virtio: add Rx checksum offload support
From: Olivier Matz @ 2016-10-13 14:16 UTC (permalink / raw)
  To: dev, yuanhan.liu
  Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang, maxime.coquelin
In-Reply-To: <1476368171-18176-1-git-send-email-olivier.matz@6wind.com>

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 drivers/net/virtio/virtio_ethdev.c | 21 ++++++----
 drivers/net/virtio/virtio_ethdev.h |  2 +-
 drivers/net/virtio/virtio_rxtx.c   | 79 ++++++++++++++++++++++++++++++++++++++
 drivers/net/virtio/virtqueue.h     |  1 +
 4 files changed, 95 insertions(+), 8 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index b5bc0ee..00b4c38 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1262,7 +1262,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 	eth_dev->data->dev_flags = dev_flags;
 
 	/* reset device and negotiate default features */
-	ret = virtio_init_device(eth_dev, VIRTIO_PMD_GUEST_FEATURES);
+	ret = virtio_init_device(eth_dev, VIRTIO_PMD_DEFAULT_GUEST_FEATURES);
 	if (ret < 0)
 		return ret;
 
@@ -1345,13 +1345,10 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 	int ret;
 
 	PMD_INIT_LOG(DEBUG, "configure");
+	req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES;
+	if (rxmode->hw_ip_checksum)
+		req_features |= (1ULL << VIRTIO_NET_F_GUEST_CSUM);
 
-	if (rxmode->hw_ip_checksum) {
-		PMD_DRV_LOG(ERR, "HW IP checksum not supported");
-		return -EINVAL;
-	}
-
-	req_features = VIRTIO_PMD_GUEST_FEATURES;
 	/* if request features changed, reinit the device */
 	if (req_features != hw->req_guest_features) {
 		ret = virtio_init_device(dev, req_features);
@@ -1359,6 +1356,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 			return ret;
 	}
 
+	if (rxmode->hw_ip_checksum &&
+		!vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_CSUM)) {
+		PMD_DRV_LOG(NOTICE,
+			"rx ip checksum not available on this host");
+		return -ENOTSUP;
+	}
+
 	/* Setup and start control queue */
 	if (vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VQ)) {
 		ret = virtio_dev_cq_queue_setup(dev,
@@ -1572,6 +1576,9 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->default_txconf = (struct rte_eth_txconf) {
 		.txq_flags = ETH_TXQ_FLAGS_NOOFFLOADS
 	};
+	dev_info->rx_offload_capa =
+		DEV_RX_OFFLOAD_TCP_CKSUM |
+		DEV_RX_OFFLOAD_UDP_CKSUM;
 }
 
 /*
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index dc18341..fd29a7f 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -54,7 +54,7 @@
 #define VIRTIO_MAX_RX_PKTLEN  9728
 
 /* Features desired/implemented by this driver. */
-#define VIRTIO_PMD_GUEST_FEATURES		\
+#define VIRTIO_PMD_DEFAULT_GUEST_FEATURES	\
 	(1u << VIRTIO_NET_F_MAC		  |	\
 	 1u << VIRTIO_NET_F_STATUS	  |	\
 	 1u << VIRTIO_NET_F_MQ		  |	\
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 9ab441b..fc0d84b 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -51,6 +51,8 @@
 #include <rte_errno.h>
 #include <rte_byteorder.h>
 #include <rte_cpuflags.h>
+#include <rte_net.h>
+#include <rte_ip.h>
 
 #include "virtio_logs.h"
 #include "virtio_ethdev.h"
@@ -632,6 +634,63 @@ virtio_update_packet_stats(struct virtnet_stats *stats, struct rte_mbuf *mbuf)
 	}
 }
 
+/* Optionally fill offload information in structure */
+static int
+virtio_rx_offload(struct rte_mbuf *m, struct virtio_net_hdr *hdr)
+{
+	struct rte_net_hdr_lens hdr_lens;
+	uint32_t hdrlen, ptype;
+	int l4_supported = 0;
+
+	/* nothing to do */
+	if (hdr->flags == 0 && hdr->gso_type == VIRTIO_NET_HDR_GSO_NONE)
+		return 0;
+
+	m->ol_flags |= PKT_RX_IP_CKSUM_UNKNOWN;
+
+	ptype = rte_net_get_ptype(m, &hdr_lens, RTE_PTYPE_ALL_MASK);
+	m->packet_type = ptype;
+	if ((ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_TCP ||
+	    (ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_UDP ||
+	    (ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_SCTP)
+		l4_supported = 1;
+
+	if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+		hdrlen = hdr_lens.l2_len + hdr_lens.l3_len + hdr_lens.l4_len;
+		if (hdr->csum_start <= hdrlen && l4_supported) {
+			m->ol_flags |= PKT_RX_L4_CKSUM_NONE;
+		} else {
+			/* Unknown proto or tunnel, do sw cksum. We can assume
+			 * the cksum field is in the first segment since the
+			 * buffers we provided to the host are large enough.
+			 * In case of SCTP, this will be wrong since it's a CRC
+			 * but there's nothing we can do.
+			 */
+			uint16_t csum, off;
+
+			rte_raw_cksum_mbuf(m, hdr->csum_start,
+				rte_pktmbuf_pkt_len(m) - hdr->csum_start,
+				&csum);
+			if (likely(csum != 0xffff))
+				csum = ~csum;
+			off = hdr->csum_offset + hdr->csum_start;
+			if (rte_pktmbuf_data_len(m) >= off + 1)
+				*rte_pktmbuf_mtod_offset(m, uint16_t *,
+					off) = csum;
+		}
+	} else if (hdr->flags & VIRTIO_NET_HDR_F_DATA_VALID && l4_supported) {
+		m->ol_flags |= PKT_RX_L4_CKSUM_GOOD;
+	}
+
+	return 0;
+}
+
+static inline int
+rx_offload_enabled(struct virtio_hw *hw)
+{
+	return vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_CSUM);
+}
+
 #define VIRTIO_MBUF_BURST_SZ 64
 #define DESC_PER_CACHELINE (RTE_CACHE_LINE_SIZE / sizeof(struct vring_desc))
 uint16_t
@@ -647,6 +706,8 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	int error;
 	uint32_t i, nb_enqueued;
 	uint32_t hdr_size;
+	int offload;
+	struct virtio_net_hdr *hdr;
 
 	nb_used = VIRTQUEUE_NUSED(vq);
 
@@ -664,6 +725,7 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	nb_rx = 0;
 	nb_enqueued = 0;
 	hdr_size = hw->vtnet_hdr_size;
+	offload = rx_offload_enabled(hw);
 
 	for (i = 0; i < num ; i++) {
 		rxm = rcv_pkts[i];
@@ -688,9 +750,18 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		rxm->pkt_len = (uint32_t)(len[i] - hdr_size);
 		rxm->data_len = (uint16_t)(len[i] - hdr_size);
 
+		hdr = (struct virtio_net_hdr *)((char *)rxm->buf_addr +
+			RTE_PKTMBUF_HEADROOM - hdr_size);
+
 		if (hw->vlan_strip)
 			rte_vlan_strip(rxm);
 
+		if (offload && virtio_rx_offload(rxm, hdr) < 0) {
+			virtio_discard_rxbuf(vq, rxm);
+			rxvq->stats.errors++;
+			continue;
+		}
+
 		VIRTIO_DUMP_PACKET(rxm, rxm->data_len);
 
 		rx_pkts[nb_rx++] = rxm;
@@ -750,6 +821,7 @@ virtio_recv_mergeable_pkts(void *rx_queue,
 	uint16_t extra_idx;
 	uint32_t seg_res;
 	uint32_t hdr_size;
+	int offload;
 
 	nb_used = VIRTQUEUE_NUSED(vq);
 
@@ -765,6 +837,7 @@ virtio_recv_mergeable_pkts(void *rx_queue,
 	extra_idx = 0;
 	seg_res = 0;
 	hdr_size = hw->vtnet_hdr_size;
+	offload = rx_offload_enabled(hw);
 
 	while (i < nb_used) {
 		struct virtio_net_hdr_mrg_rxbuf *header;
@@ -810,6 +883,12 @@ virtio_recv_mergeable_pkts(void *rx_queue,
 		rx_pkts[nb_rx] = rxm;
 		prev = rxm;
 
+		if (offload && virtio_rx_offload(rxm, &header->hdr) < 0) {
+			virtio_discard_rxbuf(vq, rxm);
+			rxvq->stats.errors++;
+			continue;
+		}
+
 		seg_res = seg_num - 1;
 
 		while (seg_res != 0) {
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index 6737b81..ef0027b 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -223,6 +223,7 @@ struct virtqueue {
  */
 struct virtio_net_hdr {
 #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1    /**< Use csum_start,csum_offset*/
+#define VIRTIO_NET_HDR_F_DATA_VALID 2    /**< Checksum is valid */
 	uint8_t flags;
 #define VIRTIO_NET_HDR_GSO_NONE     0    /**< Not a GSO frame */
 #define VIRTIO_NET_HDR_GSO_TCPV4    1    /**< GSO frame, IPv4 TCP (TSO) */
-- 
2.8.1

^ permalink raw reply related

* [PATCH v3 06/12] app/testpmd: adapt checksum stats in csum engine
From: Olivier Matz @ 2016-10-13 14:16 UTC (permalink / raw)
  To: dev, yuanhan.liu
  Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang, maxime.coquelin
In-Reply-To: <1476368171-18176-1-git-send-email-olivier.matz@6wind.com>

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 app/test-pmd/csumonly.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 27d0f08..da15185 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -697,8 +697,10 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		rx_ol_flags = m->ol_flags;
 
 		/* Update the L3/L4 checksum error packet statistics */
-		rx_bad_ip_csum += ((rx_ol_flags & PKT_RX_IP_CKSUM_BAD) != 0);
-		rx_bad_l4_csum += ((rx_ol_flags & PKT_RX_L4_CKSUM_BAD) != 0);
+		if ((rx_ol_flags & PKT_RX_IP_CKSUM_MASK) == PKT_RX_IP_CKSUM_BAD)
+			rx_bad_ip_csum += 1;
+		if ((rx_ol_flags & PKT_RX_L4_CKSUM_MASK) == PKT_RX_L4_CKSUM_BAD)
+			rx_bad_l4_csum += 1;
 
 		/* step 1: dissect packet, parsing optional vlan, ip4/ip6, vxlan
 		 * and inner headers */
-- 
2.8.1

^ permalink raw reply related

* [PATCH v3 11/12] net/virtio: add Lro support
From: Olivier Matz @ 2016-10-13 14:16 UTC (permalink / raw)
  To: dev, yuanhan.liu
  Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang, maxime.coquelin
In-Reply-To: <1476368171-18176-1-git-send-email-olivier.matz@6wind.com>

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 15 ++++++++++++++-
 drivers/net/virtio/virtio_ethdev.h |  9 ---------
 drivers/net/virtio/virtio_rxtx.c   | 25 ++++++++++++++++++++++++-
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index c3c53be..109f855 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1348,6 +1348,10 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 	req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES;
 	if (rxmode->hw_ip_checksum)
 		req_features |= (1ULL << VIRTIO_NET_F_GUEST_CSUM);
+	if (rxmode->enable_lro)
+		req_features |=
+			(1ULL << VIRTIO_NET_F_GUEST_TSO4) |
+			(1ULL << VIRTIO_NET_F_GUEST_TSO6);
 
 	/* if request features changed, reinit the device */
 	if (req_features != hw->req_guest_features) {
@@ -1363,6 +1367,14 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 		return -ENOTSUP;
 	}
 
+	if (rxmode->enable_lro &&
+		(!vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_TSO4) ||
+			!vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_TSO4))) {
+		PMD_DRV_LOG(NOTICE,
+			"lro not available on this host");
+		return -ENOTSUP;
+	}
+
 	/* Setup and start control queue */
 	if (vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VQ)) {
 		ret = virtio_dev_cq_queue_setup(dev,
@@ -1578,7 +1590,8 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	};
 	dev_info->rx_offload_capa =
 		DEV_RX_OFFLOAD_TCP_CKSUM |
-		DEV_RX_OFFLOAD_UDP_CKSUM;
+		DEV_RX_OFFLOAD_UDP_CKSUM |
+		DEV_RX_OFFLOAD_TCP_LRO;
 	dev_info->tx_offload_capa = 0;
 
 	if (hw->guest_features & (1ULL << VIRTIO_NET_F_CSUM)) {
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index adca6ba..d55e7ed 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -117,13 +117,4 @@ uint16_t virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts,
 
 int eth_virtio_dev_init(struct rte_eth_dev *eth_dev);
 
-/*
- * The VIRTIO_NET_F_GUEST_TSO[46] features permit the host to send us
- * frames larger than 1514 bytes. We do not yet support software LRO
- * via tcp_lro_rx().
- */
-#define VTNET_LRO_FEATURES (VIRTIO_NET_F_GUEST_TSO4 | \
-			    VIRTIO_NET_F_GUEST_TSO6 | VIRTIO_NET_F_GUEST_ECN)
-
-
 #endif /* _VIRTIO_ETHDEV_H_ */
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 675dc43..0fa635a 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -715,13 +715,36 @@ virtio_rx_offload(struct rte_mbuf *m, struct virtio_net_hdr *hdr)
 		m->ol_flags |= PKT_RX_L4_CKSUM_GOOD;
 	}
 
+	/* GSO request, save required information in mbuf */
+	if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
+		/* Check unsupported modes */
+		if ((hdr->gso_type & VIRTIO_NET_HDR_GSO_ECN) ||
+		    (hdr->gso_size == 0)) {
+			return -EINVAL;
+		}
+
+		/* Update mss lengthes in mbuf */
+		m->tso_segsz = hdr->gso_size;
+		switch (hdr->gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
+			case VIRTIO_NET_HDR_GSO_TCPV4:
+			case VIRTIO_NET_HDR_GSO_TCPV6:
+				m->ol_flags |= PKT_RX_LRO | \
+					PKT_RX_L4_CKSUM_NONE;
+				break;
+			default:
+				return -EINVAL;
+		}
+	}
+
 	return 0;
 }
 
 static inline int
 rx_offload_enabled(struct virtio_hw *hw)
 {
-	return vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_CSUM);
+	return vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_CSUM) ||
+		vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_TSO4) ||
+		vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_TSO6);
 }
 
 #define VIRTIO_MBUF_BURST_SZ 64
-- 
2.8.1

^ permalink raw reply related

* [PATCH v3 12/12] net/virtio: add Tso support
From: Olivier Matz @ 2016-10-13 14:16 UTC (permalink / raw)
  To: dev, yuanhan.liu
  Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang, maxime.coquelin
In-Reply-To: <1476368171-18176-1-git-send-email-olivier.matz@6wind.com>

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 drivers/net/virtio/virtio_ethdev.c |   6 ++
 drivers/net/virtio/virtio_ethdev.h |   2 +
 drivers/net/virtio/virtio_rxtx.c   | 133 +++++++++++++++++++++++++++++++++++--
 3 files changed, 136 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 109f855..969edb6 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1572,6 +1572,7 @@ virtio_dev_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complet
 static void
 virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
+	uint64_t tso_mask;
 	struct virtio_hw *hw = dev->data->dev_private;
 
 	if (dev->pci_dev)
@@ -1599,6 +1600,11 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 			DEV_TX_OFFLOAD_UDP_CKSUM |
 			DEV_TX_OFFLOAD_TCP_CKSUM;
 	}
+
+	tso_mask = (1ULL << VIRTIO_NET_F_HOST_TSO4) |
+		(1ULL << VIRTIO_NET_F_HOST_TSO6);
+	if ((hw->guest_features & tso_mask) == tso_mask)
+		dev_info->tx_offload_capa |= DEV_TX_OFFLOAD_TCP_TSO;
 }
 
 /*
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index d55e7ed..f77f618 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -63,6 +63,8 @@
 	 1u << VIRTIO_NET_F_CTRL_RX	  |	\
 	 1u << VIRTIO_NET_F_CTRL_VLAN	  |	\
 	 1u << VIRTIO_NET_F_CSUM	  |	\
+	 1u << VIRTIO_NET_F_HOST_TSO4	  |	\
+	 1u << VIRTIO_NET_F_HOST_TSO6	  |	\
 	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
 	 1u << VIRTIO_RING_F_INDIRECT_DESC |    \
 	 1ULL << VIRTIO_F_VERSION_1)
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 0fa635a..4b01ea3 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -209,10 +209,117 @@ virtqueue_enqueue_recv_refill(struct virtqueue *vq, struct rte_mbuf *cookie)
 	return 0;
 }
 
+/* When doing TSO, the IP length is not included in the pseudo header
+ * checksum of the packet given to the PMD, but for virtio it is
+ * expected.
+ */
+static void
+virtio_tso_fix_cksum(struct rte_mbuf *m)
+{
+	/* common case: header is not fragmented */
+	if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len +
+			m->l4_len)) {
+		struct ipv4_hdr *iph;
+		struct ipv6_hdr *ip6h;
+		struct tcp_hdr *th;
+		uint16_t prev_cksum, new_cksum, ip_len, ip_paylen;
+		uint32_t tmp;
+
+		iph = rte_pktmbuf_mtod_offset(m, struct ipv4_hdr *, m->l2_len);
+		th = RTE_PTR_ADD(iph, m->l3_len);
+		if ((iph->version_ihl >> 4) == 4) {
+			iph->hdr_checksum = 0;
+			iph->hdr_checksum = rte_ipv4_cksum(iph);
+			ip_len = iph->total_length;
+			ip_paylen = rte_cpu_to_be_16(rte_be_to_cpu_16(ip_len) -
+				m->l3_len);
+		} else {
+			ip6h = (struct ipv6_hdr *)iph;
+			ip_paylen = ip6h->payload_len;
+		}
+
+		/* calculate the new phdr checksum not including ip_paylen */
+		prev_cksum = th->cksum;
+		tmp = prev_cksum;
+		tmp += ip_paylen;
+		tmp = (tmp & 0xffff) + (tmp >> 16);
+		new_cksum = tmp;
+
+		/* replace it in the packet */
+		th->cksum = new_cksum;
+	} else {
+		const struct ipv4_hdr *iph;
+		struct ipv4_hdr iph_copy;
+		union {
+			uint16_t u16;
+			uint8_t u8[2];
+		} prev_cksum, new_cksum, ip_len, ip_paylen, ip_csum;
+		uint32_t tmp;
+
+		/* Same code than above, but we use rte_pktmbuf_read()
+		 * or we read/write in mbuf data one byte at a time to
+		 * avoid issues if the packet is multi segmented.
+		 */
+
+		uint8_t ip_version;
+
+		ip_version = *rte_pktmbuf_mtod_offset(m, uint8_t *,
+			m->l2_len) >> 4;
+
+		/* calculate ip checksum (API imposes to set it to 0)
+		 * and get ip payload len */
+		if (ip_version == 4) {
+			*rte_pktmbuf_mtod_offset(m, uint8_t *,
+				m->l2_len + 10) = 0;
+			*rte_pktmbuf_mtod_offset(m, uint8_t *,
+				m->l2_len + 11) = 0;
+			iph = rte_pktmbuf_read(m, m->l2_len,
+				sizeof(*iph), &iph_copy);
+			ip_csum.u16 = rte_ipv4_cksum(iph);
+			*rte_pktmbuf_mtod_offset(m, uint8_t *,
+				m->l2_len + 10) = ip_csum.u8[0];
+			*rte_pktmbuf_mtod_offset(m, uint8_t *,
+				m->l2_len + 11) = ip_csum.u8[1];
+
+			ip_len.u8[0] = *rte_pktmbuf_mtod_offset(m, uint8_t *,
+				m->l2_len + 2);
+			ip_len.u8[1] = *rte_pktmbuf_mtod_offset(m, uint8_t *,
+				m->l2_len + 3);
+
+			ip_paylen.u16 = rte_cpu_to_be_16(
+				rte_be_to_cpu_16(ip_len.u16) - m->l3_len);
+		} else {
+			ip_paylen.u8[0] = *rte_pktmbuf_mtod_offset(m, uint8_t *,
+				m->l2_len + 4);
+			ip_paylen.u8[1] = *rte_pktmbuf_mtod_offset(m, uint8_t *,
+				m->l2_len + 5);
+		}
+
+		/* calculate the new phdr checksum not including ip_paylen */
+		/* get phdr cksum at offset 16 of TCP header */
+		prev_cksum.u8[0] = *rte_pktmbuf_mtod_offset(m, uint8_t *,
+			m->l2_len + m->l3_len + 16);
+		prev_cksum.u8[1] = *rte_pktmbuf_mtod_offset(m, uint8_t *,
+			m->l2_len + m->l3_len + 17);
+		tmp = prev_cksum.u16;
+		tmp += ip_paylen.u16;
+		tmp = (tmp & 0xffff) + (tmp >> 16);
+		new_cksum.u16 = tmp;
+
+		/* replace it in the packet */
+		*rte_pktmbuf_mtod_offset(m, uint8_t *,
+			m->l2_len + m->l3_len + 16) = new_cksum.u8[0];
+		*rte_pktmbuf_mtod_offset(m, uint8_t *,
+			m->l2_len + m->l3_len + 17) = new_cksum.u8[1];
+	}
+}
+
 static inline int
 tx_offload_enabled(struct virtio_hw *hw)
 {
-	return vtpci_with_feature(hw, VIRTIO_NET_F_CSUM);
+	return vtpci_with_feature(hw, VIRTIO_NET_F_CSUM) ||
+		vtpci_with_feature(hw, VIRTIO_NET_F_HOST_TSO4) ||
+		vtpci_with_feature(hw, VIRTIO_NET_F_HOST_TSO6);
 }
 
 static inline void
@@ -274,8 +381,11 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
 		idx = start_dp[idx].next;
 	}
 
+	/* Checksum Offload / TSO */
 	if (offload) {
-		/* Checksum Offload */
+		if (cookie->ol_flags & PKT_TX_TCP_SEG)
+			cookie->ol_flags |= PKT_TX_TCP_CKSUM;
+
 		switch (cookie->ol_flags & PKT_TX_L4_MASK) {
 		case PKT_TX_UDP_CKSUM:
 			hdr->csum_start = cookie->l2_len + cookie->l3_len;
@@ -297,9 +407,22 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
 			break;
 		}
 
-		hdr->gso_type = 0;
-		hdr->gso_size = 0;
-		hdr->hdr_len = 0;
+		/* TCP Segmentation Offload */
+		if (cookie->ol_flags & PKT_TX_TCP_SEG) {
+			virtio_tso_fix_cksum(cookie);
+			hdr->gso_type = (cookie->ol_flags & PKT_TX_IPV6) ?
+				VIRTIO_NET_HDR_GSO_TCPV6 :
+				VIRTIO_NET_HDR_GSO_TCPV4;
+			hdr->gso_size = cookie->tso_segsz;
+			hdr->hdr_len =
+				cookie->l2_len +
+				cookie->l3_len +
+				cookie->l4_len;
+		} else {
+			hdr->gso_type = 0;
+			hdr->gso_size = 0;
+			hdr->hdr_len = 0;
+		}
 	}
 
 	do {
-- 
2.8.1

^ permalink raw reply related

* Re: [PATCH v2 12/12] virtio: add Tso support
From: Yuanhan Liu @ 2016-10-13 14:16 UTC (permalink / raw)
  To: Olivier MATZ
  Cc: dev, konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang
In-Reply-To: <57FF9409.2090205@6wind.com>

On Thu, Oct 13, 2016 at 04:02:49PM +0200, Olivier MATZ wrote:
> 
> 
> On 10/13/2016 10:18 AM, Yuanhan Liu wrote:
> >On Mon, Oct 03, 2016 at 11:00:23AM +0200, Olivier Matz wrote:
> >>+/* When doing TSO, the IP length is not included in the pseudo header
> >>+ * checksum of the packet given to the PMD, but for virtio it is
> >>+ * expected.
> >>+ */
> >>+static void
> >>+virtio_tso_fix_cksum(struct rte_mbuf *m)
> >>+{
> >>+	/* common case: header is not fragmented */
> >>+	if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len +
> >>+			m->l4_len)) {
> >...
> >>+		/* replace it in the packet */
> >>+		th->cksum = new_cksum;
> >>+	} else {
> >...
> >>+		/* replace it in the packet */
> >>+		*rte_pktmbuf_mtod_offset(m, uint8_t *,
> >>+			m->l2_len + m->l3_len + 16) = new_cksum.u8[0];
> >>+		*rte_pktmbuf_mtod_offset(m, uint8_t *,
> >>+			m->l2_len + m->l3_len + 17) = new_cksum.u8[1];
> >>+	}
> >
> >The tcp header will always be in the mbuf, right? Otherwise, you can't
> >update the cksum field here. What's the point of introducing the "else
> >clause" then?
> 
> Sorry, I don't see the problem you're pointing out here.
> 
> What I want to solve here is to support the cases where the mbuf is
> segmented in the middle of the network header (which is probably a rare
> case).

How it's gonna segmented?

> 
> In the "else" part, I only access the mbuf byte by byte using the
> rte_pktmbuf_mtod_offset() accessor. An alternative would have been to copy
> the header in a linear buffer, fix the checksum, then copy it again in the
> packet, but there is no mbuf helpers to do these copies for now.

In the "else" clause, the ip header is still in the mbuf, right?
Why do you have to access it the way like:

	ip_version = *rte_pktmbuf_mtod_offset(m, uint8_t *,
		m->l2_len) >> 4;

Why can't you just use

	iph = rte_pktmbuf_mtod_offset(m, struct ipv4_hdr *, m->l2_len);
	iph->version_ihl ....;

Sorry, I'm just a bit lost.

	--yliu

^ permalink raw reply

* [PATCH v3 10/12] net/virtio: add Tx checksum offload support
From: Olivier Matz @ 2016-10-13 14:16 UTC (permalink / raw)
  To: dev, yuanhan.liu
  Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
	jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
	xiao.w.wang, maxime.coquelin
In-Reply-To: <1476368171-18176-1-git-send-email-olivier.matz@6wind.com>

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 drivers/net/virtio/virtio_ethdev.c |  7 ++++
 drivers/net/virtio/virtio_ethdev.h |  1 +
 drivers/net/virtio/virtio_rxtx.c   | 73 +++++++++++++++++++++++++++-----------
 3 files changed, 61 insertions(+), 20 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 00b4c38..c3c53be 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1579,6 +1579,13 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->rx_offload_capa =
 		DEV_RX_OFFLOAD_TCP_CKSUM |
 		DEV_RX_OFFLOAD_UDP_CKSUM;
+	dev_info->tx_offload_capa = 0;
+
+	if (hw->guest_features & (1ULL << VIRTIO_NET_F_CSUM)) {
+		dev_info->tx_offload_capa |=
+			DEV_TX_OFFLOAD_UDP_CKSUM |
+			DEV_TX_OFFLOAD_TCP_CKSUM;
+	}
 }
 
 /*
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index fd29a7f..adca6ba 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -62,6 +62,7 @@
 	 1u << VIRTIO_NET_F_CTRL_VQ	  |	\
 	 1u << VIRTIO_NET_F_CTRL_RX	  |	\
 	 1u << VIRTIO_NET_F_CTRL_VLAN	  |	\
+	 1u << VIRTIO_NET_F_CSUM	  |	\
 	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
 	 1u << VIRTIO_RING_F_INDIRECT_DESC |    \
 	 1ULL << VIRTIO_F_VERSION_1)
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index fc0d84b..675dc43 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -53,6 +53,8 @@
 #include <rte_cpuflags.h>
 #include <rte_net.h>
 #include <rte_ip.h>
+#include <rte_udp.h>
+#include <rte_tcp.h>
 
 #include "virtio_logs.h"
 #include "virtio_ethdev.h"
@@ -207,18 +209,27 @@ virtqueue_enqueue_recv_refill(struct virtqueue *vq, struct rte_mbuf *cookie)
 	return 0;
 }
 
+static inline int
+tx_offload_enabled(struct virtio_hw *hw)
+{
+	return vtpci_with_feature(hw, VIRTIO_NET_F_CSUM);
+}
+
 static inline void
 virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
 		       uint16_t needed, int use_indirect, int can_push)
 {
+	struct virtio_tx_region *txr = txvq->virtio_net_hdr_mz->addr;
 	struct vq_desc_extra *dxp;
 	struct virtqueue *vq = txvq->vq;
 	struct vring_desc *start_dp;
 	uint16_t seg_num = cookie->nb_segs;
 	uint16_t head_idx, idx;
 	uint16_t head_size = vq->hw->vtnet_hdr_size;
-	unsigned long offs;
+	struct virtio_net_hdr *hdr;
+	int offload;
 
+	offload = tx_offload_enabled(vq->hw);
 	head_idx = vq->vq_desc_head_idx;
 	idx = head_idx;
 	dxp = &vq->vq_descx[idx];
@@ -228,10 +239,12 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
 	start_dp = vq->vq_ring.desc;
 
 	if (can_push) {
-		/* put on zero'd transmit header (no offloads) */
-		void *hdr = rte_pktmbuf_prepend(cookie, head_size);
-
-		memset(hdr, 0, head_size);
+		/* prepend cannot fail, checked by caller */
+		hdr = (struct virtio_net_hdr *)
+			rte_pktmbuf_prepend(cookie, head_size);
+		/* if offload disabled, it is not zeroed below, do it now */
+		if (offload == 0)
+			memset(hdr, 0, head_size);
 	} else if (use_indirect) {
 		/* setup tx ring slot to point to indirect
 		 * descriptor list stored in reserved region.
@@ -239,14 +252,11 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
 		 * the first slot in indirect ring is already preset
 		 * to point to the header in reserved region
 		 */
-		struct virtio_tx_region *txr = txvq->virtio_net_hdr_mz->addr;
-
-		offs = idx * sizeof(struct virtio_tx_region)
-			+ offsetof(struct virtio_tx_region, tx_indir);
-
-		start_dp[idx].addr  = txvq->virtio_net_hdr_mem + offs;
+		start_dp[idx].addr  = txvq->virtio_net_hdr_mem +
+			RTE_PTR_DIFF(&txr[idx].tx_indir, txr);
 		start_dp[idx].len   = (seg_num + 1) * sizeof(struct vring_desc);
 		start_dp[idx].flags = VRING_DESC_F_INDIRECT;
+		hdr = (struct virtio_net_hdr *)&txr[idx].tx_hdr;
 
 		/* loop below will fill in rest of the indirect elements */
 		start_dp = txr[idx].tx_indir;
@@ -255,15 +265,43 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
 		/* setup first tx ring slot to point to header
 		 * stored in reserved region.
 		 */
-		offs = idx * sizeof(struct virtio_tx_region)
-			+ offsetof(struct virtio_tx_region, tx_hdr);
-
-		start_dp[idx].addr  = txvq->virtio_net_hdr_mem + offs;
+		start_dp[idx].addr  = txvq->virtio_net_hdr_mem +
+			RTE_PTR_DIFF(&txr[idx].tx_hdr, txr);
 		start_dp[idx].len   = vq->hw->vtnet_hdr_size;
 		start_dp[idx].flags = VRING_DESC_F_NEXT;
+		hdr = (struct virtio_net_hdr *)&txr[idx].tx_hdr;
+
 		idx = start_dp[idx].next;
 	}
 
+	if (offload) {
+		/* Checksum Offload */
+		switch (cookie->ol_flags & PKT_TX_L4_MASK) {
+		case PKT_TX_UDP_CKSUM:
+			hdr->csum_start = cookie->l2_len + cookie->l3_len;
+			hdr->csum_offset = offsetof(struct udp_hdr,
+				dgram_cksum);
+			hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
+			break;
+
+		case PKT_TX_TCP_CKSUM:
+			hdr->csum_start = cookie->l2_len + cookie->l3_len;
+			hdr->csum_offset = offsetof(struct tcp_hdr, cksum);
+			hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
+			break;
+
+		default:
+			hdr->csum_start = 0;
+			hdr->csum_offset = 0;
+			hdr->flags = 0;
+			break;
+		}
+
+		hdr->gso_type = 0;
+		hdr->gso_size = 0;
+		hdr->hdr_len = 0;
+	}
+
 	do {
 		start_dp[idx].addr  = VIRTIO_MBUF_DATA_DMA_ADDR(cookie, vq);
 		start_dp[idx].len   = cookie->data_len;
@@ -527,11 +565,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	if ((tx_conf->txq_flags & ETH_TXQ_FLAGS_NOXSUMS)
-	    != ETH_TXQ_FLAGS_NOXSUMS) {
-		PMD_INIT_LOG(ERR, "TX checksum offload not supported\n");
-		return -EINVAL;
-	}
 
 	virtio_update_rxtx_handler(dev, tx_conf);
 
-- 
2.8.1

^ permalink raw reply related

* Re: 17.02 Roadmap
From: Hunt, David @ 2016-10-13 14:18 UTC (permalink / raw)
  To: Thomas Monjalon, O'Driscoll, Tim; +Cc: dev
In-Reply-To: <1998191.9HGrB6oKr3@xps13>

Hi Thomas,


On 10/10/2016 9:42 PM, Thomas Monjalon wrote:
> Thanks Tim for the interesting inputs.
> Some of them may require a dedicated thread to continue the discussion
> based on some preliminary specifications or drafts.
>
> 2016-10-10 16:13, O'Driscoll, Tim:
>> Packet Distributor Enhancements: Enhancements will be made to the Packet Distributor library to improve performance:
>> 1. Introduce burst functionality to allow batches of packets to be sent to workers.
>> 2. Improve the performance of the flow/core affinity through the use of SSE/AVX instructions.
> The packet distributor looks more and more like a DPDK application
> at the same level of BESS, VPP, OVS, etc.

Lets discuss this further.  Would you see other libraries heading in 
this direction also (reorder, acl, hash, etc)?
Do you think it would be an idea to add as an item of discussion for the 
technical steering group when we're all at Userspace next week?

Regards,
Dave.

^ permalink raw reply

* [PATCH 0/3] Improvements in packet timestamps support
From: Oleg Kuporosov @ 2016-10-13 14:35 UTC (permalink / raw)
  To: dev


Hello DPDK Developers,

Financial Services Industry which is pretty eager for several DPDK
features especially low latency while high throughput. The major issue
so far for increasing DPDK adoption there is requirement for several
applications (capturers, some trading algorithms) to support of accurate
timestamping. The requirement mostly came from regulatory and customers
need strictly follow it.

Current state of timestamping support in DPDK looks pretty good:
 - there is API to enable/disable timestamping acquisition by 
   rte_eth_timesync_enable/disable
 - get timestamps itself by timesync_read_rx/tx_timestamp
 - and even implementation of NTP IEEE 1588 for time synchronization
   by rte_eth_timesync_adjust_read/write_time APIs.
   
But it misses the most important feature there – embedding timestamp
in rte_mbuf aligning it with packet.

We would like to change this to increase DPDK adoption for several new
DPDK-based applications for FSI segment. It also might be very
applicable for several RT media and debugging purposes of network
flows/streams in other segments like HPC.

There are several thoughts how to improve there:
 - include uint64_t timestamp field into rte_mbuf with minimal impact
   to throughput/latency. Keep it just simple uint64_t in ns (more than
   580 years) would be enough for immediate needs while using full
   struct timespec with twice bigger size would have much stronger
   performance impact as missed cacheline0. It is possible as there is
   6-bytes gap in 1st cacheline (fast path) and moving uint16_t
   vlan_tci_outer field to 2nd cacheline. 
 - such move will only impact for pretty rare usable VLAN RX stripping
   mode for outer TCI (it used only for one NIC i40e from the whole
   set and keep minimal performance impact for timestamps.  
 - PMD can fill the field in their callback completion routines
   depending on enabling this feature in runtime.

We evaluated other possible options but looks it will have even worse
performance impact.


Oleg Kuporosov (3):
  mbuf: embedding timestamp into the packet
  app/testpmd: enabled control for packet timestamps
  net/mlx5: implementation of Rx packet timestamping support

 app/test-pmd/cmdline.c                      | 122 +++++++++++++++
 app/test-pmd/parameters.c                   |   4 +
 app/test-pmd/testpmd.c                      |   5 +
 app/test-pmd/testpmd.h                      |   1 +
 doc/guides/testpmd_app_ug/run_app.rst       |   4 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   7 +
 drivers/net/mlx5/mlx5.c                     |   7 +-
 drivers/net/mlx5/mlx5.h                     |  10 +-
 drivers/net/mlx5/mlx5_defs.h                |   4 +
 drivers/net/mlx5/mlx5_ethdev.c              | 222 +++++++++++++++++++++++++++-
 drivers/net/mlx5/mlx5_rxq.c                 |   2 +
 drivers/net/mlx5/mlx5_rxtx.c                |  19 ++-
 drivers/net/mlx5/mlx5_rxtx.h                |   7 +-
 drivers/net/mlx5/mlx5_time.h                |  53 +++++++
 drivers/net/mlx5/mlx5_trigger.c             |   1 +
 lib/librte_eal/common/include/rte_time.h    |  45 ++++++
 lib/librte_mbuf/rte_mbuf.h                  |   6 +-
 17 files changed, 507 insertions(+), 12 deletions(-)
 create mode 100644 drivers/net/mlx5/mlx5_time.h

Thanks,
Oleg.
-- 
1.8.3.1

^ permalink raw reply

* [PATCH 1/3] mbuf: embedding timestamp into the packet
From: Oleg Kuporosov @ 2016-10-13 14:35 UTC (permalink / raw)
  To: dev
In-Reply-To: <1476369308-17021-1-git-send-email-olegk@mellanox.com>

The hard requirement of financial services industry is accurate
timestamping aligned with the packet itself. This patch is to satisfy
this requirement:

- include uint64_t timestamp field into rte_mbuf with minimal impact to
  throughput/latency. Keep it just simple uint64_t in ns (more than 580
  years) would be enough for immediate needs while using full
  struct timespec with twice bigger size would have much stronger
  performance impact as missed cacheline0.

- it is possible as there is 6-bytes gap in 1st cacheline (fast path)
  and moving uint16_t vlan_tci_outer field to 2nd cacheline.

- such move will only impact for pretty rare usable VLAN RX stripping
  mode for outer TCI (it used only for one NIC i40e from the whole set and
  allows to keep minimal performance impact for RX/TX timestamps.

Signed-off-by: Oleg Kuporosov <olegk@mellanox.com>
---
 lib/librte_mbuf/rte_mbuf.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 23b7bf8..1e1f2ed 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -851,8 +851,7 @@ struct rte_mbuf {
 
 	uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
 
-	/** Outer VLAN TCI (CPU order), valid if PKT_RX_QINQ_STRIPPED is set. */
-	uint16_t vlan_tci_outer;
+	uint64_t timestamp;       /**< Packet's timestamp, usually in ns */
 
 	/* second cache line - fields only used in slow path or on TX */
 	MARKER cacheline1 __rte_cache_min_aligned;
@@ -885,6 +884,9 @@ struct rte_mbuf {
 		};
 	};
 
+	/** Outer VLAN TCI (CPU order), valid if PKT_RX_QINQ_STRIPPED is set. */
+	uint16_t vlan_tci_outer;
+
 	/** Size of the application private data. In case of an indirect
 	 * mbuf, it stores the direct mbuf private data size. */
 	uint16_t priv_size;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 3/3] net/mlx5: implementation of Rx packet timestamping support
From: Oleg Kuporosov @ 2016-10-13 14:35 UTC (permalink / raw)
  To: dev
In-Reply-To: <1476369308-17021-1-git-send-email-olegk@mellanox.com>

Accurate RX timestaping is minimal requirement for several important
financial services industry requirement for several applications like
packet capture.

Support of periodic time synchronization with system time and adjusting
clock deviation was also added. We assume the system is synchronized by
PTP itself, so there is no links to PTP client implementation in DPDK.
Time synchronization is run by control thread by rte_alarm for each 5
seconds.  New synchronization object is copied to each configured RXQ
for TS calculation.

RX timestamp is calculated in nanoseconds and stored in rte_mbuf.
RX timestamp doesn't require additional API as simply can be
read from timestamp field of rte_mbuf.

TX timestamps were not added due to several reasons - there is no API yet,
issue with burst mode as TS will be provided only for the latest packet in
the burst and rte_mbuf will be discarded immediately after completion.

Enabling/disabling time synchronization DPDK API was added.

Signed-off-by: Oleg Kuporosov <olegk@mellanox.com>
---
 drivers/net/mlx5/mlx5.c                  |   7 +-
 drivers/net/mlx5/mlx5.h                  |  10 +-
 drivers/net/mlx5/mlx5_defs.h             |   4 +
 drivers/net/mlx5/mlx5_ethdev.c           | 222 ++++++++++++++++++++++++++++++-
 drivers/net/mlx5/mlx5_rxq.c              |   2 +
 drivers/net/mlx5/mlx5_rxtx.c             |  19 ++-
 drivers/net/mlx5/mlx5_rxtx.h             |   7 +-
 drivers/net/mlx5/mlx5_time.h             |  53 ++++++++
 drivers/net/mlx5/mlx5_trigger.c          |   1 +
 lib/librte_eal/common/include/rte_time.h |  45 +++++++
 10 files changed, 360 insertions(+), 10 deletions(-)
 create mode 100644 drivers/net/mlx5/mlx5_time.h

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 83bdf65..64dab28 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1,8 +1,8 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright 2015 6WIND S.A.
- *   Copyright 2015 Mellanox.
+ *   Copyright 2015-2016 6WIND S.A.
+ *   Copyright 2015-2016 Mellanox.
  *
  *   Redistribution and use in source and binary forms, with or without
  *   modification, are permitted provided that the following conditions
@@ -122,6 +122,7 @@ mlx5_dev_close(struct rte_eth_dev *dev)
 	      (void *)dev,
 	      ((priv->ctx != NULL) ? priv->ctx->device->name : ""));
 	/* In case mlx5_dev_stop() has not been called. */
+	mlx5_timesync_disable(dev);
 	priv_dev_interrupt_handler_uninstall(priv, dev);
 	priv_special_flow_disable_all(priv);
 	priv_mac_addrs_disable(priv);
@@ -219,6 +220,8 @@ static const struct eth_dev_ops mlx5_dev_ops = {
 	.rss_hash_update = mlx5_rss_hash_update,
 	.rss_hash_conf_get = mlx5_rss_hash_conf_get,
 	.filter_ctrl = mlx5_dev_filter_ctrl,
+	.timesync_enable = mlx5_timesync_enable,
+	.timesync_disable = mlx5_timesync_disable,
 };
 
 static struct {
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index d4fb5ff..49447c4 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1,8 +1,8 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright 2015 6WIND S.A.
- *   Copyright 2015 Mellanox.
+ *   Copyright 2015-2016 6WIND S.A.
+ *   Copyright 2015-2016 Mellanox.
  *
  *   Redistribution and use in source and binary forms, with or without
  *   modification, are permitted provided that the following conditions
@@ -54,6 +54,7 @@
 #ifdef PEDANTIC
 #pragma GCC diagnostic ignored "-pedantic"
 #endif
+#include <rte_alarm.h>
 #include <rte_ether.h>
 #include <rte_ethdev.h>
 #include <rte_spinlock.h>
@@ -64,6 +65,7 @@
 #endif
 
 #include "mlx5_utils.h"
+#include "mlx5_time.h"
 #include "mlx5_rxtx.h"
 #include "mlx5_autoconf.h"
 #include "mlx5_defs.h"
@@ -113,6 +115,7 @@ struct priv {
 	unsigned int mps:1; /* Whether multi-packet send is supported. */
 	unsigned int cqe_comp:1; /* Whether CQE compression is enabled. */
 	unsigned int pending_alarm:1; /* An alarm is pending. */
+	unsigned int timesync_en:1; /* Timesync (timestamping) enabled */
 	unsigned int txq_inline; /* Maximum packet size for inlining. */
 	unsigned int txqs_inline; /* Queue number threshold for inlining. */
 	/* RX/TX queues. */
@@ -120,6 +123,7 @@ struct priv {
 	unsigned int txqs_n; /* TX queues array size. */
 	struct rxq *(*rxqs)[]; /* RX queues. */
 	struct txq *(*txqs)[]; /* TX queues. */
+	struct mlx5_timesync timesync; /* time synronization object */
 	/* Indirection tables referencing all RX WQs. */
 	struct ibv_exp_rwq_ind_table *(*ind_tables)[];
 	unsigned int ind_tables_n; /* Number of indirection tables. */
@@ -203,6 +207,8 @@ int mlx5_set_link_up(struct rte_eth_dev *dev);
 struct priv *mlx5_secondary_data_setup(struct priv *priv);
 void priv_select_tx_function(struct priv *);
 void priv_select_rx_function(struct priv *);
+int mlx5_timesync_enable(struct rte_eth_dev *dev);
+int mlx5_timesync_disable(struct rte_eth_dev *dev);
 
 /* mlx5_mac.c */
 
diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h
index cc2a6f3..10b3f04 100644
--- a/drivers/net/mlx5/mlx5_defs.h
+++ b/drivers/net/mlx5/mlx5_defs.h
@@ -79,4 +79,8 @@
 /* Alarm timeout. */
 #define MLX5_ALARM_TIMEOUT_US 100000
 
+/* Clock deviation fix alarm timeout*/
+#define MLX5_ALARM_CLOCK_DEVIATION_US 5000000
+#define MLX5_CLOCK_DEVIATION_THRESHOLD 10
+
 #endif /* RTE_PMD_MLX5_DEFS_H_ */
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 264299a..16ba733 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -1,8 +1,8 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright 2015 6WIND S.A.
- *   Copyright 2015 Mellanox.
+ *   Copyright 2015-2016 6WIND S.A.
+ *   Copyright 2015-2016 Mellanox.
  *
  *   Redistribution and use in source and binary forms, with or without
  *   modification, are permitted provided that the following conditions
@@ -59,6 +59,7 @@
 #include <rte_interrupts.h>
 #include <rte_alarm.h>
 #include <rte_malloc.h>
+#include <rte_time.h>
 #ifdef PEDANTIC
 #pragma GCC diagnostic error "-pedantic"
 #endif
@@ -1412,3 +1413,220 @@ priv_select_rx_function(struct priv *priv)
 {
 	priv->dev->rx_pkt_burst = mlx5_rx_burst;
 }
+
+/**
+ * Synchronize with system time. We get the best (min) from 10 attempts
+ * to minimize shift from sys time and HCA clocks.
+ *
+ * @param ibv_context
+ *   Pointer to IB verbs context.
+ * @param st
+ *   Pointer to store system time.
+ * @param st_ns
+ *   Pointer to store system time represented in ns.
+ * @param hw_clock
+ *   Pointer to store HCA HW clock.
+ *
+ * @return
+ *   0 on success, -1 value on failure.
+ */
+static int
+mlx5_sync_clocks(struct ibv_context *ibv_ctx, struct timespec *st,
+		 volatile uint64_t *st_ns, volatile uint64_t *hw_clock)
+{
+	struct timespec st1, st2, diff, st_min = TIMESPEC_INITIALIZER;
+	struct ibv_exp_values query_val = {0};
+	int64_t interval, best_interval = 0;
+	uint64_t hw_clock_min = 0;
+
+	memset(&query_val, 0, sizeof(query_val));
+	query_val.comp_mask = IBV_EXP_VALUES_HW_CLOCK;
+	for (int i = 0 ; i < 10 ; ++i) {
+		clock_gettime(CLOCK_REALTIME, &st1);
+		if (ibv_exp_query_values(ibv_ctx, IBV_EXP_VALUES_HW_CLOCK,
+					 &query_val) || !query_val.hwclock)
+			return -1;
+		clock_gettime(CLOCK_REALTIME, &st2);
+		interval = (st2.tv_sec - st1.tv_sec) * NSEC_PER_SEC +
+			   (st2.tv_nsec - st1.tv_nsec);
+
+		if (!best_interval || interval < best_interval) {
+			best_interval = interval;
+			hw_clock_min = query_val.hwclock;
+
+			interval /= 2;
+			diff.tv_sec = interval / NSEC_PER_SEC;
+			diff.tv_nsec = interval - (diff.tv_sec * NSEC_PER_SEC);
+			rte_timespec_add(&st1, &diff, &st_min);
+		}
+	}
+	*st = st_min;
+	*st_ns = st->tv_sec * NSEC_PER_SEC + st->tv_nsec;
+	*hw_clock = hw_clock_min;
+	return 0;
+}
+
+/**
+ * Periodic function to run by rte_eal_alarm and to synchronize with system
+ * time and calculate HCA HW сlock deviation. The deviation will be included
+ * into timestamp calculation in RX/TX callbacks.
+ *
+ * @param arg
+ *   Void pointer to struct priv.
+ *
+ */
+static void
+mlx5_fix_hw_clock_deviation_handler(void *arg)
+{
+	struct priv *pv = arg;
+	struct timespec current_time, diff_systime;
+	uint64_t diff_hw_clock, hw_clock, estimated_hw_clock;
+	uint64_t systime_ns, diff_systime_ns;
+	int64_t clock_deviation_hw;
+	volatile struct mlx5_timestamp_sync *ts = &pv->timesync.sync_timestamp;
+
+	if (!ts->port_clock_frequency)
+		return;
+	if (mlx5_sync_clocks(pv->ctx, &current_time, &systime_ns, &hw_clock))
+		return;
+	/* time between current and previous time sync */
+	rte_timespec_sub(&current_time, &pv->timesync.sync_systime,
+			 &diff_systime);
+	/* also clocks */
+	diff_hw_clock = hw_clock - ts->sync_hw_clock;
+	diff_systime_ns = rte_timespec_to_ns(&diff_systime);
+	estimated_hw_clock = (diff_systime.tv_sec * ts->port_clock_frequency) +
+			     (diff_systime.tv_nsec * ts->port_clock_frequency /
+			     NSEC_PER_SEC);
+	clock_deviation_hw = estimated_hw_clock - diff_hw_clock;
+	priv_lock(pv);
+	if (abs(clock_deviation_hw) >= MLX5_CLOCK_DEVIATION_THRESHOLD) {
+		ts->port_clock_frequency = (diff_hw_clock * NSEC_PER_SEC) /
+					   diff_systime_ns;
+		ts->mskd_duration = (NSEC_PER_SEC << 30) /
+				    ts->port_clock_frequency;
+	}
+	ts->sync_hw_clock = hw_clock;
+	ts->sync_time_ns = systime_ns;
+	pv->timesync.sync_systime = current_time;
+	DEBUG("%ld.%09ld since last fix, time_ns: %lu estimate_hw_clock = %ld,"
+	      "diff_hw_clock = %ld, deviation = %ld, freq = %ld durat: %lu",
+	      diff_systime.tv_sec, diff_systime.tv_nsec, systime_ns,
+	      estimated_hw_clock, diff_hw_clock, clock_deviation_hw,
+	      ts->port_clock_frequency, ts->mskd_duration);
+	/* update all queues */
+	for (uint32_t i = 0; i != pv->rxqs_n; i++) {
+		struct rxq *rxq = (*pv->rxqs)[i];
+
+		if (rxq == NULL)
+			continue;
+		rxq->timestamps_enabled = pv->timesync_en;
+		rxq->timesync = *ts;
+	}
+	priv_unlock(pv);
+	rte_eal_alarm_set(MLX5_ALARM_CLOCK_DEVIATION_US,
+			  mlx5_fix_hw_clock_deviation_handler,
+			  (void *)pv);
+}
+
+/**
+ * Return HCA port clock frequency in Hz.
+ *
+ * @param priv
+ *   Pointer to private structure.
+ */
+static uint64_t
+mlx5_get_port_clock_frequency(struct priv *pv)
+{
+	struct ibv_exp_device_attr exp_device_attr;
+	exp_device_attr.comp_mask = IBV_EXP_DEVICE_ATTR_WITH_HCA_CORE_CLOCK;
+	if (ibv_exp_query_device(pv->ctx, &exp_device_attr)) {
+		ERROR("ibv_exp_query_device() failed");
+		return 0;
+	}
+	return exp_device_attr.hca_core_clock * 1000; /* orig in KHz */
+}
+
+/**
+ *  DPDK callback to enable timestamping. rte_mbuf.timestamp will hold
+ *  value of the packet in ns.
+ *
+ * @param dev
+ *   Pointer to Ethernet device structure.
+ *
+ * @return
+ *   0 on success, errno value on failure.
+ */
+int
+mlx5_timesync_enable(struct rte_eth_dev *dev)
+{
+	struct priv *pv = dev->data->dev_private;
+	struct mlx5_timesync *tso = &pv->timesync;
+	volatile struct mlx5_timestamp_sync *sts = &tso->sync_timestamp;
+
+	priv_lock(pv);
+	sts->port_clock_frequency = mlx5_get_port_clock_frequency(pv);
+	if (!sts->port_clock_frequency) {
+		pv->timesync_en = 0;
+		INFO("Timesync disabled: %d as port clock frequency is 0",
+		     pv->timesync_en);
+		priv_unlock(pv);
+		return -ENOTSUP;
+	}
+	INFO("port %u Clock frequency: %lu Hz", pv->port,
+	     sts->port_clock_frequency);
+	if (mlx5_sync_clocks(pv->ctx, &tso->sync_systime, &sts->sync_time_ns,
+			     &sts->sync_hw_clock)) {
+		pv->timesync_en = 0;
+		INFO("Timesync disabled: %d", pv->timesync_en);
+		priv_unlock(pv);
+		return -ENOTSUP;
+	}
+	sts->mskd_duration = (NSEC_PER_SEC << 30) / sts->port_clock_frequency;
+	pv->timesync_en = 1;
+	DEBUG("%p: Timesync enabled, masked duration: %lu", (void *)dev,
+	      sts->mskd_duration);
+	rte_eal_alarm_set(1000,
+			  mlx5_fix_hw_clock_deviation_handler,
+			  pv);
+	DEBUG("%p: sync_systime: %lu.%lu, time_ns: %lu sync_hw_clock: %lu",
+	      (void *)dev, tso->sync_systime.tv_sec,
+	      tso->sync_systime.tv_nsec, sts->sync_time_ns, sts->sync_hw_clock);
+	/* update all queues */
+	for (uint32_t i = 0; i != pv->rxqs_n; i++) {
+		struct rxq *rxq = (*pv->rxqs)[i];
+
+		if (rxq == NULL)
+			continue;
+		rxq->timestamps_enabled = pv->timesync_en;
+		rxq->timesync = *sts;
+	}
+	priv_unlock(pv);
+	return 0;
+}
+
+/**
+ *  DPDK callback to disable timestamping. Value of rte_mbuf.timestamp is
+ *  undefined.
+ *
+ * @param dev
+ *   Pointer to Ethernet device structure.
+ *
+ * @return
+ *   0 on success, errno value on failure.
+ */
+int
+mlx5_timesync_disable(struct rte_eth_dev *dev)
+{
+	struct priv *pv = dev->data->dev_private;
+
+	pv->timesync_en = 0;
+	rte_eal_alarm_cancel(mlx5_fix_hw_clock_deviation_handler, pv);
+	for (uint32_t i = 0; i != pv->rxqs_n; i++) {
+		struct rxq *rxq = (*pv->rxqs)[i];
+		if (rxq == NULL)
+			continue;
+		rxq->timestamps_enabled = pv->timesync_en;
+	}
+	return 0;
+}
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index d32ad68..b9a5fe6 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -1258,6 +1258,8 @@ mlx5_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 		rte_free(rxq_ctrl);
 	else {
 		rxq_ctrl->rxq.stats.idx = idx;
+		rxq_ctrl->rxq.timestamps_enabled = priv->timesync_en;
+		rxq_ctrl->rxq.timesync = priv->timesync.sync_timestamp;
 		DEBUG("%p: adding RX queue %p to list",
 		      (void *)dev, (void *)rxq_ctrl);
 		(*priv->rxqs)[idx] = &rxq_ctrl->rxq;
diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 91b0131..54f284d 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -1,8 +1,8 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright 2015 6WIND S.A.
- *   Copyright 2015 Mellanox.
+ *   Copyright 2015-2016 6WIND S.A.
+ *   Copyright 2015-2016 Mellanox.
  *
  *   Redistribution and use in source and binary forms, with or without
  *   modification, are permitted provided that the following conditions
@@ -59,6 +59,7 @@
 #include <rte_branch_prediction.h>
 #include <rte_ether.h>
 #include <rte_vect.h>
+#include <rte_time.h>
 #ifdef PEDANTIC
 #pragma GCC diagnostic error "-pedantic"
 #endif
@@ -1385,6 +1386,20 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 					len -= ETHER_CRC_LEN;
 			}
 			PKT_LEN(pkt) = len;
+			/* Calculate synchronized timestamp in ns */
+			if (unlikely(rxq->timestamps_enabled)) {
+				volatile struct mlx5_timestamp_sync *tso =
+					&rxq->timesync;
+				uint64_t clock_diff;
+				rte_prefetch0(tso);
+				clock_diff = ntohll(cqe->timestamp) -
+					tso->sync_hw_clock;
+				clock_diff = (clock_diff * tso->mskd_duration)
+					>> 30;
+				pkt->timestamp = tso->sync_time_ns +
+					clock_diff;
+				pkt->ol_flags |= PKT_RX_IEEE1588_TMST;
+			}
 		}
 		DATA_LEN(rep) = DATA_LEN(seg);
 		PKT_LEN(rep) = PKT_LEN(seg);
diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 844cabc..2ff873c 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -1,8 +1,8 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright 2015 6WIND S.A.
- *   Copyright 2015 Mellanox.
+ *   Copyright 2015-2016 6WIND S.A.
+ *   Copyright 2015-2016 Mellanox.
  *
  *   Redistribution and use in source and binary forms, with or without
  *   modification, are permitted provided that the following conditions
@@ -60,6 +60,7 @@
 #endif
 
 #include "mlx5_utils.h"
+#include "mlx5_time.h"
 #include "mlx5.h"
 #include "mlx5_autoconf.h"
 #include "mlx5_defs.h"
@@ -109,6 +110,7 @@ struct rxq {
 	unsigned int csum_l2tun:1; /* Same for L2 tunnels. */
 	unsigned int vlan_strip:1; /* Enable VLAN stripping. */
 	unsigned int crc_present:1; /* CRC must be subtracted. */
+	unsigned int timestamps_enabled:1; /* timestamping enabled */
 	unsigned int sges_n:2; /* Log 2 of SGEs (max buffers per packet). */
 	unsigned int cqe_n:4; /* Log 2 of CQ elements. */
 	unsigned int elts_n:4; /* Log 2 of Mbufs. */
@@ -125,6 +127,7 @@ struct rxq {
 	struct rte_mbuf *(*elts)[];
 	struct rte_mempool *mp;
 	struct mlx5_rxq_stats stats;
+	volatile struct mlx5_timestamp_sync timesync; /* per queue copy */
 } __rte_cache_aligned;
 
 /* RX queue control descriptor. */
diff --git a/drivers/net/mlx5/mlx5_time.h b/drivers/net/mlx5/mlx5_time.h
new file mode 100644
index 0000000..b44bcb1
--- /dev/null
+++ b/drivers/net/mlx5/mlx5_time.h
@@ -0,0 +1,53 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright 2016 Mellanox.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of 6WIND S.A. nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef RTE_PMD_MLX5_TIME_H_
+#define RTE_PMD_MLX5_TIME_H_
+
+#include <stddef.h>
+#include <stdint.h>
+#include <limits.h>
+
+
+struct mlx5_timestamp_sync {
+	uint64_t sync_hw_clock; /* the last HW clocks */
+	uint64_t sync_time_ns; /* the last system time in ns */
+	uint64_t mskd_duration; /* adjusted masked duration */
+	uint64_t port_clock_frequency; /* in Hz */
+};
+
+struct mlx5_timesync {
+	volatile struct mlx5_timestamp_sync sync_timestamp;
+	struct timespec sync_systime; /* the last system time */
+};
+
+#endif /* RTE_PMD_MLX5_TIME_H_ */
diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index e9b9a29..ff94e6b 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -116,6 +116,7 @@ mlx5_dev_stop(struct rte_eth_dev *dev)
 		return;
 	}
 	DEBUG("%p: cleaning up and destroying hash RX queues", (void *)dev);
+	mlx5_timesync_disable(dev);
 	priv_special_flow_disable_all(priv);
 	priv_mac_addrs_disable(priv);
 	priv_destroy_hash_rxqs(priv);
diff --git a/lib/librte_eal/common/include/rte_time.h b/lib/librte_eal/common/include/rte_time.h
index 28c6274..7fa8e64 100644
--- a/lib/librte_eal/common/include/rte_time.h
+++ b/lib/librte_eal/common/include/rte_time.h
@@ -37,6 +37,8 @@
 #include <stdint.h>
 #include <time.h>
 
+#define TIMESPEC_INITIALIZER    {0, 0}
+
 #define NSEC_PER_SEC             1000000000L
 
 /**
@@ -127,4 +129,47 @@ rte_ns_to_timespec(uint64_t nsec)
 	return ts;
 }
 
+/**
+ * Addition of two timespec times to result.
+ *
+ * @param a
+ *    Pointer to the first time
+ * @param b
+ *    Pointer to the second time
+ * @param res
+ *    Pointer to result
+ *
+ */
+static inline void rte_timespec_add(struct timespec *a, struct timespec *b,
+	struct timespec *res)
+{
+	res->tv_sec = a->tv_sec + b->tv_sec;
+	res->tv_nsec = a->tv_nsec + b->tv_nsec;
+	if (res->tv_nsec >= NSEC_PER_SEC) {
+		++res->tv_sec;
+		res->tv_nsec -= NSEC_PER_SEC;
+	}
+}
+
+/**
+ * Substruction of the first timespec by second to result.
+ *
+ * @param a
+ *    Pointer to the first time
+ * @param b
+ *    Pointer to the second time
+ * @param res
+ *    Pointer to result
+ */
+static inline void rte_timespec_sub(struct timespec *a, struct timespec *b,
+	struct timespec *res)
+{
+	res->tv_sec = a->tv_sec - b->tv_sec;
+	res->tv_nsec = a->tv_nsec - b->tv_nsec;
+	if (res->tv_nsec < 0) {
+		--res->tv_sec;
+		res->tv_nsec += NSEC_PER_SEC;
+	}
+}
+
 #endif /* _RTE_TIME_H_ */
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 2/3] app/testpmd: enabled control for packet timestamps
From: Oleg Kuporosov @ 2016-10-13 14:35 UTC (permalink / raw)
  To: dev
In-Reply-To: <1476369308-17021-1-git-send-email-olegk@mellanox.com>

Implemented two methods of control

- by --enable-timestamps CL testpmd application we can enable timestamping
  for all ports;
- in interactive mode port config <port> timestamps on|off is able to
  configure timestamping per specific port.

The control doesn't interact with IEEE1588 PTP implementation there as
it is under macro compilation but can be extended in the future.

This feature is required for debugging/testing purposes for real time HW
packet timestamping.

Signed-off-by: Oleg Kuporosov <olegk@mellanox.com>
---
 app/test-pmd/cmdline.c                      | 122 ++++++++++++++++++++++++++++
 app/test-pmd/parameters.c                   |   4 +
 app/test-pmd/testpmd.c                      |   5 ++
 app/test-pmd/testpmd.h                      |   1 +
 doc/guides/testpmd_app_ug/run_app.rst       |   4 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   7 ++
 6 files changed, 143 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 17d238f..9b202ce 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -561,6 +561,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Set crc-strip/scatter/rx-checksum/hardware-vlan/drop_en"
 			" for ports.\n\n"
 
+			"port config (port_id|all) timestamps (on|off)\n"
+			"    Enable/disable packet timestamps.\n\n"
+
 			"port config all rss (all|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none)\n"
 			"    Set the RSS mode.\n\n"
 
@@ -10188,6 +10191,123 @@ cmdline_parse_inst_t cmd_config_l2_tunnel_en_dis_specific = {
 	},
 };
 
+/* Configure port timestamping */
+struct cmd_config_timestamps_result {
+	cmdline_fixed_string_t port;
+	cmdline_fixed_string_t config;
+	cmdline_fixed_string_t all;
+	uint8_t id;
+	cmdline_fixed_string_t timestamps;
+	cmdline_fixed_string_t mode;
+};
+
+cmdline_parse_token_string_t cmd_config_timestamps_port =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_config_timestamps_result,
+		 port, "port");
+cmdline_parse_token_string_t cmd_config_timestamps_config =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_config_timestamps_result,
+		 config, "config");
+cmdline_parse_token_string_t cmd_config_timestamps_all_str =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_config_timestamps_result,
+		 all, "all");
+cmdline_parse_token_num_t cmd_config_timestamps_id =
+	TOKEN_NUM_INITIALIZER
+		(struct cmd_config_timestamps_result,
+		 id, UINT8);
+cmdline_parse_token_string_t cmd_config_timestamps_ts_str =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_config_timestamps_result,
+		timestamps, "timestamps");
+cmdline_parse_token_string_t cmd_config_timestamps_path =
+	TOKEN_STRING_INITIALIZER
+		(struct cmd_config_timestamps_result,
+		mode, "on#off");
+
+/* enable/disable timestamps (on/off) for a port */
+static void
+cmd_config_timestamps_all_parsed(
+	void *parsed_result,
+	__attribute__((unused)) struct cmdline *cl,
+	__attribute__((unused)) void *data)
+{
+	struct cmd_config_timestamps_result *res = parsed_result;
+	portid_t pid;
+	uint8_t mode = 0;
+
+	if (!strcmp("on", res->mode))
+		mode = 1;
+	else if (!strcmp("off", res->mode))
+		mode = 0;
+	else {
+		printf("Unknown timestamps mode parameter\n");
+		return;
+	}
+	FOREACH_PORT(pid, ports) {
+		if (mode)
+			rte_eth_timesync_enable(pid);
+		else
+			rte_eth_timesync_disable(pid);
+	}
+}
+
+cmdline_parse_inst_t cmd_config_timestamps_all = {
+	.f = cmd_config_timestamps_all_parsed,
+	.data = NULL,
+	.help_str = "port config all timestamps on|off",
+	.tokens = {
+		(void *)&cmd_config_timestamps_port,
+		(void *)&cmd_config_timestamps_config,
+		(void *)&cmd_config_timestamps_all_str,
+		(void *)&cmd_config_timestamps_ts_str,
+		(void *)&cmd_config_timestamps_path,
+		NULL,
+	},
+};
+
+/* enable/disable timestamps (rx/tx/both) for a port */
+static void
+cmd_config_timestamps_specific_parsed(
+	void *parsed_result,
+	__attribute__((unused)) struct cmdline *cl,
+	__attribute__((unused)) void *data)
+{
+	struct cmd_config_timestamps_result *res =
+		parsed_result;
+	uint8_t mode = 0;
+
+	if (port_id_is_invalid(res->id, ENABLED_WARN))
+		return;
+	if (!strcmp("on", res->mode))
+		mode = 1;
+	else if (!strcmp("off", res->mode))
+		mode = 0;
+	else {
+		printf("Unknown timestamps mode parameter\n");
+		return;
+	}
+	if (mode)
+		rte_eth_timesync_enable(res->id);
+	else
+		rte_eth_timesync_disable(res->id);
+}
+
+cmdline_parse_inst_t cmd_config_timestamps_specific = {
+	.f = cmd_config_timestamps_specific_parsed,
+	.data = NULL,
+	.help_str = "port config <port> timestamps on|off",
+	.tokens = {
+		(void *)&cmd_config_timestamps_port,
+		(void *)&cmd_config_timestamps_config,
+		(void *)&cmd_config_timestamps_id,
+		(void *)&cmd_config_timestamps_ts_str,
+		(void *)&cmd_config_timestamps_path,
+		NULL,
+	},
+};
+
 /* E-tag configuration */
 
 /* Common result structure for all E-tag configuration */
@@ -10739,6 +10859,8 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_config_e_tag_forwarding_en_dis,
 	(cmdline_parse_inst_t *)&cmd_config_e_tag_filter_add,
 	(cmdline_parse_inst_t *)&cmd_config_e_tag_filter_del,
+	(cmdline_parse_inst_t *)&cmd_config_timestamps_all,
+	(cmdline_parse_inst_t *)&cmd_config_timestamps_specific,
 	NULL,
 };
 
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 6a6a07e..f15cd5a 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -150,6 +150,7 @@ usage(char* progname)
 	       "By default drop-queue=127.\n");
 	printf("  --crc-strip: enable CRC stripping by hardware.\n");
 	printf("  --enable-rx-cksum: enable rx hardware checksum offload.\n");
+	printf("  --enable-timestamps: enable rx hardware timestamp.\n");
 	printf("  --disable-hw-vlan: disable hardware vlan.\n");
 	printf("  --disable-hw-vlan-filter: disable hardware vlan filter.\n");
 	printf("  --disable-hw-vlan-strip: disable hardware vlan strip.\n");
@@ -525,6 +526,7 @@ launch_args_parse(int argc, char** argv)
 		{ "pkt-filter-drop-queue",      1, 0, 0 },
 		{ "crc-strip",                  0, 0, 0 },
 		{ "enable-rx-cksum",            0, 0, 0 },
+		{ "enable-timestamps",          0, 0, 0 },
 		{ "enable-scatter",             0, 0, 0 },
 		{ "disable-hw-vlan",            0, 0, 0 },
 		{ "disable-hw-vlan-filter",     0, 0, 0 },
@@ -768,6 +770,8 @@ launch_args_parse(int argc, char** argv)
 				rx_mode.enable_scatter = 1;
 			if (!strcmp(lgopts[opt_idx].name, "enable-rx-cksum"))
 				rx_mode.hw_ip_checksum = 1;
+			if (!strcmp(lgopts[opt_idx].name, "enable-timestamps"))
+				timestamps_enabled = 1;
 
 			if (!strcmp(lgopts[opt_idx].name, "disable-hw-vlan")) {
 				rx_mode.hw_vlan_filter = 0;
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e2403c3..ee76282 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -189,6 +189,8 @@ uint8_t dcb_config = 0;
 /* Whether the dcb is in testing status */
 uint8_t dcb_test = 0;
 
+/**< Enabling runtime packet timestamps by CL: --enable-timestamps */
+uint8_t timestamps_enabled = 0;
 /*
  * Configurable number of RX/TX queues.
  */
@@ -1851,6 +1853,9 @@ init_port_config(void)
 
 		rte_eth_macaddr_get(pid, &port->eth_addr);
 
+		if (timestamps_enabled)
+			rte_eth_timesync_enable(pid);
+
 		map_port_queue_stats_mapping_registers(pid, port);
 #ifdef RTE_NIC_BYPASS
 		rte_eth_dev_bypass_init(pid);
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 2b281cc..489661e 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -353,6 +353,7 @@ extern int32_t txq_flags;
 
 extern uint8_t dcb_config;
 extern uint8_t dcb_test;
+extern uint8_t timestamps_enabled; /**< Timestamps by --enable-timestamps */
 extern enum dcb_queue_mapping_mode dcb_q_mapping;
 
 extern uint16_t mbuf_data_size; /**< Mbuf data space size. */
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 7712bd2..fb56846 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -313,6 +313,10 @@ The commandline options are:
 
     Enable per-queue packet drop for packets with no descriptors.
 
+*   ``--enable-timestamps``
+
+    Enable timesync and per packet timestamping for all ports.
+
 *   ``--disable-rss``
 
     Disable RSS (Receive Side Scaling).
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index f87e0c2..ed84c6d 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1312,6 +1312,13 @@ Set the DCB mode for an individual port::
 
 The traffic class should be 4 or 8.
 
+port config - Timesync/timestamping
+~~~~~~~~~~~~~~~~~~~
+
+Enable/disable time synhronzation/packet timestamping for specific port or all ports::
+
+   testpmd> port config (port_id|all) timestamps (on|off)
+
 port config - Burst
 ~~~~~~~~~~~~~~~~~~~
 
-- 
1.8.3.1

^ permalink raw reply related

* Re: 17.02 Roadmap
From: Thomas Monjalon @ 2016-10-13 14:39 UTC (permalink / raw)
  To: Hunt, David; +Cc: O'Driscoll, Tim, dev
In-Reply-To: <c92322c9-a8a4-8a9b-54b9-6fc1b403f986@intel.com>

2016-10-13 15:18, Hunt, David:
> Hi Thomas,
> 
> On 10/10/2016 9:42 PM, Thomas Monjalon wrote:
> > 2016-10-10 16:13, O'Driscoll, Tim:
> >> Packet Distributor Enhancements: Enhancements will be made to the Packet Distributor library to improve performance:
> >> 1. Introduce burst functionality to allow batches of packets to be sent to workers.
> >> 2. Improve the performance of the flow/core affinity through the use of SSE/AVX instructions.
> > 
> > The packet distributor looks more and more like a DPDK application
> > at the same level of BESS, VPP, OVS, etc.
> 
> Lets discuss this further.  Would you see other libraries heading in 
> this direction also (reorder, acl, hash, etc)?

Not so easy to put things in a category.

> Do you think it would be an idea to add as an item of discussion for the 
> technical steering group when we're all at Userspace next week?

If others are interested to discuss about libraries categories and growth,
yes I'll jump in.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox