* Recall: [PATCH 1/1] net/i40e: enable auto link update for XXV710
From: Zhang, Qi Z @ 2016-11-24 8:35 UTC (permalink / raw)
To: Wu, Jingjing, Zhang, Helin; +Cc: dev@dpdk.org, root
Zhang, Qi Z would like to recall the message, "[PATCH 1/1] net/i40e: enable auto link update for XXV710".
^ permalink raw reply
* [PATCH 3/5] mbuf: new helper to write data in a mbuf chain
From: Olivier Matz @ 2016-11-24 8:56 UTC (permalink / raw)
To: dev, yuanhan.liu; +Cc: maxime.coquelin, huawei.xie, stephen
In-Reply-To: <1479977798-13417-1-git-send-email-olivier.matz@6wind.com>
Introduce a new helper to write data in a chain of mbufs,
spreading it in the segments.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
app/test/test_mbuf.c | 21 +++++++++++++++
lib/librte_mbuf/rte_mbuf.c | 44 +++++++++++++++++++++++++++++++
lib/librte_mbuf/rte_mbuf.h | 50 ++++++++++++++++++++++++++++++++++++
lib/librte_mbuf/rte_mbuf_version.map | 6 +++++
4 files changed, 121 insertions(+)
diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index 7656a4d..5f1bc5d 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -335,6 +335,10 @@ testclone_testupdate_testdetach(void)
struct rte_mbuf *clone2 = NULL;
struct rte_mbuf *m2 = NULL;
unaligned_uint32_t *data;
+ uint32_t magic = MAGIC_DATA;
+ uint32_t check_data[2];
+
+ memset(check_data, 0, sizeof(check_data));
/* alloc a mbuf */
m = rte_pktmbuf_alloc(pktmbuf_pool);
@@ -421,6 +425,8 @@ testclone_testupdate_testdetach(void)
if (m2 == NULL)
GOTO_FAIL("cannot allocate m2");
rte_pktmbuf_append(m2, sizeof(uint32_t));
+ if (rte_pktmbuf_write(m2, 0, sizeof(uint32_t), &magic) < 0)
+ GOTO_FAIL("cannot write data in m2");
rte_pktmbuf_chain(m2, clone);
clone = NULL;
@@ -430,6 +436,21 @@ testclone_testupdate_testdetach(void)
rte_pktmbuf_pkt_len(m2) - sizeof(uint32_t)) == 0)
GOTO_FAIL("m2 data should be marked as shared");
+ /* check data content */
+ data = rte_pktmbuf_read(m2, 0, sizeof(uint32_t), check_data);
+ if (data == NULL)
+ GOTO_FAIL("cannot read data");
+ if (*data != MAGIC_DATA)
+ GOTO_FAIL("invalid data");
+ if (data == check_data)
+ GOTO_FAIL("data should not have been copied");
+ data = rte_pktmbuf_read(m2, 0, sizeof(uint32_t) * 2, check_data);
+ if (data == NULL)
+ GOTO_FAIL("cannot read data");
+ if (data[0] != MAGIC_DATA || data[1] != MAGIC_DATA)
+ GOTO_FAIL("invalid data");
+ if (data != check_data)
+ GOTO_FAIL("data should have been copied");
/* free mbuf */
rte_pktmbuf_free(m);
m = NULL;
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index b31958e..ed56193 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -298,6 +298,50 @@ void *__rte_pktmbuf_read(const struct rte_mbuf *m, uint32_t off,
return buf;
}
+/* write len data bytes in a mbuf at specified offset (internal) */
+int
+__rte_pktmbuf_write(const struct rte_mbuf *m, uint32_t off,
+ uint32_t len, const void *buf)
+{
+ const struct rte_mbuf *seg = m;
+ uint32_t buf_off = 0, copy_len;
+ char *dst;
+
+ if (off + len > rte_pktmbuf_pkt_len(m))
+ return -1;
+
+ while (off >= rte_pktmbuf_data_len(seg)) {
+ off -= rte_pktmbuf_data_len(seg);
+ seg = seg->next;
+ }
+
+ dst = rte_pktmbuf_mtod_offset(seg, char *, off);
+ if (buf == dst)
+ return 0;
+
+ if (off + len <= rte_pktmbuf_data_len(seg)) {
+ RTE_ASSERT(!rte_pktmbuf_is_shared(seg));
+ rte_memcpy(dst, buf, len);
+ return 0;
+ }
+
+ /* copy data in several segments */
+ while (len > 0) {
+ RTE_ASSERT(!rte_pktmbuf_is_shared(seg));
+ copy_len = rte_pktmbuf_data_len(seg) - off;
+ if (copy_len > len)
+ copy_len = len;
+ dst = rte_pktmbuf_mtod_offset(seg, char *, off);
+ rte_memcpy(dst, (const char *)buf + buf_off, copy_len);
+ off = 0;
+ buf_off += copy_len;
+ len -= copy_len;
+ seg = seg->next;
+ }
+
+ return 0;
+}
+
/*
* Get the name of a RX offload flag. Must be kept synchronized with flag
* definitions in rte_mbuf.h.
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index cd77a56..e898d25 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1678,6 +1678,56 @@ static inline void *rte_pktmbuf_read(const struct rte_mbuf *m,
}
/**
+ * @internal used by rte_pktmbuf_write().
+ */
+int __rte_pktmbuf_write(const struct rte_mbuf *m, uint32_t off,
+ uint32_t len, const void *buf);
+
+/**
+ * Write len data bytes in a mbuf at specified offset.
+ *
+ * If the mbuf is contiguous between off and off+len, rte_memcpy() is
+ * called. Else, it will split the data in the segments.
+ *
+ * The caller must ensure that all destination segments are writable
+ * (not shared).
+ *
+ * If the destination pointer in the mbuf is the same than the source
+ * buffer, the function do nothing and is successful.
+ *
+ * If the mbuf is too small, the function fails.
+ *
+ * @param m
+ * The pointer to the mbuf.
+ * @param off
+ * The offset of the data in the mbuf.
+ * @param len
+ * The amount of bytes to read.
+ * @param buf
+ * The buffer where data is copied if it is not contiguous in mbuf
+ * data. Its length should be at least equal to the len parameter.
+ * @return
+ * - (0) on success (data is updated in the mbuf)
+ * - (-1) on error: mbuf is too small or not writable
+ */
+static inline int rte_pktmbuf_write(const struct rte_mbuf *m,
+ uint32_t off, uint32_t len, const void *buf)
+{
+ char *dst = rte_pktmbuf_mtod_offset(m, char *, off);
+
+ if (buf == dst)
+ return 0;
+
+ if (off + len <= rte_pktmbuf_data_len(m)) {
+ RTE_ASSERT(!rte_pktmbuf_is_shared(m));
+ rte_memcpy(dst, buf, len);
+ return 0;
+ }
+
+ return __rte_pktmbuf_write(m, off, len, buf);
+}
+
+/**
* Chain an mbuf to another, thereby creating a segmented packet.
*
* Note: The implementation will do a linear walk over the segments to find
diff --git a/lib/librte_mbuf/rte_mbuf_version.map b/lib/librte_mbuf/rte_mbuf_version.map
index 6e2ea84..b2c057e 100644
--- a/lib/librte_mbuf/rte_mbuf_version.map
+++ b/lib/librte_mbuf/rte_mbuf_version.map
@@ -35,3 +35,9 @@ DPDK_16.11 {
rte_get_tx_ol_flag_list;
} DPDK_2.1;
+
+DPDK_17.02 {
+ global:
+
+ __rte_pktmbuf_write;
+} DPDK_16.11;
--
2.8.1
^ permalink raw reply related
* [PATCH 4/5] mbuf: new helper to copy data from a mbuf
From: Olivier Matz @ 2016-11-24 8:56 UTC (permalink / raw)
To: dev, yuanhan.liu; +Cc: maxime.coquelin, huawei.xie, stephen
In-Reply-To: <1479977798-13417-1-git-send-email-olivier.matz@6wind.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
app/test/test_mbuf.c | 7 +++++++
lib/librte_mbuf/rte_mbuf.h | 32 +++++++++++++++++++++++++++++++-
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index 5f1bc5d..73fd7df 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -451,6 +451,13 @@ testclone_testupdate_testdetach(void)
GOTO_FAIL("invalid data");
if (data != check_data)
GOTO_FAIL("data should have been copied");
+ if (rte_pktmbuf_read_copy(m2, 0, sizeof(uint32_t), check_data) < 0)
+ GOTO_FAIL("cannot copy data");
+ if (check_data[0] != MAGIC_DATA)
+ GOTO_FAIL("invalid data");
+ if (data != check_data)
+ GOTO_FAIL("data should have been copied");
+
/* free mbuf */
rte_pktmbuf_free(m);
m = NULL;
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index e898d25..edae89f 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1643,7 +1643,7 @@ static inline int rte_pktmbuf_data_is_shared(const struct rte_mbuf *m,
}
/**
- * @internal used by rte_pktmbuf_read().
+ * @internal used by rte_pktmbuf_read() and rte_pktmbuf_read_copy().
*/
void *__rte_pktmbuf_read(const struct rte_mbuf *m, uint32_t off,
uint32_t len, void *buf);
@@ -1728,6 +1728,36 @@ static inline int rte_pktmbuf_write(const struct rte_mbuf *m,
}
/**
+ * Copy data from a mbuf into a linear buffer
+ *
+ * @param m
+ * The pointer to the mbuf.
+ * @param off
+ * The offset of the data in the mbuf.
+ * @param len
+ * The amount of bytes to copy.
+ * @param buf
+ * The buffer where data is copied, it should be at least
+ * as large as len.
+ * @return
+ * - (0) on success
+ * - (-1) on error: mbuf is too small
+ */
+static inline int rte_pktmbuf_read_copy(const struct rte_mbuf *m,
+ uint32_t off, uint32_t len, void *buf)
+{
+ if (likely(off + len <= rte_pktmbuf_data_len(m))) {
+ rte_memcpy(buf, rte_pktmbuf_mtod_offset(m, char *, off), len);
+ return 0;
+ }
+
+ if (__rte_pktmbuf_read(m, off, len, buf) == NULL)
+ return -1;
+
+ return 0;
+}
+
+/**
* Chain an mbuf to another, thereby creating a segmented packet.
*
* Note: The implementation will do a linear walk over the segments to find
--
2.8.1
^ permalink raw reply related
* [PATCH 0/5] virtio/mbuf: fix virtio tso with shared mbufs
From: Olivier Matz @ 2016-11-24 8:56 UTC (permalink / raw)
To: dev, yuanhan.liu; +Cc: maxime.coquelin, huawei.xie, stephen
This patchset fixes the transmission of cloned mbufs when using
virtio + TSO. The problem is we need to fix the L4 checksum in the
packet, but it should be considered as read-only, as pointed-out
by Stephen here:
http://dpdk.org/ml/archives/dev/2016-October/048873.html
Unfortunatly the patchset is quite big, but I did not manage to
find a shorter solution. The first patches add some mbuf helpers
that are used in virtio in the last patch.
This last patch adds a zone for each tx ring entry where headers
can be copied, patched, and referenced by virtio descriptors in
case the mbuf is read-only. If its not the case, the mbuf is
modified as before.
I tested with the same test plan than the one described in
http://dpdk.org/ml/archives/dev/2016-October/048092.html
(only the TSO test case).
I also replayed the test with the following patches to validate
the code path for:
- segmented packets (it forces a local copy in virtio_tso_fix_cksum)
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -279,7 +279,7 @@ void *__rte_pktmbuf_read(const struct rte_mbuf *m, uint32_t off,
seg = seg->next;
}
- if (off + len <= rte_pktmbuf_data_len(seg))
+ if (0 && off + len <= rte_pktmbuf_data_len(seg))
return rte_pktmbuf_mtod_offset(seg, char *, off);
/* rare case: header is split among several segments */
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 9dc6f10..5a4312a 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1671,7 +1671,7 @@ void *__rte_pktmbuf_read(const struct rte_mbuf *m, uint32_t off,
static inline void *rte_pktmbuf_read(const struct rte_mbuf *m,
uint32_t off, uint32_t len, void *buf)
{
- if (likely(off + len <= rte_pktmbuf_data_len(m)))
+ if (likely(0 && off + len <= rte_pktmbuf_data_len(m)))
return rte_pktmbuf_mtod_offset(m, char *, off);
else
return __rte_pktmbuf_read(m, off, len, buf);
- and for shared mbuf (force to use the buffer in virtio tx ring)
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -225,7 +225,7 @@ virtio_tso_fix_cksum(struct rte_mbuf *m, char *hdr, size_t hdr_sz)
int shared = 0;
/* mbuf is write-only, we need to copy the headers in a linear buffer */
- if (unlikely(rte_pktmbuf_data_is_shared(m, 0, hdrlen))) {
+ if (unlikely(1 || rte_pktmbuf_data_is_shared(m, 0, hdrlen))) {
shared = 1;
/* network headers are too big, there's nothing we can do */
Olivier Matz (5):
mbuf: remove const attribute in mbuf read function
mbuf: new helper to check if a mbuf is shared
mbuf: new helper to write data in a mbuf chain
mbuf: new helper to copy data from a mbuf
net/virtio: fix Tso when mbuf is shared
app/test/test_mbuf.c | 62 +++++++++++++-
drivers/net/virtio/virtio_rxtx.c | 119 ++++++++++++++++++--------
drivers/net/virtio/virtqueue.h | 2 +
lib/librte_mbuf/rte_mbuf.c | 46 +++++++++-
lib/librte_mbuf/rte_mbuf.h | 157 ++++++++++++++++++++++++++++++++++-
lib/librte_mbuf/rte_mbuf_version.map | 6 ++
6 files changed, 347 insertions(+), 45 deletions(-)
--
2.8.1
^ permalink raw reply related
* [PATCH 1/5] mbuf: remove const attribute in mbuf read function
From: Olivier Matz @ 2016-11-24 8:56 UTC (permalink / raw)
To: dev, yuanhan.liu; +Cc: maxime.coquelin, huawei.xie, stephen
In-Reply-To: <1479977798-13417-1-git-send-email-olivier.matz@6wind.com>
There is no good reason to have this const attribute: rte_pktmbuf_read()
returns a pointer which is either in a private buffer, or in the mbuf.
In the first case, it is clearly not const. In the second case, it is up
to the user to check that the mbuf is not shared and that data can be
modified.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
lib/librte_mbuf/rte_mbuf.c | 2 +-
lib/librte_mbuf/rte_mbuf.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 63f43c8..b31958e 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -265,7 +265,7 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m, unsigned dump_len)
}
/* read len data bytes in a mbuf at specified offset (internal) */
-const void *__rte_pktmbuf_read(const struct rte_mbuf *m, uint32_t off,
+void *__rte_pktmbuf_read(const struct rte_mbuf *m, uint32_t off,
uint32_t len, void *buf)
{
const struct rte_mbuf *seg = m;
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index ead7c6e..14956f6 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1576,7 +1576,7 @@ static inline int rte_pktmbuf_is_contiguous(const struct rte_mbuf *m)
/**
* @internal used by rte_pktmbuf_read().
*/
-const void *__rte_pktmbuf_read(const struct rte_mbuf *m, uint32_t off,
+void *__rte_pktmbuf_read(const struct rte_mbuf *m, uint32_t off,
uint32_t len, void *buf);
/**
@@ -1599,7 +1599,7 @@ const void *__rte_pktmbuf_read(const struct rte_mbuf *m, uint32_t off,
* The pointer to the data, either in the mbuf if it is contiguous,
* or in the user buffer. If mbuf is too small, NULL is returned.
*/
-static inline const void *rte_pktmbuf_read(const struct rte_mbuf *m,
+static inline void *rte_pktmbuf_read(const struct rte_mbuf *m,
uint32_t off, uint32_t len, void *buf)
{
if (likely(off + len <= rte_pktmbuf_data_len(m)))
--
2.8.1
^ permalink raw reply related
* [PATCH 5/5] net/virtio: fix Tso when mbuf is shared
From: Olivier Matz @ 2016-11-24 8:56 UTC (permalink / raw)
To: dev, yuanhan.liu; +Cc: maxime.coquelin, huawei.xie, stephen
In-Reply-To: <1479977798-13417-1-git-send-email-olivier.matz@6wind.com>
With virtio, doing tso requires to modify the network
packet data:
- the dpdk API requires to set the l4 checksum to an
Intel-Nic-like pseudo header checksum that does
not include the ip length
- the virtio peer expects that the l4 checksum is
a standard pseudo header checksum.
This is a problem with shared packets, because they
should not be modified.
This patch fixes this issue by copying the headers into
a linear buffer in that case. This buffer is located in
the virtio_tx_region, at the same place where the
virtio header is stored.
The size of this buffer is set to 256, which should
be enough in all cases:
sizeof(ethernet) + sizeof(vlan) * 2 + sizeof(ip6)
sizeof(ip6-ext) + sizeof(tcp) + sizeof(tcp-opts)
= 14 + 8 + 40 + sizeof(ip6-ext) + 40 + sizeof(tcp-opts)
= 102 + sizeof(ip6-ext) + sizeof(tcp-opts)
Fixes: 696573046e9e ("net/virtio: support TSO")
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/virtio/virtio_rxtx.c | 119 +++++++++++++++++++++++++++------------
drivers/net/virtio/virtqueue.h | 2 +
2 files changed, 85 insertions(+), 36 deletions(-)
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 22d97a4..577c775 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -211,43 +211,73 @@ virtqueue_enqueue_recv_refill(struct virtqueue *vq, struct rte_mbuf *cookie)
/* 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.
+ * expected. Fix the mbuf or a copy if the mbuf is shared.
*/
-static void
-virtio_tso_fix_cksum(struct rte_mbuf *m)
+static unsigned int
+virtio_tso_fix_cksum(struct rte_mbuf *m, char *hdr, size_t hdr_sz)
{
- /* 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;
+ struct ipv4_hdr *iph, iph_copy;
+ struct ipv6_hdr *ip6h = NULL, ip6h_copy;
+ struct tcp_hdr *th, th_copy;
+ size_t hdrlen = m->l2_len + m->l3_len + m->l4_len;
+ uint16_t prev_cksum, new_cksum, ip_len, ip_paylen;
+ uint32_t tmp;
+ int shared = 0;
+
+ /* mbuf is write-only, we need to copy the headers in a linear buffer */
+ if (unlikely(rte_pktmbuf_data_is_shared(m, 0, hdrlen))) {
+ shared = 1;
+
+ /* network headers are too big, there's nothing we can do */
+ if (hdrlen > hdr_sz)
+ return 0;
+
+ rte_pktmbuf_read_copy(m, 0, hdrlen, hdr);
+ iph = (struct ipv4_hdr *)(hdr + m->l2_len);
+ ip6h = (struct ipv6_hdr *)(hdr + m->l2_len);
+ th = (struct tcp_hdr *)(hdr + m->l2_len + m->l3_len);
+ } else {
+ iph = rte_pktmbuf_read(m, m->l2_len, sizeof(*iph), &iph_copy);
+ th = rte_pktmbuf_read(m, m->l2_len + m->l3_len, sizeof(*th),
+ &th_copy);
+ }
+
+ 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 {
+ if (!shared) {
+ ip6h = rte_pktmbuf_read(m, m->l2_len, sizeof(*ip6h),
+ &ip6h_copy);
}
+ 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;
+ /* 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;
- }
+ /* replace it in the header */
+ th->cksum = new_cksum;
+
+ /* the update was done in the linear buffer, return */
+ if (shared)
+ return hdrlen;
+
+ /* copy from local buffer into mbuf if required */
+ if ((iph->version_ihl >> 4) == 4)
+ rte_pktmbuf_write(m, m->l2_len, sizeof(*iph), iph);
+ else
+ rte_pktmbuf_write(m, m->l2_len, sizeof(*ip6h), ip6h);
+ rte_pktmbuf_write(m, m->l2_len + m->l3_len, sizeof(*th), th);
+
+ return 0;
}
static inline int
@@ -268,7 +298,9 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
struct vring_desc *start_dp;
uint16_t seg_num = cookie->nb_segs;
uint16_t head_idx, idx;
+ uint16_t hdr_idx = 0;
uint16_t head_size = vq->hw->vtnet_hdr_size;
+ unsigned int offset = 0;
struct virtio_net_hdr *hdr;
int offload;
@@ -303,6 +335,8 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
/* loop below will fill in rest of the indirect elements */
start_dp = txr[idx].tx_indir;
+ hdr_idx = 0;
+ start_dp[hdr_idx].len = vq->hw->vtnet_hdr_size;
idx = 1;
} else {
/* setup first tx ring slot to point to header
@@ -313,7 +347,7 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
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;
-
+ hdr_idx = idx;
idx = start_dp[idx].next;
}
@@ -345,7 +379,14 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
/* TCP Segmentation Offload */
if (cookie->ol_flags & PKT_TX_TCP_SEG) {
- virtio_tso_fix_cksum(cookie);
+ offset = virtio_tso_fix_cksum(cookie,
+ RTE_PTR_ADD(hdr, start_dp[hdr_idx].len),
+ VIRTIO_MAX_HDR_SZ);
+ if (offset > 0) {
+ RTE_ASSERT(can_push != 0);
+ start_dp[hdr_idx].len += offset;
+ }
+
hdr->gso_type = (cookie->ol_flags & PKT_TX_IPV6) ?
VIRTIO_NET_HDR_GSO_TCPV6 :
VIRTIO_NET_HDR_GSO_TCPV4;
@@ -362,10 +403,16 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
}
do {
- start_dp[idx].addr = VIRTIO_MBUF_DATA_DMA_ADDR(cookie, vq);
- start_dp[idx].len = cookie->data_len;
+ if (offset > cookie->data_len) {
+ offset -= cookie->data_len;
+ continue;
+ }
+ start_dp[idx].addr = VIRTIO_MBUF_DATA_DMA_ADDR(cookie, vq) +
+ offset;
+ start_dp[idx].len = cookie->data_len - offset;
start_dp[idx].flags = cookie->next ? VRING_DESC_F_NEXT : 0;
idx = start_dp[idx].next;
+ offset = 0;
} while ((cookie = cookie->next) != NULL);
if (use_indirect)
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index f0bb089..edfe0dd 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -254,8 +254,10 @@ struct virtio_net_hdr_mrg_rxbuf {
/* Region reserved to allow for transmit header and indirect ring */
#define VIRTIO_MAX_TX_INDIRECT 8
+#define VIRTIO_MAX_HDR_SZ 256
struct virtio_tx_region {
struct virtio_net_hdr_mrg_rxbuf tx_hdr;
+ char net_headers[VIRTIO_MAX_HDR_SZ]; /* for offload if mbuf is RO */
struct vring_desc tx_indir[VIRTIO_MAX_TX_INDIRECT]
__attribute__((__aligned__(16)));
};
--
2.8.1
^ permalink raw reply related
* [PATCH 2/5] mbuf: new helper to check if a mbuf is shared
From: Olivier Matz @ 2016-11-24 8:56 UTC (permalink / raw)
To: dev, yuanhan.liu; +Cc: maxime.coquelin, huawei.xie, stephen
In-Reply-To: <1479977798-13417-1-git-send-email-olivier.matz@6wind.com>
Introduce 2 new helpers rte_pktmbuf_seg_is_shared() and
rte_pktmbuf_data_is_shared() to check if the packet data inside
a mbuf is shared (and shall not be modified).
To avoid a "discards const qualifier" error, add a const to the argument
of rte_mbuf_from_indirect().
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
app/test/test_mbuf.c | 34 +++++++++++++++++++---
lib/librte_mbuf/rte_mbuf.h | 71 +++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 100 insertions(+), 5 deletions(-)
diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index c0823ea..7656a4d 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -333,6 +333,7 @@ testclone_testupdate_testdetach(void)
struct rte_mbuf *m = NULL;
struct rte_mbuf *clone = NULL;
struct rte_mbuf *clone2 = NULL;
+ struct rte_mbuf *m2 = NULL;
unaligned_uint32_t *data;
/* alloc a mbuf */
@@ -384,6 +385,11 @@ testclone_testupdate_testdetach(void)
if (*data != MAGIC_DATA)
GOTO_FAIL("invalid data in clone->next\n");
+ if (rte_pktmbuf_seg_is_shared(m) == 0)
+ GOTO_FAIL("m should be marked as shared\n");
+ if (rte_pktmbuf_seg_is_shared(clone) == 0)
+ GOTO_FAIL("clone should be marked as shared\n");
+
if (rte_mbuf_refcnt_read(m) != 2)
GOTO_FAIL("invalid refcnt in m\n");
@@ -410,14 +416,32 @@ testclone_testupdate_testdetach(void)
if (rte_mbuf_refcnt_read(m->next) != 3)
GOTO_FAIL("invalid refcnt in m->next\n");
+ /* prepend data to one of the clone */
+ m2 = rte_pktmbuf_alloc(pktmbuf_pool);
+ if (m2 == NULL)
+ GOTO_FAIL("cannot allocate m2");
+ rte_pktmbuf_append(m2, sizeof(uint32_t));
+ rte_pktmbuf_chain(m2, clone);
+ clone = NULL;
+
+ if (rte_pktmbuf_data_is_shared(m2, 0, sizeof(uint32_t)))
+ GOTO_FAIL("m2 headers should not be marked as shared");
+ if (rte_pktmbuf_data_is_shared(m2, sizeof(uint32_t),
+ rte_pktmbuf_pkt_len(m2) - sizeof(uint32_t)) == 0)
+ GOTO_FAIL("m2 data should be marked as shared");
+
/* free mbuf */
rte_pktmbuf_free(m);
- rte_pktmbuf_free(clone);
- rte_pktmbuf_free(clone2);
-
m = NULL;
- clone = NULL;
+ rte_pktmbuf_free(m2);
+ m2 = NULL;
+
+ if (rte_pktmbuf_seg_is_shared(clone2))
+ GOTO_FAIL("clone2 should not be marked as shared\n");
+
+ rte_pktmbuf_free(clone2);
clone2 = NULL;
+
printf("%s ok\n", __func__);
return 0;
@@ -428,6 +452,8 @@ testclone_testupdate_testdetach(void)
rte_pktmbuf_free(clone);
if (clone2)
rte_pktmbuf_free(clone2);
+ if (m2)
+ rte_pktmbuf_free(m2);
return -1;
}
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 14956f6..cd77a56 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -576,7 +576,7 @@ rte_mbuf_data_dma_addr_default(const struct rte_mbuf *mb)
* The address of the direct mbuf corresponding to buffer_addr.
*/
static inline struct rte_mbuf *
-rte_mbuf_from_indirect(struct rte_mbuf *mi)
+rte_mbuf_from_indirect(const struct rte_mbuf *mi)
{
return (struct rte_mbuf *)RTE_PTR_SUB(mi->buf_addr, sizeof(*mi) + mi->priv_size);
}
@@ -1574,6 +1574,75 @@ static inline int rte_pktmbuf_is_contiguous(const struct rte_mbuf *m)
}
/**
+ * Test if a mbuf segment is shared
+ *
+ * Return true if the data embedded in this segment is shared by several
+ * mbufs. In this case, the mbuf data should be considered as read-only.
+ *
+ * @param m
+ * The packet mbuf.
+ * @return
+ * - (1), the mbuf segment is shared (read-only)
+ * - (0), the mbuf segment is not shared (writable)
+ */
+static inline int rte_pktmbuf_seg_is_shared(const struct rte_mbuf *m)
+{
+ if (rte_mbuf_refcnt_read(m) > 1)
+ return 1;
+
+ if (RTE_MBUF_INDIRECT(m) &&
+ rte_mbuf_refcnt_read(rte_mbuf_from_indirect(m)) > 1)
+ return 1;
+
+ return 0;
+}
+
+/**
+ * Test if some data in an mbuf chain is shared
+ *
+ * Return true if the specified data area in the mbuf chain is shared by
+ * several mbufs. In this case, this data should be considered as
+ * read-only.
+ *
+ * If the area described by off and len exceeds the bounds of the mbuf
+ * chain (off + len <= rte_pktmbuf_pkt_len()), the exceeding part of the
+ * area is ignored.
+ *
+ * @param m
+ * The packet mbuf.
+ * @return
+ * - (1), the mbuf data is shared (read-only)
+ * - (0), the mbuf data is not shared (writable)
+ */
+static inline int rte_pktmbuf_data_is_shared(const struct rte_mbuf *m,
+ uint32_t off, uint32_t len)
+{
+ const struct rte_mbuf *seg = m;
+
+ if (likely(off + len <= rte_pktmbuf_data_len(seg)))
+ return rte_pktmbuf_seg_is_shared(seg);
+
+ while (seg->next && off >= rte_pktmbuf_data_len(seg)) {
+ off -= rte_pktmbuf_data_len(seg);
+ seg = seg->next;
+ }
+
+ if (off + len <= rte_pktmbuf_data_len(seg))
+ return rte_pktmbuf_seg_is_shared(seg);
+
+ while (seg->next && len > 0) {
+ if (rte_pktmbuf_seg_is_shared(seg))
+ return 1;
+
+ len -= (rte_pktmbuf_data_len(seg) - off);
+ off = 0;
+ seg = seg->next;
+ }
+
+ return 0;
+}
+
+/**
* @internal used by rte_pktmbuf_read().
*/
void *__rte_pktmbuf_read(const struct rte_mbuf *m, uint32_t off,
--
2.8.1
^ permalink raw reply related
* Re: Proposal for a new Committer model
From: Thomas Monjalon @ 2016-11-24 9:17 UTC (permalink / raw)
To: Neil Horman; +Cc: Ferruh Yigit, dev, Mcnamara, John
In-Reply-To: <20161123201330.GD6961@hmsreliant.think-freely.org>
2016-11-23 15:13, Neil Horman:
> Can either you or thomas provide some detail as to how you are doing patch
> management between trees (details of the commands you use are what I would be
> interested in). It sounds to me like there may be some optimization to be made
> here before we even make changes to the subtrees.
Until now, we preferred avoiding merge commits.
That's why I rebase sub-trees to integrate them in the mainline.
As Ferruh explained, it does not require more work because sub-trees are
regularly rebased anyway.
And I use a script to keep original committer name and date.
Hope it's clear now
^ permalink raw reply
* Re: dpdk/vpp and cross-version migration for vhost
From: Kevin Traynor @ 2016-11-24 9:30 UTC (permalink / raw)
To: Yuanhan Liu, Michael S. Tsirkin
Cc: Maxime Coquelin, dev, Stephen Hemminger, qemu-devel, libvir-list,
vpp-dev, Marc-André Lureau
In-Reply-To: <20161124063129.GE5048@yliu-dev.sh.intel.com>
On 11/24/2016 06:31 AM, Yuanhan Liu wrote:
> On Tue, Nov 22, 2016 at 04:53:05PM +0200, Michael S. Tsirkin wrote:
>>>> You keep assuming that you have the VM started first and
>>>> figure out things afterwards, but this does not work.
>>>>
>>>> Think about a cluster of machines. You want to start a VM in
>>>> a way that will ensure compatibility with all hosts
>>>> in a cluster.
>>>
>>> I see. I was more considering about the case when the dst
>>> host (including the qemu and dpdk combo) is given, and
>>> then determine whether it will be a successfull migration
>>> or not.
>>>
>>> And you are asking that we need to know which host could
>>> be a good candidate before starting the migration. In such
>>> case, we indeed need some inputs from both the qemu and
>>> vhost-user backend.
>>>
>>> For DPDK, I think it could be simple, just as you said, it
>>> could be either a tiny script, or even a macro defined in
>>> the source code file (we extend it every time we add a
>>> new feature) to let the libvirt to read it. Or something
>>> else.
>>
>> There's the issue of APIs that tweak features as Maxime
>> suggested.
>
> Yes, it's a good point.
>
>> Maybe the only thing to do is to deprecate it,
>
> Looks like so.
>
>> but I feel some way for application to pass info into
>> guest might be benefitial.
>
> The two APIs are just for tweaking feature bits DPDK supports before
> any device got connected. It's another way to disable some features
> (the another obvious way is to through QEMU command lines).
>
> IMO, it's bit handy only in a case like: we have bunch of VMs. Instead
> of disabling something though qemu one by one, we could disable it
> once in DPDK.
>
> But I doubt the useful of it. It's only used in DPDK's vhost example
> after all. Nor is it used in vhost pmd, neither is it used in OVS.
rte_vhost_feature_disable() is currently used in OVS, lib/netdev-dpdk.c
netdev_dpdk_vhost_class_init(void)
{
static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
/* This function can be called for different classes. The
initialization
* needs to be done only once */
if (ovsthread_once_start(&once)) {
rte_vhost_driver_callback_register(&virtio_net_device_ops);
rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_HOST_TSO4
| 1ULL << VIRTIO_NET_F_HOST_TSO6
| 1ULL << VIRTIO_NET_F_CSUM);
>
>>>> If you don't, guest visible interface will change
>>>> and you won't be able to migrate.
>>>>
>>>> It does not make sense to discuss feature bits specifically
>>>> since that is not the only part of interface.
>>>> For example, max ring size supported might change.
>>>
>>> I don't quite understand why we have to consider the max ring
>>> size here? Isn't it a virtio device attribute, that QEMU could
>>> provide such compatibility information?
>>>
>>> I mean, DPDK is supposed to support vary vring size, it's QEMU
>>> to give a specifc value.
>>
>> If backend supports s/g of any size up to 2^16, there's no issue.
>
> I don't know others, but I see no issues in DPDK.
>
>> ATM some backends might be assuming up to 1K s/g since
>> QEMU never supported bigger ones. We might classify this
>> as a bug, or not and add a feature flag.
>>
>> But it's just an example. There might be more values at issue
>> in the future.
>
> Yeah, maybe. But we could analysis it one by one.
>
>>>> Let me describe how it works in qemu/libvirt.
>>>> When you install a VM, you can specify compatibility
>>>> level (aka "machine type"), and you can query the supported compatibility
>>>> levels. Management uses that to find the supported compatibility
>>>> and stores the compatibility in XML that is migrated with the VM.
>>>> There's also a way to find the latest level which is the
>>>> default unless overridden by user, again this level
>>>> is recorded and then
>>>> - management can make sure migration destination is compatible
>>>> - management can avoid migration to hosts without that support
>>>
>>> Thanks for the info, it helps.
>>>
>>> ...
>>>>>>>> As version here is an opaque string for libvirt and qemu,
>>>>>>>> anything can be used - but I suggest either a list
>>>>>>>> of values defining the interface, e.g.
>>>>>>>> any_layout=on,max_ring=256
>>>>>>>> or a version including the name and vendor of the backend,
>>>>>>>> e.g. "org.dpdk.v4.5.6".
>>>
>>> The version scheme may not be ideal here. Assume a QEMU is supposed
>>> to work with a specific DPDK version, however, user may disable some
>>> newer features through qemu command line, that it also could work with
>>> an elder DPDK version. Using the version scheme will not allow us doing
>>> such migration to an elder DPDK version. The MTU is a lively example
>>> here? (when MTU feature is provided by QEMU but is actually disabled
>>> by user, that it could also work with an elder DPDK without MTU support).
>>>
>>> --yliu
>>
>> OK, so does a list of values look better to you then?
>
> Yes, if there are no better way.
>
> And I think it may be better to not list all those features, literally.
> But instead, using the number should be better, say, features=0xdeadbeef.
>
> Listing the feature names means we have to come to an agreement in all
> components involved here (QEMU, libvirt, DPDK, VPP, and maybe more
> backends), that we have to use the exact same feature names. Though it
> may not be a big deal, it lacks some flexibility.
>
> A feature bits will not have this issue.
>
> --yliu
>
>>
>>
>>>>>>>>
>>>>>>>> Note that typically the list of supported versions can only be
>>>>>>>> extended, not shrunk. Also, if the host/guest interface
>>>>>>>> does not change, don't change the current version as
>>>>>>>> this just creates work for everyone.
>>>>>>>>
>>>>>>>> Thoughts? Would this work well for management? dpdk? vpp?
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>>
>>>>>>>> --
>>>>>>>> MST
^ permalink raw reply
* [RFC 2/9] ethdev: move queue id check in generic layer
From: Olivier Matz @ 2016-11-24 9:54 UTC (permalink / raw)
To: dev; +Cc: thomas.monjalon, konstantin.ananyev, wenzhuo.lu, helin.zhang
In-Reply-To: <1479981261-19512-1-git-send-email-olivier.matz@6wind.com>
The check of queue_id is done in all drivers implementing
rte_eth_rx_queue_count(). Factorize this check in the generic function.
Note that the nfp driver was doing the check differently, which could
induce crashes if the queue index was too big.
By the way, also move the is_supported test before the port valid and
queue valid test.
PR=52423
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Ivan Boule <ivan.boule@6wind.com>
---
drivers/net/e1000/em_rxtx.c | 5 -----
drivers/net/e1000/igb_rxtx.c | 5 -----
drivers/net/i40e/i40e_rxtx.c | 5 -----
drivers/net/ixgbe/ixgbe_rxtx.c | 5 -----
drivers/net/nfp/nfp_net.c | 6 ------
lib/librte_ether/rte_ethdev.h | 6 ++++--
6 files changed, 4 insertions(+), 28 deletions(-)
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index 41f51c0..c1c724b 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -1390,11 +1390,6 @@ eth_em_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
struct em_rx_queue *rxq;
uint32_t desc = 0;
- if (rx_queue_id >= dev->data->nb_rx_queues) {
- PMD_RX_LOG(DEBUG, "Invalid RX queue_id=%d", rx_queue_id);
- return 0;
- }
-
rxq = dev->data->rx_queues[rx_queue_id];
rxdp = &(rxq->rx_ring[rxq->rx_tail]);
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index dbd37ac..e9aa356 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -1512,11 +1512,6 @@ eth_igb_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
struct igb_rx_queue *rxq;
uint32_t desc = 0;
- if (rx_queue_id >= dev->data->nb_rx_queues) {
- PMD_RX_LOG(ERR, "Invalid RX queue id=%d", rx_queue_id);
- return 0;
- }
-
rxq = dev->data->rx_queues[rx_queue_id];
rxdp = &(rxq->rx_ring[rxq->rx_tail]);
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 7ae7d9f..79a72f0 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1793,11 +1793,6 @@ i40e_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
struct i40e_rx_queue *rxq;
uint16_t desc = 0;
- if (unlikely(rx_queue_id >= dev->data->nb_rx_queues)) {
- PMD_DRV_LOG(ERR, "Invalid RX queue id %u", rx_queue_id);
- return 0;
- }
-
rxq = dev->data->rx_queues[rx_queue_id];
rxdp = &(rxq->rx_ring[rxq->rx_tail]);
while ((desc < rxq->nb_rx_desc) &&
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index b2d9f45..1a8ea5f 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2857,11 +2857,6 @@ ixgbe_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
struct ixgbe_rx_queue *rxq;
uint32_t desc = 0;
- if (rx_queue_id >= dev->data->nb_rx_queues) {
- PMD_RX_LOG(ERR, "Invalid RX queue id=%d", rx_queue_id);
- return 0;
- }
-
rxq = dev->data->rx_queues[rx_queue_id];
rxdp = &(rxq->rx_ring[rxq->rx_tail]);
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index e315dd8..f1d00fb 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1084,12 +1084,6 @@ nfp_net_rx_queue_count(struct rte_eth_dev *dev, uint16_t queue_idx)
uint32_t count;
rxq = (struct nfp_net_rxq *)dev->data->rx_queues[queue_idx];
-
- if (rxq == NULL) {
- PMD_INIT_LOG(ERR, "Bad queue: %u\n", queue_idx);
- return 0;
- }
-
idx = rxq->rd_p % rxq->rx_count;
rxds = &rxq->rxds[idx];
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index c3edc23..9551cfd 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -2693,7 +2693,7 @@ rte_eth_rx_burst(uint8_t port_id, uint16_t queue_id,
* The queue id on the specific port.
* @return
* The number of used descriptors in the specific queue, or:
- * (-EINVAL) if *port_id* is invalid
+ * (-EINVAL) if *port_id* or *queue_id* is invalid
* (-ENOTSUP) if the device does not support this function
*/
static inline int
@@ -2701,8 +2701,10 @@ rte_eth_rx_queue_count(uint8_t port_id, uint16_t queue_id)
{
struct rte_eth_dev *dev = &rte_eth_devices[port_id];
- RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_count, -ENOTSUP);
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
+ if (queue_id >= dev->data->nb_rx_queues)
+ return -EINVAL;
return (*dev->dev_ops->rx_queue_count)(dev, queue_id);
}
--
2.8.1
^ permalink raw reply related
* [RFC 0/9] get Rx and Tx used descriptors
From: Olivier Matz @ 2016-11-24 9:54 UTC (permalink / raw)
To: dev; +Cc: thomas.monjalon, konstantin.ananyev, wenzhuo.lu, helin.zhang
This RFC patchset introduces a new ethdev API function
rte_eth_tx_queue_count() which is the tx counterpart of
rte_eth_rx_queue_count(). It implements this API on some
Intel drivers for reference, and it also optimizes the
implementation of rte_eth_rx_queue_count().
The usage of these functions can be:
- on Rx, anticipate that the cpu is not fast enough to process
all incoming packets, and take dispositions to solve the
problem (add more cpus, drop specific packets, ...)
- on Tx, detect that the link is overloaded, and take dispositions
to solve the problem (notify flow control, drop specific
packets)
The tests I've done (instrumenting testpmd) show that browsing
the descriptors linearly is slow when the ring size increases.
Accessing the head/tail registers through pci is also slow
whatever the size of the ring. A binary search is a good compromise
that gives quite good performance whatever the size of the ring.
Remaining question are about:
- should we keep this name? I'd say "queue_count" is quite confusing,
and I would expect it returns the number of queues, not the
number of used descriptors
- how shall we count the used descriptors, knowing that the driver
can hold some to free them by bulk, which reduces the effective
size of the ring
I would be happy to have some feedback about this RFC before
I send it as a patch.
Here are some helpers to understand the code more easily (I sometimes
make some shortcuts between like 1 pkt == 1 desc).
RX side
=======
- sw advances the tail pointer
- hw advances the head pointer
- the software populates the ring with descs to buffers that are filled
when the hw receives packets
- head == tail means there is no available buffer for hw to receive a packet
- head points to the next descriptor to be filled
- hw owns all descriptors between [head...tail]
- when a packet is written in a descriptor, the DD (descriptor done)
bit is set, and the head is advanced
- the driver never reads the head (needs a pci transaction), instead it
monitors the DD bit of next descriptor
- when a filled packet is retrieved by the software, the descriptor has
to be populated with a new empty buffer. This is not done for each
packet: the driver holds them and waits until it has many descriptors
to populate, and do it by bulk.
(by the way, it means that the effective size a queue of size=N is
lower than N since these descriptors cannot be used by the hw)
rxq->rx_tail: current value of the sw tail (the idx of the next packet to
be received). The real tail (hw) can be different since the driver can
hold descriptors.
rxq->nb_rx_hold: number of held descriptors
rxq->rxrearm_nb: same, but for vector driver
rxq->rx_free_thresh: when the number of held descriptors reaches this threshold,
descriptors are populated with buffers to be filled, and sw advances the tail
Example with a ring size of 64:
|----------------------------------------------------------------|
| xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx |
| x buffers filled with data by hw x |
| xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx |
|----------------------------------------------------------------|
^hw_tail=20
^sw_tail=35
^hw_head=54
<--- nb_hold -->
<-pkts in hw queue->
The descriptors marked with 'x' has their DD bit set, the other
(' ') reference empty buffers.
The next packet to be received by software is at index 35.
The software holds 15 descriptors that will be rearmed later.
There are 19 packets waiting in the hw queue.
We want the function rx_queue_count() to return the number of
"used" descriptors. The first question is: what does that mean
exactly? Should it be pkts_in_hw_queue or pkts_in_hw_queue + nb_hold?
The current implementation returns pkts_in_hw_queue, but including
nb_hold could be useful to know how many descriptors are really
free (= size - used).
The current implementation checks the DD bit starting from sw_tail,
every 4 packets. It can be quite slow for large rings. An alternative
is to read the head register, but it's also slow.
This patchset optimizes rx_queue_count() by doing a binary
search (checking for DD) between sw_tail and hw_tail, instead of a
linear search.
TX side
=======
- sw advances the tail pointer
- hw advances the head pointer
- the software populates the ring with full buffers to be sent by
the hw
- head points to the in-progress descriptor.
- sw writes new descriptors at tail
- head == tail means that the transmit queue is empty
- when the hw has processed a descriptor, it sets the DD bit if
the descriptor has the RS (report status) bit.
- the driver never reads the head (needs a pci transaction), instead it
monitors the DD bit of a descriptor that has the RS bit
txq->tx_tail: sw value for tail register
txq->tx_free_thresh: free buffers if count(free descriptors) < this value
txq->tx_rs_thresh: RS bit is set every X descriptor
txq->tx_next_dd: next desc to scan for DD bit
txq->tx_next_rs: next desc to set RS bit
txq->last_desc_cleaned: last descriptor that have been cleaned
txq->nb_tx_free: number of free descriptors
Example:
|----------------------------------------------------------------|
| D R R R |
| ............xxxxxxxxxxxxxxxxxxxxxxxxx |
| <descs sent><- descs not sent yet -> |
| ............xxxxxxxxxxxxxxxxxxxxxxxxx |
|----------------------------------------------------------------|
^last_desc_cleaned=8 ^next_rs=47
^next_dd=15 ^tail=45
^hw_head=20
<---- nb_used --------->
The hardware is currently processing the descriptor 20
'R' means the descriptor has the RS bit
'D' means the descriptor has the DD + RS bits
'x' are packets in txq (not sent)
'.' are packet already sent but not freed by sw
In this example, we have rs_thres=8. On next call to ixgbe_tx_free_bufs(),
some buffers will be freed.
The new implementation does a binary search (checking for DD) between next_dd
and tail.
Olivier Matz (9):
ethdev: clarify api comments of rx queue count
ethdev: move queue id check in generic layer
ethdev: add handler for Tx queue descriptor count
net/ixgbe: optimize Rx queue descriptor count
net/ixgbe: add handler for Tx queue descriptor count
net/igb: optimize rx queue descriptor count
net/igb: add handler for tx queue descriptor count
net/e1000: optimize rx queue descriptor count
net/e1000: add handler for tx queue descriptor count
drivers/net/e1000/e1000_ethdev.h | 10 +++-
drivers/net/e1000/em_ethdev.c | 1 +
drivers/net/e1000/em_rxtx.c | 109 ++++++++++++++++++++++++++++------
drivers/net/e1000/igb_ethdev.c | 1 +
drivers/net/e1000/igb_rxtx.c | 109 ++++++++++++++++++++++++++++------
drivers/net/i40e/i40e_rxtx.c | 5 --
drivers/net/ixgbe/ixgbe_ethdev.c | 1 +
drivers/net/ixgbe/ixgbe_ethdev.h | 4 +-
drivers/net/ixgbe/ixgbe_rxtx.c | 123 +++++++++++++++++++++++++++++++++------
drivers/net/ixgbe/ixgbe_rxtx.h | 2 +
drivers/net/nfp/nfp_net.c | 6 --
lib/librte_ether/rte_ethdev.h | 48 +++++++++++++--
12 files changed, 344 insertions(+), 75 deletions(-)
--
2.8.1
^ permalink raw reply
* [RFC 1/9] ethdev: clarify api comments of rx queue count
From: Olivier Matz @ 2016-11-24 9:54 UTC (permalink / raw)
To: dev; +Cc: thomas.monjalon, konstantin.ananyev, wenzhuo.lu, helin.zhang
In-Reply-To: <1479981261-19512-1-git-send-email-olivier.matz@6wind.com>
The API comments are not consistent between each other.
The function rte_eth_rx_queue_count() returns the number of used
descriptors on a receive queue.
PR=52423
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Ivan Boule <ivan.boule@6wind.com>
---
lib/librte_ether/rte_ethdev.h | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9678179..c3edc23 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1145,7 +1145,7 @@ typedef void (*eth_queue_release_t)(void *queue);
typedef uint32_t (*eth_rx_queue_count_t)(struct rte_eth_dev *dev,
uint16_t rx_queue_id);
-/**< @internal Get number of available descriptors on a receive queue of an Ethernet device. */
+/**< @internal Get number of used descriptors on a receive queue. */
typedef int (*eth_rx_descriptor_done_t)(void *rxq, uint16_t offset);
/**< @internal Check DD bit of specific RX descriptor */
@@ -1459,7 +1459,8 @@ struct eth_dev_ops {
eth_queue_stop_t tx_queue_stop;/**< Stop TX for a queue.*/
eth_rx_queue_setup_t rx_queue_setup;/**< Set up device RX queue.*/
eth_queue_release_t rx_queue_release;/**< Release RX queue.*/
- eth_rx_queue_count_t rx_queue_count; /**< Get Rx queue count. */
+ eth_rx_queue_count_t rx_queue_count;
+ /**< Get the number of used RX descriptors. */
eth_rx_descriptor_done_t rx_descriptor_done; /**< Check rxd DD bit */
/**< Enable Rx queue interrupt. */
eth_rx_enable_intr_t rx_queue_intr_enable;
@@ -2684,7 +2685,7 @@ rte_eth_rx_burst(uint8_t port_id, uint16_t queue_id,
}
/**
- * Get the number of used descriptors in a specific queue
+ * Get the number of used descriptors of a rx queue
*
* @param port_id
* The port identifier of the Ethernet device.
@@ -2699,9 +2700,11 @@ static inline int
rte_eth_rx_queue_count(uint8_t port_id, uint16_t queue_id)
{
struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_count, -ENOTSUP);
- return (*dev->dev_ops->rx_queue_count)(dev, queue_id);
+
+ return (*dev->dev_ops->rx_queue_count)(dev, queue_id);
}
/**
--
2.8.1
^ permalink raw reply related
* [RFC 3/9] ethdev: add handler for Tx queue descriptor count
From: Olivier Matz @ 2016-11-24 9:54 UTC (permalink / raw)
To: dev; +Cc: thomas.monjalon, konstantin.ananyev, wenzhuo.lu, helin.zhang
In-Reply-To: <1479981261-19512-1-git-send-email-olivier.matz@6wind.com>
Implement the Tx counterpart of rte_eth_rx_queue_count() in ethdev API,
which returns the number of used descriptors in a Tx queue.
It can help an application to detect that a link is too slow and cannot
send at the desired rate. In this case, the application can decide to
decrease the rate, or drop the packets with the lowest priority.
PR=52423
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Ivan Boule <ivan.boule@6wind.com>
---
lib/librte_ether/rte_ethdev.h | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9551cfd..8244807 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1147,6 +1147,10 @@ typedef uint32_t (*eth_rx_queue_count_t)(struct rte_eth_dev *dev,
uint16_t rx_queue_id);
/**< @internal Get number of used descriptors on a receive queue. */
+typedef uint32_t (*eth_tx_queue_count_t)(struct rte_eth_dev *dev,
+ uint16_t tx_queue_id);
+/**< @internal Get number of used descriptors on a transmit queue */
+
typedef int (*eth_rx_descriptor_done_t)(void *rxq, uint16_t offset);
/**< @internal Check DD bit of specific RX descriptor */
@@ -1461,6 +1465,8 @@ struct eth_dev_ops {
eth_queue_release_t rx_queue_release;/**< Release RX queue.*/
eth_rx_queue_count_t rx_queue_count;
/**< Get the number of used RX descriptors. */
+ eth_tx_queue_count_t tx_queue_count;
+ /**< Get the number of used TX descriptors. */
eth_rx_descriptor_done_t rx_descriptor_done; /**< Check rxd DD bit */
/**< Enable Rx queue interrupt. */
eth_rx_enable_intr_t rx_queue_intr_enable;
@@ -2710,6 +2716,31 @@ rte_eth_rx_queue_count(uint8_t port_id, uint16_t queue_id)
}
/**
+ * Get the number of used descriptors of a tx queue
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param queue_id
+ * The queue id on the specific port.
+ * @return
+ * - number of free descriptors if positive or zero
+ * - (-EINVAL) if *port_id* or *queue_id* is invalid.
+ * - (-ENOTSUP) if the device does not support this function
+ */
+static inline int
+rte_eth_tx_queue_count(uint8_t port_id, uint16_t queue_id)
+{
+ struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_queue_count, -ENOTSUP);
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
+ if (queue_id >= dev->data->nb_tx_queues)
+ return -EINVAL;
+
+ return (*dev->dev_ops->tx_queue_count)(dev, queue_id);
+}
+
+/**
* Check if the DD bit of the specific RX descriptor in the queue has been set
*
* @param port_id
--
2.8.1
^ permalink raw reply related
* [RFC 4/9] net/ixgbe: optimize Rx queue descriptor count
From: Olivier Matz @ 2016-11-24 9:54 UTC (permalink / raw)
To: dev; +Cc: thomas.monjalon, konstantin.ananyev, wenzhuo.lu, helin.zhang
In-Reply-To: <1479981261-19512-1-git-send-email-olivier.matz@6wind.com>
Use a binary search algorithm to find the first empty DD bit. The
ring-empty and ring-full cases are managed separately as they are more
likely to happen.
PR=52423
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Ivan Boule <ivan.boule@6wind.com>
---
drivers/net/ixgbe/ixgbe_rxtx.c | 63 ++++++++++++++++++++++++++++++++----------
1 file changed, 48 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 1a8ea5f..07509b4 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2852,25 +2852,58 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
uint32_t
ixgbe_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
{
-#define IXGBE_RXQ_SCAN_INTERVAL 4
- volatile union ixgbe_adv_rx_desc *rxdp;
+ volatile uint32_t *status;
struct ixgbe_rx_queue *rxq;
- uint32_t desc = 0;
+ uint32_t offset, interval, nb_hold, resolution;
+ int32_t idx;
rxq = dev->data->rx_queues[rx_queue_id];
- rxdp = &(rxq->rx_ring[rxq->rx_tail]);
-
- while ((desc < rxq->nb_rx_desc) &&
- (rxdp->wb.upper.status_error &
- rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD))) {
- desc += IXGBE_RXQ_SCAN_INTERVAL;
- rxdp += IXGBE_RXQ_SCAN_INTERVAL;
- if (rxq->rx_tail + desc >= rxq->nb_rx_desc)
- rxdp = &(rxq->rx_ring[rxq->rx_tail +
- desc - rxq->nb_rx_desc]);
- }
- return desc;
+ /* check if ring empty */
+ idx = rxq->rx_tail;
+ status = &rxq->rx_ring[idx].wb.upper.status_error;
+ if (!(*status & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
+ return 0;
+
+ /* decrease the precision if ring is large */
+ if (rxq->nb_rx_desc <= 256)
+ resolution = 4;
+ else
+ resolution = 16;
+
+ /* check if ring full */
+#ifdef RTE_IXGBE_INC_VECTOR
+ if (rxq->rx_using_sse)
+ nb_hold = rxq->rxrearm_nb;
+ else
+#endif
+ nb_hold = rxq->nb_rx_hold;
+
+ idx = rxq->rx_tail - nb_hold - resolution;
+ if (idx < 0)
+ idx += rxq->nb_rx_desc;
+ status = &rxq->rx_ring[idx].wb.upper.status_error;
+ if (*status & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD))
+ return rxq->nb_rx_desc;
+
+ /* use a binary search */
+ interval = (rxq->nb_rx_desc - nb_hold) >> 1;
+ offset = interval;
+
+ do {
+ idx = rxq->rx_tail + offset;
+ if (idx >= rxq->nb_rx_desc)
+ idx -= rxq->nb_rx_desc;
+
+ interval >>= 1;
+ status = &rxq->rx_ring[idx].wb.upper.status_error;
+ if (*status & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD))
+ offset += interval;
+ else
+ offset -= interval;
+ } while (interval >= resolution);
+
+ return offset;
}
int
--
2.8.1
^ permalink raw reply related
* [RFC 5/9] net/ixgbe: add handler for Tx queue descriptor count
From: Olivier Matz @ 2016-11-24 9:54 UTC (permalink / raw)
To: dev; +Cc: thomas.monjalon, konstantin.ananyev, wenzhuo.lu, helin.zhang
In-Reply-To: <1479981261-19512-1-git-send-email-olivier.matz@6wind.com>
Like for TX, use a binary search algorithm to get the number of used Tx
descriptors.
PR=52423
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Ivan Boule <ivan.boule@6wind.com>
---
drivers/net/ixgbe/ixgbe_ethdev.c | 1 +
drivers/net/ixgbe/ixgbe_ethdev.h | 4 ++-
drivers/net/ixgbe/ixgbe_rxtx.c | 57 ++++++++++++++++++++++++++++++++++++++++
drivers/net/ixgbe/ixgbe_rxtx.h | 2 ++
4 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index baffc71..0ba098a 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -553,6 +553,7 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
.rx_queue_intr_disable = ixgbe_dev_rx_queue_intr_disable,
.rx_queue_release = ixgbe_dev_rx_queue_release,
.rx_queue_count = ixgbe_dev_rx_queue_count,
+ .tx_queue_count = ixgbe_dev_tx_queue_count,
.rx_descriptor_done = ixgbe_dev_rx_descriptor_done,
.tx_queue_setup = ixgbe_dev_tx_queue_setup,
.tx_queue_release = ixgbe_dev_tx_queue_release,
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 4ff6338..e060c3d 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -348,7 +348,9 @@ int ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
const struct rte_eth_txconf *tx_conf);
uint32_t ixgbe_dev_rx_queue_count(struct rte_eth_dev *dev,
- uint16_t rx_queue_id);
+ uint16_t rx_queue_id);
+uint32_t ixgbe_dev_tx_queue_count(struct rte_eth_dev *dev,
+ uint16_t tx_queue_id);
int ixgbe_dev_rx_descriptor_done(void *rx_queue, uint16_t offset);
int ixgbevf_dev_rx_descriptor_done(void *rx_queue, uint16_t offset);
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 07509b4..5bf6b1a 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2437,6 +2437,7 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev,
txq->nb_tx_desc = nb_desc;
txq->tx_rs_thresh = tx_rs_thresh;
+ txq->tx_rs_thresh_div = nb_desc / tx_rs_thresh;
txq->tx_free_thresh = tx_free_thresh;
txq->pthresh = tx_conf->tx_thresh.pthresh;
txq->hthresh = tx_conf->tx_thresh.hthresh;
@@ -2906,6 +2907,62 @@ ixgbe_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
return offset;
}
+uint32_t
+ixgbe_dev_tx_queue_count(struct rte_eth_dev *dev, uint16_t tx_queue_id)
+{
+ struct ixgbe_tx_queue *txq;
+ uint32_t status;
+ int32_t offset, interval, idx = 0;
+ int32_t max_offset, used_desc;
+
+ txq = dev->data->tx_queues[tx_queue_id];
+
+ /* if DD on next threshold desc is not set, assume used packets
+ * are pending.
+ */
+ status = txq->tx_ring[txq->tx_next_dd].wb.status;
+ if (!(status & rte_cpu_to_le_32(IXGBE_ADVTXD_STAT_DD)))
+ return txq->nb_tx_desc - txq->nb_tx_free - 1;
+
+ /* browse DD bits between tail starting from tx_next_dd: we have
+ * to be careful since DD bits are only set every tx_rs_thresh
+ * descriptor.
+ */
+ interval = txq->tx_rs_thresh_div >> 1;
+ offset = interval * txq->tx_rs_thresh;
+
+ /* don't go beyond tail */
+ max_offset = txq->tx_tail - txq->tx_next_dd;
+ if (max_offset < 0)
+ max_offset += txq->nb_tx_desc;
+
+ do {
+ interval >>= 1;
+
+ if (offset >= max_offset) {
+ offset -= (interval * txq->tx_rs_thresh);
+ continue;
+ }
+
+ idx = txq->tx_next_dd + offset;
+ if (idx >= txq->nb_tx_desc)
+ idx -= txq->nb_tx_desc;
+
+ status = txq->tx_ring[idx].wb.status;
+ if (status & rte_cpu_to_le_32(IXGBE_ADVTXD_STAT_DD))
+ offset += (interval * txq->tx_rs_thresh);
+ else
+ offset -= (interval * txq->tx_rs_thresh);
+ } while (interval > 0);
+
+ /* idx is now the index of the head */
+ used_desc = txq->tx_tail - idx;
+ if (used_desc < 0)
+ used_desc += txq->nb_tx_desc;
+
+ return used_desc;
+}
+
int
ixgbe_dev_rx_descriptor_done(void *rx_queue, uint16_t offset)
{
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 2608b36..f69b5de 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -221,6 +221,8 @@ struct ixgbe_tx_queue {
uint16_t tx_free_thresh;
/** Number of TX descriptors to use before RS bit is set. */
uint16_t tx_rs_thresh;
+ /** Number of TX descriptors divided by tx_rs_thresh. */
+ uint16_t tx_rs_thresh_div;
/** Number of TX descriptors used since RS bit was set. */
uint16_t nb_tx_used;
/** Index to last TX descriptor to have been cleaned. */
--
2.8.1
^ permalink raw reply related
* [RFC 6/9] net/igb: optimize rx queue descriptor count
From: Olivier Matz @ 2016-11-24 9:54 UTC (permalink / raw)
To: dev; +Cc: thomas.monjalon, konstantin.ananyev, wenzhuo.lu, helin.zhang
In-Reply-To: <1479981261-19512-1-git-send-email-olivier.matz@6wind.com>
Use a binary search algorithm to find the first empty DD bit. The
ring-empty and ring-full cases are managed separately as they are more
likely to happen.
PR=52423
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Ivan Boule <ivan.boule@6wind.com>
---
drivers/net/e1000/igb_rxtx.c | 55 +++++++++++++++++++++++++++++++++-----------
1 file changed, 41 insertions(+), 14 deletions(-)
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index e9aa356..6b0111f 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -1507,24 +1507,51 @@ eth_igb_rx_queue_setup(struct rte_eth_dev *dev,
uint32_t
eth_igb_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
{
-#define IGB_RXQ_SCAN_INTERVAL 4
- volatile union e1000_adv_rx_desc *rxdp;
+ volatile uint32_t *status;
struct igb_rx_queue *rxq;
- uint32_t desc = 0;
+ uint32_t offset, interval, resolution;
+ int32_t idx;
rxq = dev->data->rx_queues[rx_queue_id];
- rxdp = &(rxq->rx_ring[rxq->rx_tail]);
-
- while ((desc < rxq->nb_rx_desc) &&
- (rxdp->wb.upper.status_error & E1000_RXD_STAT_DD)) {
- desc += IGB_RXQ_SCAN_INTERVAL;
- rxdp += IGB_RXQ_SCAN_INTERVAL;
- if (rxq->rx_tail + desc >= rxq->nb_rx_desc)
- rxdp = &(rxq->rx_ring[rxq->rx_tail +
- desc - rxq->nb_rx_desc]);
- }
- return desc;
+ /* check if ring empty */
+ idx = rxq->rx_tail;
+ status = &rxq->rx_ring[idx].wb.upper.status_error;
+ if (!(*status & rte_cpu_to_le_32(E1000_RXD_STAT_DD)))
+ return 0;
+
+ /* decrease the precision if ring is large */
+ if (rxq->nb_rx_desc <= 256)
+ resolution = 4;
+ else
+ resolution = 16;
+
+ /* check if ring full */
+ idx = rxq->rx_tail - rxq->nb_rx_hold - resolution;
+ if (idx < 0)
+ idx += rxq->nb_rx_desc;
+ status = &rxq->rx_ring[idx].wb.upper.status_error;
+ if (*status & rte_cpu_to_le_32(E1000_RXD_STAT_DD))
+ return rxq->nb_rx_desc;
+
+ /* use a binary search */
+ interval = (rxq->nb_rx_desc - rxq->nb_rx_hold) >> 1;
+ offset = interval;
+
+ do {
+ idx = rxq->rx_tail + offset;
+ if (idx >= rxq->nb_rx_desc)
+ idx -= rxq->nb_rx_desc;
+
+ interval >>= 1;
+ status = &rxq->rx_ring[idx].wb.upper.status_error;
+ if (*status & rte_cpu_to_le_32(E1000_RXD_STAT_DD))
+ offset += interval;
+ else
+ offset -= interval;
+ } while (interval >= resolution);
+
+ return offset;
}
int
--
2.8.1
^ permalink raw reply related
* [RFC 7/9] net/igb: add handler for tx queue descriptor count
From: Olivier Matz @ 2016-11-24 9:54 UTC (permalink / raw)
To: dev; +Cc: thomas.monjalon, konstantin.ananyev, wenzhuo.lu, helin.zhang
In-Reply-To: <1479981261-19512-1-git-send-email-olivier.matz@6wind.com>
Like for TX, use a binary search algorithm to get the number of used Tx
descriptors.
PR=52423
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Ivan Boule <ivan.boule@6wind.com>
---
drivers/net/e1000/e1000_ethdev.h | 5 +++-
drivers/net/e1000/igb_ethdev.c | 1 +
drivers/net/e1000/igb_rxtx.c | 51 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 6c25c8d..ad9ddaf 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -300,7 +300,10 @@ int eth_igb_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
struct rte_mempool *mb_pool);
uint32_t eth_igb_rx_queue_count(struct rte_eth_dev *dev,
- uint16_t rx_queue_id);
+ uint16_t rx_queue_id);
+
+uint32_t eth_igb_tx_queue_count(struct rte_eth_dev *dev,
+ uint16_t tx_queue_id);
int eth_igb_rx_descriptor_done(void *rx_queue, uint16_t offset);
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 08f2a68..a54d374 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -399,6 +399,7 @@ static const struct eth_dev_ops eth_igb_ops = {
.rx_queue_intr_disable = eth_igb_rx_queue_intr_disable,
.rx_queue_release = eth_igb_rx_queue_release,
.rx_queue_count = eth_igb_rx_queue_count,
+ .tx_queue_count = eth_igb_tx_queue_count,
.rx_descriptor_done = eth_igb_rx_descriptor_done,
.tx_queue_setup = eth_igb_tx_queue_setup,
.tx_queue_release = eth_igb_tx_queue_release,
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 6b0111f..2ff2417 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -1554,6 +1554,57 @@ eth_igb_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
return offset;
}
+uint32_t
+eth_igb_tx_queue_count(struct rte_eth_dev *dev, uint16_t tx_queue_id)
+{
+ volatile uint32_t *status;
+ struct igb_tx_queue *txq;
+ int32_t offset, interval, idx, resolution;
+
+ txq = dev->data->tx_queues[tx_queue_id];
+
+ /* check if ring empty */
+ idx = txq->tx_tail - 1;
+ if (idx < 0)
+ idx += txq->nb_tx_desc;
+ status = &txq->tx_ring[idx].wb.status;
+ if (*status & rte_cpu_to_le_32(E1000_TXD_STAT_DD))
+ return 0;
+
+ /* check if ring full */
+ idx = txq->tx_tail + 1;
+ if (idx >= txq->nb_tx_desc)
+ idx -= txq->nb_tx_desc;
+ status = &txq->tx_ring[idx].wb.status;
+ if (!(*status & rte_cpu_to_le_32(E1000_TXD_STAT_DD)))
+ return txq->nb_tx_desc;
+
+ /* decrease the precision if ring is large */
+ if (txq->nb_tx_desc <= 256)
+ resolution = 4;
+ else
+ resolution = 16;
+
+ /* use a binary search */
+ interval = txq->nb_tx_desc >> 1;
+ offset = interval;
+
+ do {
+ interval >>= 1;
+ idx = txq->tx_tail + offset;
+ if (idx >= txq->nb_tx_desc)
+ idx -= txq->nb_tx_desc;
+
+ status = &txq->tx_ring[idx].wb.status;
+ if (*status & rte_cpu_to_le_32(E1000_TXD_STAT_DD))
+ offset += interval;
+ else
+ offset -= interval;
+ } while (interval >= resolution);
+
+ return txq->nb_tx_desc - offset;
+}
+
int
eth_igb_rx_descriptor_done(void *rx_queue, uint16_t offset)
{
--
2.8.1
^ permalink raw reply related
* [RFC 8/9] net/e1000: optimize rx queue descriptor count
From: Olivier Matz @ 2016-11-24 9:54 UTC (permalink / raw)
To: dev; +Cc: thomas.monjalon, konstantin.ananyev, wenzhuo.lu, helin.zhang
In-Reply-To: <1479981261-19512-1-git-send-email-olivier.matz@6wind.com>
Use a binary search algorithm to find the first empty DD bit. The
ring-empty and ring-full cases are managed separately as they are more
likely to happen.
PR=52423
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Ivan Boule <ivan.boule@6wind.com>
---
drivers/net/e1000/em_rxtx.c | 55 +++++++++++++++++++++++++++++++++------------
1 file changed, 41 insertions(+), 14 deletions(-)
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index c1c724b..a469fd7 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -1385,24 +1385,51 @@ eth_em_rx_queue_setup(struct rte_eth_dev *dev,
uint32_t
eth_em_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
{
-#define EM_RXQ_SCAN_INTERVAL 4
- volatile struct e1000_rx_desc *rxdp;
+ volatile uint8_t *status;
struct em_rx_queue *rxq;
- uint32_t desc = 0;
+ uint32_t offset, interval, resolution;
+ int32_t idx;
rxq = dev->data->rx_queues[rx_queue_id];
- rxdp = &(rxq->rx_ring[rxq->rx_tail]);
-
- while ((desc < rxq->nb_rx_desc) &&
- (rxdp->status & E1000_RXD_STAT_DD)) {
- desc += EM_RXQ_SCAN_INTERVAL;
- rxdp += EM_RXQ_SCAN_INTERVAL;
- if (rxq->rx_tail + desc >= rxq->nb_rx_desc)
- rxdp = &(rxq->rx_ring[rxq->rx_tail +
- desc - rxq->nb_rx_desc]);
- }
- return desc;
+ /* check if ring empty */
+ idx = rxq->rx_tail;
+ status = &rxq->rx_ring[idx].status;
+ if (!(*status & E1000_RXD_STAT_DD))
+ return 0;
+
+ /* decrease the precision if ring is large */
+ if (rxq->nb_rx_desc <= 256)
+ resolution = 4;
+ else
+ resolution = 16;
+
+ /* check if ring full */
+ idx = rxq->rx_tail - rxq->nb_rx_hold - resolution;
+ if (idx < 0)
+ idx += rxq->nb_rx_desc;
+ status = &rxq->rx_ring[idx].status;
+ if (*status & E1000_RXD_STAT_DD)
+ return rxq->nb_rx_desc;
+
+ /* use a binary search */
+ interval = (rxq->nb_rx_desc - rxq->nb_rx_hold) >> 1;
+ offset = interval;
+
+ do {
+ idx = rxq->rx_tail + offset;
+ if (idx >= rxq->nb_rx_desc)
+ idx -= rxq->nb_rx_desc;
+
+ interval >>= 1;
+ status = &rxq->rx_ring[idx].status;
+ if (*status & E1000_RXD_STAT_DD)
+ offset += interval;
+ else
+ offset -= interval;
+ } while (interval >= resolution);
+
+ return offset;
}
int
--
2.8.1
^ permalink raw reply related
* [RFC 9/9] net/e1000: add handler for tx queue descriptor count
From: Olivier Matz @ 2016-11-24 9:54 UTC (permalink / raw)
To: dev; +Cc: thomas.monjalon, konstantin.ananyev, wenzhuo.lu, helin.zhang
In-Reply-To: <1479981261-19512-1-git-send-email-olivier.matz@6wind.com>
Like for TX, use a binary search algorithm to get the number of used Tx
descriptors.
PR=52423
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Ivan Boule <ivan.boule@6wind.com>
---
drivers/net/e1000/e1000_ethdev.h | 5 +++-
drivers/net/e1000/em_ethdev.c | 1 +
drivers/net/e1000/em_rxtx.c | 51 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index ad9ddaf..8945916 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -364,7 +364,10 @@ int eth_em_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
struct rte_mempool *mb_pool);
uint32_t eth_em_rx_queue_count(struct rte_eth_dev *dev,
- uint16_t rx_queue_id);
+ uint16_t rx_queue_id);
+
+uint32_t eth_em_tx_queue_count(struct rte_eth_dev *dev,
+ uint16_t tx_queue_id);
int eth_em_rx_descriptor_done(void *rx_queue, uint16_t offset);
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 866a5cf..7fe5e3b 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -190,6 +190,7 @@ static const struct eth_dev_ops eth_em_ops = {
.rx_queue_setup = eth_em_rx_queue_setup,
.rx_queue_release = eth_em_rx_queue_release,
.rx_queue_count = eth_em_rx_queue_count,
+ .tx_queue_count = eth_em_tx_queue_count,
.rx_descriptor_done = eth_em_rx_descriptor_done,
.tx_queue_setup = eth_em_tx_queue_setup,
.tx_queue_release = eth_em_tx_queue_release,
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index a469fd7..8afcfda 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -1432,6 +1432,57 @@ eth_em_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
return offset;
}
+uint32_t
+eth_em_tx_queue_count(struct rte_eth_dev *dev, uint16_t tx_queue_id)
+{
+ volatile uint8_t *status;
+ struct em_tx_queue *txq;
+ int32_t offset, interval, idx, resolution;
+
+ txq = dev->data->tx_queues[tx_queue_id];
+
+ /* check if ring empty */
+ idx = txq->tx_tail - 1;
+ if (idx < 0)
+ idx += txq->nb_tx_desc;
+ status = &txq->tx_ring[idx].upper.fields.status;
+ if (*status & E1000_TXD_STAT_DD)
+ return 0;
+
+ /* check if ring full */
+ idx = txq->tx_tail + 1;
+ if (idx >= txq->nb_tx_desc)
+ idx -= txq->nb_tx_desc;
+ status = &txq->tx_ring[idx].upper.fields.status;
+ if (!(*status & E1000_TXD_STAT_DD))
+ return txq->nb_tx_desc;
+
+ /* decrease the precision if ring is large */
+ if (txq->nb_tx_desc <= 256)
+ resolution = 4;
+ else
+ resolution = 16;
+
+ /* use a binary search */
+ offset = txq->nb_tx_desc >> 1;
+ interval = offset;
+
+ do {
+ idx = txq->tx_tail + offset;
+ if (idx >= txq->nb_tx_desc)
+ idx -= txq->nb_tx_desc;
+
+ interval >>= 1;
+ status = &txq->tx_ring[idx].upper.fields.status;
+ if (*status & E1000_TXD_STAT_DD)
+ offset += interval;
+ else
+ offset -= interval;
+ } while (interval >= resolution);
+
+ return txq->nb_tx_desc - offset;
+}
+
int
eth_em_rx_descriptor_done(void *rx_queue, uint16_t offset)
{
--
2.8.1
^ permalink raw reply related
* Re: [RFC 1/9] ethdev: clarify api comments of rx queue count
From: Ferruh Yigit @ 2016-11-24 10:52 UTC (permalink / raw)
To: Olivier Matz, dev
Cc: thomas.monjalon, konstantin.ananyev, wenzhuo.lu, helin.zhang
In-Reply-To: <1479981261-19512-2-git-send-email-olivier.matz@6wind.com>
On 11/24/2016 9:54 AM, Olivier Matz wrote:
> The API comments are not consistent between each other.
>
> The function rte_eth_rx_queue_count() returns the number of used
> descriptors on a receive queue.
>
> PR=52423
What is this marker?
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> Acked-by: Ivan Boule <ivan.boule@6wind.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
^ permalink raw reply
* Re: [RFC 2/9] ethdev: move queue id check in generic layer
From: Ferruh Yigit @ 2016-11-24 10:59 UTC (permalink / raw)
To: Olivier Matz, dev
Cc: thomas.monjalon, konstantin.ananyev, wenzhuo.lu, helin.zhang
In-Reply-To: <1479981261-19512-3-git-send-email-olivier.matz@6wind.com>
On 11/24/2016 9:54 AM, Olivier Matz wrote:
> The check of queue_id is done in all drivers implementing
> rte_eth_rx_queue_count(). Factorize this check in the generic function.
>
> Note that the nfp driver was doing the check differently, which could
> induce crashes if the queue index was too big.
>
> By the way, also move the is_supported test before the port valid and
> queue valid test.
>
> PR=52423
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> Acked-by: Ivan Boule <ivan.boule@6wind.com>
> ---
<...>
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index c3edc23..9551cfd 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -2693,7 +2693,7 @@ rte_eth_rx_burst(uint8_t port_id, uint16_t queue_id,
> * The queue id on the specific port.
> * @return
> * The number of used descriptors in the specific queue, or:
> - * (-EINVAL) if *port_id* is invalid
> + * (-EINVAL) if *port_id* or *queue_id* is invalid
> * (-ENOTSUP) if the device does not support this function
> */
> static inline int
> @@ -2701,8 +2701,10 @@ rte_eth_rx_queue_count(uint8_t port_id, uint16_t queue_id)
> {
> struct rte_eth_dev *dev = &rte_eth_devices[port_id];
>
> - RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_count, -ENOTSUP);
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
Doing port validity check before accessing dev->dev_ops->rx_queue_count
can be good idea.
What about validating port_id even before accessing
rte_eth_devices[port_id]?
> + if (queue_id >= dev->data->nb_rx_queues)
> + return -EINVAL;
>
> return (*dev->dev_ops->rx_queue_count)(dev, queue_id);
> }
>
^ permalink raw reply
* Re: [PATCH 00/56] Solarflare libefx-based PMD
From: Andrew Rybchenko @ 2016-11-24 10:59 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Ferruh Yigit, dev
In-Reply-To: <20161123112145.4aa2d794@samsung9.wavecable.com>
On 11/23/2016 10:21 PM, Stephen Hemminger wrote:
> On Wed, 23 Nov 2016 10:49:33 +0300
> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
>
>> I've tried to explain it above in item (2):
>>
>> >>>
>>
>> 2. Another Solarflare PMD with in-kernel part (for control operations)
>> is considered and could be added in the future. Code for data path
>> should be shared by these two drivers. libefx-based PMD is put into
>> 'efx' subdirectory to have a space for another PMD and shared code.
>>
>> <<<
>>
>> So, main reason is to have location for the code shared by two Solarflare
>> network PMDs. May be it better to relocate when we really have it.
>> I'm open for other ideas/suggestions.
> So is this driver dependent on another non-upstream kernel driver to work?
No, this driver does not have any external dependencies.
> Is this documented in docs directory. If it does depend on other non-upstream
> components, then the default config should disable the driver.
Andrew.
^ permalink raw reply
* Re: [RFC 1/9] ethdev: clarify api comments of rx queue count
From: Olivier Matz @ 2016-11-24 11:13 UTC (permalink / raw)
To: Ferruh Yigit, dev
Cc: thomas.monjalon, konstantin.ananyev, wenzhuo.lu, helin.zhang
In-Reply-To: <e0f9dd84-4013-5692-45cc-ddf23fa99355@intel.com>
On Thu, 2016-11-24 at 10:52 +0000, Ferruh Yigit wrote:
> On 11/24/2016 9:54 AM, Olivier Matz wrote:
> > The API comments are not consistent between each other.
> >
> > The function rte_eth_rx_queue_count() returns the number of used
> > descriptors on a receive queue.
> >
> > PR=52423
>
> What is this marker?
>
Sorry, this is a mistake, it's an internal marker...
I hoped nobody would notice it ;)
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > Acked-by: Ivan Boule <ivan.boule@6wind.com>
>
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
>
Thanks for reviewing!
Regards,
Olivier
^ permalink raw reply
* [PATCH] crypto/qat: fix to avoid buffer overwrite in OOP case
From: Fiona Trahe @ 2016-11-24 11:17 UTC (permalink / raw)
To: dev; +Cc: pablo.de.lara.guarch, fiona.trahe, john.griffin,
arkadiuszx.kusztal
In out-of-place operation, data is DMAed from source mbuf
to destination mbuf. To avoid header data in dest mbuf being
overwritten, the minimal data-set should be DMAed.
Fixes: 39e0bee48e81 ("crypto/qat: rework request builder for performance")
Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
---
This patch depends on following patch :
crypto: remove unused digest-appended feature
http://dpdk.org/dev/patchwork/patch/17079/
drivers/crypto/qat/qat_crypto.c | 66 ++++++++++++++++++++---------------------
drivers/crypto/qat/qat_crypto.h | 1 +
2 files changed, 34 insertions(+), 33 deletions(-)
diff --git a/drivers/crypto/qat/qat_crypto.c b/drivers/crypto/qat/qat_crypto.c
index 6a6bd2e..afce4ac 100644
--- a/drivers/crypto/qat/qat_crypto.c
+++ b/drivers/crypto/qat/qat_crypto.c
@@ -955,7 +955,7 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
uint32_t cipher_len = 0, cipher_ofs = 0;
uint32_t auth_len = 0, auth_ofs = 0;
uint32_t min_ofs = 0;
- uint64_t buf_start = 0;
+ uint64_t src_buf_start = 0, dst_buf_start = 0;
#ifdef RTE_LIBRTE_PMD_QAT_DEBUG_TX
@@ -1077,27 +1077,40 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
if (do_cipher && do_auth)
min_ofs = cipher_ofs < auth_ofs ? cipher_ofs : auth_ofs;
-
- /* Start DMA at nearest aligned address below min_ofs */
- #define QAT_64_BTYE_ALIGN_MASK (~0x3f)
- buf_start = rte_pktmbuf_mtophys_offset(op->sym->m_src, min_ofs) &
- QAT_64_BTYE_ALIGN_MASK;
-
- if (unlikely((rte_pktmbuf_mtophys(op->sym->m_src)
- - rte_pktmbuf_headroom(op->sym->m_src)) > buf_start)) {
- /* alignment has pushed addr ahead of start of mbuf
- * so revert and take the performance hit
+ if (unlikely(op->sym->m_dst != NULL)) {
+ /* Out-of-place operation (OOP)
+ * Don't align DMA start. DMA the minimum data-set
+ * so as not to overwrite data in dest buffer
*/
- buf_start = rte_pktmbuf_mtophys(op->sym->m_src);
+ src_buf_start =
+ rte_pktmbuf_mtophys_offset(op->sym->m_src, min_ofs);
+ dst_buf_start =
+ rte_pktmbuf_mtophys_offset(op->sym->m_dst, min_ofs);
+ } else {
+ /* In-place operation
+ * Start DMA at nearest aligned address below min_ofs
+ */
+ src_buf_start =
+ rte_pktmbuf_mtophys_offset(op->sym->m_src, min_ofs)
+ & QAT_64_BTYE_ALIGN_MASK;
+
+ if (unlikely((rte_pktmbuf_mtophys(op->sym->m_src) -
+ rte_pktmbuf_headroom(op->sym->m_src))
+ > src_buf_start)) {
+ /* alignment has pushed addr ahead of start of mbuf
+ * so revert and take the performance hit
+ */
+ src_buf_start =
+ rte_pktmbuf_mtophys_offset(op->sym->m_src,
+ min_ofs);
+ }
+ dst_buf_start = src_buf_start;
}
- qat_req->comn_mid.dest_data_addr =
- qat_req->comn_mid.src_data_addr = buf_start;
-
if (do_cipher) {
cipher_param->cipher_offset =
- (uint32_t)rte_pktmbuf_mtophys_offset(
- op->sym->m_src, cipher_ofs) - buf_start;
+ (uint32_t)rte_pktmbuf_mtophys_offset(
+ op->sym->m_src, cipher_ofs) - src_buf_start;
cipher_param->cipher_length = cipher_len;
} else {
cipher_param->cipher_offset = 0;
@@ -1105,7 +1118,7 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
}
if (do_auth) {
auth_param->auth_off = (uint32_t)rte_pktmbuf_mtophys_offset(
- op->sym->m_src, auth_ofs) - buf_start;
+ op->sym->m_src, auth_ofs) - src_buf_start;
auth_param->auth_len = auth_len;
} else {
auth_param->auth_off = 0;
@@ -1118,21 +1131,8 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
(cipher_param->cipher_offset + cipher_param->cipher_length)
: (auth_param->auth_off + auth_param->auth_len);
-
- /* out-of-place operation (OOP) */
- if (unlikely(op->sym->m_dst != NULL)) {
-
- if (do_auth)
- qat_req->comn_mid.dest_data_addr =
- rte_pktmbuf_mtophys_offset(op->sym->m_dst,
- auth_ofs)
- - auth_param->auth_off;
- else
- qat_req->comn_mid.dest_data_addr =
- rte_pktmbuf_mtophys_offset(op->sym->m_dst,
- cipher_ofs)
- - cipher_param->cipher_offset;
- }
+ qat_req->comn_mid.src_data_addr = src_buf_start;
+ qat_req->comn_mid.dest_data_addr = dst_buf_start;
if (ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_GALOIS_128 ||
ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_GALOIS_64) {
diff --git a/drivers/crypto/qat/qat_crypto.h b/drivers/crypto/qat/qat_crypto.h
index 0afe74e..6b84488 100644
--- a/drivers/crypto/qat/qat_crypto.h
+++ b/drivers/crypto/qat/qat_crypto.h
@@ -43,6 +43,7 @@
*/
#define ALIGN_POW2_ROUNDUP(num, align) \
(((num) + (align) - 1) & ~((align) - 1))
+#define QAT_64_BTYE_ALIGN_MASK (~0x3f)
/**
* Structure associated with each queue.
--
2.5.0
^ permalink raw reply related
* [PATCH v2 0/5] bonding: setup all queues of slave devices
From: Jan Blunck @ 2016-11-24 11:26 UTC (permalink / raw)
To: dev
Cc: ferruh.yigit, i.maximets, bruce.richardson, declan.doherty,
ehkinzie, bernard.iremonger, stable
Prior to 16.11 some drivers (e.g. virtio) still had problems if their
queues where setup repeatedly. The bonding driver was working around the
problem by reusing already setup queues. This series of patches changes the
way how queue setup is done to give control to the driver to properly release
already initialized queues before they are setup again. Therefore the driver
call sequence is as if the number of queues is temporarily reduced before the
queues are setup again.
Ilya Maximets (1):
Revert "bonding: use existing enslaved device queues"
Jan Blunck (4):
ethdev: Call rx/tx_queue_release before rx/tx_queue_setup
ethdev: Free rx/tx_queues after releasing all queues
ethdev: Add DPDK internal _rte_eth_dev_reset()
net/bonding: Force reconfiguration of removed slave interfaces
drivers/net/bonding/rte_eth_bond_pmd.c | 13 +++++------
lib/librte_ether/rte_ethdev.c | 40 ++++++++++++++++++++++++++++++++++
lib/librte_ether/rte_ethdev.h | 13 +++++++++++
lib/librte_ether/rte_ether_version.map | 6 +++++
4 files changed, 64 insertions(+), 8 deletions(-)
--
2.7.4
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox