mbox series

[RFC,0/3] net: ethernet: ti: cpsw: Add XDP support

Message ID 20190417174942.11811-1-ivan.khoronzhuk@linaro.org
Headers show
Series net: ethernet: ti: cpsw: Add XDP support | expand

Message

Ivan Khoronzhuk April 17, 2019, 5:49 p.m. UTC 
This patchset is RFC adding XDP support for TI cpsw driver that is
based on page_pool allocator. It was verified with af_xdp sockets
and on xdp drop. For XDP redirect to another interface it's under
verification, still not sure about all cases that should be verified.
Also regular tests with iperf2 were done in order to verify impact on
regular netstack performance, compared with base commit from
net-next/master: 432bc230700f86801cffa5e159e05dea6229f722

It was verified with following configs enabled:
CONFIG_JIT=y
CONFIG_BPFILTER=y
CONFIG_BPF_SYSCALL=y
CONFIG_XDP_SOCKETS=y
CONFIG_BPF_EVENTS=y
CONFIG_HAVE_EBPF_JIT=y
CONFIG_BPF_JIT=y
CONFIG_CGROUP_BPF=y

iperf2 UDP RX summary (packet size / Mbps):
+--------------------------------------------------------------+
| pkt_size/rate | 1024 | 1500 | 1470 |  64  | 128  | 512 | 256 |
|---------------+------+------+------+------+------+-----+-----|
| base commit   | 561  | 470  | 796  |  35  | 70.1 | 281 | 140 |
|---------------+------+------+------+------+------+-----+-----|
| XDP patched   | 563  | 455  | 808  |  35  | 70.2 | 282 | 141 |
+--------------------------------------------------------------+

iperf2 UDP TX summary (packet size / Mbps):
+--------------------------------------------------------------+
| pkt_size/rate | 1024 | 1500 | 1470 |  64  | 128  | 512 | 256 |
|---------------+------+------+------+------+------+-----+-----|
| base commit   | 555  | 666  | 736  | 34.5 | 70.3 | 281 | 140 |
|---------------+------+------+------+------+------+-----+-----|
| XDP patched   | 558  | 696  | 759  | 35.2 | 69.2 | 279 | 140 |
+--------------------------------------------------------------+

iperf2 TCP summary (window size / Mbps):
+------------------------------------------------------------+
| window size/rate |  16  |  32  |  64  |  128 |  8   | 256  |
|------------------+------+------+------+------+------+------|
| base commit      | 753  | 887  | 931  | 932  | 676  | 932  |
|------------------+------+------+------+------+------+------|
| XDP patched      | 823  | 888  | 932  | 933  | 669  | 933  |
+------------------------------------------------------------+

For af_xdp socket type verification several generic changes should be added
that can be seen here (rough fixes, for samples related seems like last version
of samples is more integrated with libbpf api, so should be rebased,
witch I will send as RFC separately):
https://github.com/ikhorn/af_xdp_stuff/tree/af_xdp_armv7

Ivan Khoronzhuk (3):
  net: ethernet: ti: davinci_cpdma: add dma mapped submit
  net: ethernet: ti: davinci_cpdma: return handler status
  net: ethernet: ti: cpsw: add XDP support

 drivers/net/ethernet/ti/Kconfig         |   1 +
 drivers/net/ethernet/ti/cpsw.c          | 552 +++++++++++++++++++++---
 drivers/net/ethernet/ti/davinci_cpdma.c | 117 +++--
 drivers/net/ethernet/ti/davinci_cpdma.h |   6 +-
 drivers/net/ethernet/ti/davinci_emac.c  |  18 +-
 5 files changed, 591 insertions(+), 103 deletions(-)

-- 
2.17.1

Comments

Ivan Khoronzhuk April 19, 2019, 10:42 a.m. UTC | #1 
On Fri, Apr 19, 2019 at 11:31:56AM +0300, Ilias Apalodimas wrote:
>Hi Ivan,

>

>> +static struct page *cpsw_alloc_page(struct cpsw_common *cpsw)

>> +{

>> +	struct page_pool *pool = cpsw->rx_page_pool;

>> +	struct page *page;

>> +	int i = 0;

>> +

>> +	do {

>> +		page = page_pool_dev_alloc_pages(pool);

>> +		if (!page)

>> +			return NULL;

>> +

>> +		/* skip pages used by skb netstack */

>I think the comment here is wrong and might confuse people.

>The page ref cnt is 1, which means the packet was *processed* and netstack is

>done with it, hence you can re-use it.

>If it's !=1 then you correctly unmap the buffer and decrease the ref cnt, so it

>will eventually be freed and not returned to the pool, right?

It's compensation substitution for page_pool support in skb netsack.
And should be considered in combine with:

skb = build_skb(xmeta, cpsw_rxbuf_total_len(pkt_size));
...
page_pool_recycle_direct(pool, page);
page_ref_inc(page);
netif_receive_skb(skb);

Here order is important.

I will correct comments in final version (w/o overloading) ofc, leaving
thinking environment for people. I think it's fair enough about this.

>> +		if (page_ref_count(page) == 1)

>> +			break;

>> +

>> +		/* it's a pitty, but free page */

>> +		page_pool_recycle_direct(pool, page);

>> +	} while (++i < descs_pool_size);

>> +

>> +	return page;

>> +}

>> +

>

> /Ilias


-- 
Regards,
Ivan Khoronzhuk