| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a78d3011-738c-2289-7a70-cd046cde12d5@engleder-embedded.com>
Date: Tue, 10 Jan 2023 22:28:51 +0100
From: Gerhard Engleder <gerhard@...leder-embedded.com>
To: Alexander H Duyck <alexander.duyck@...il.com>,
netdev@...r.kernel.org
Cc: davem@...emloft.net, kuba@...nel.org, edumazet@...gle.com,
pabeni@...hat.com
Subject: Re: [PATCH net-next v4 09/10] tsnep: Add XDP RX support
On 10.01.23 18:40, Alexander H Duyck wrote:
> On Mon, 2023-01-09 at 20:15 +0100, Gerhard Engleder wrote:
>> If BPF program is set up, then run BPF program for every received frame
>> and execute the selected action.
>>
>> Signed-off-by: Gerhard Engleder <gerhard@...leder-embedded.com>
>> ---
>> drivers/net/ethernet/engleder/tsnep_main.c | 122 ++++++++++++++++++++-
>> 1 file changed, 120 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
>> index 451ad1849b9d..002c879639db 100644
>> --- a/drivers/net/ethernet/engleder/tsnep_main.c
>> +++ b/drivers/net/ethernet/engleder/tsnep_main.c
>> @@ -27,6 +27,7 @@
>> #include <linux/phy.h>
>> #include <linux/iopoll.h>
>> #include <linux/bpf.h>
>> +#include <linux/bpf_trace.h>
>>
>> #define TSNEP_SKB_PAD (NET_SKB_PAD + NET_IP_ALIGN)
>> #define TSNEP_HEADROOM ALIGN(max(TSNEP_SKB_PAD, XDP_PACKET_HEADROOM), 4)
>> @@ -44,6 +45,9 @@
>> #define TSNEP_COALESCE_USECS_MAX ((ECM_INT_DELAY_MASK >> ECM_INT_DELAY_SHIFT) * \
>> ECM_INT_DELAY_BASE_US + ECM_INT_DELAY_BASE_US - 1)
>>
>> +#define TSNEP_XDP_TX BIT(0)
>> +#define TSNEP_XDP_REDIRECT BIT(1)
>> +
>> enum {
>> __TSNEP_DOWN,
>> };
>> @@ -625,6 +629,28 @@ static void tsnep_xdp_xmit_flush(struct tsnep_tx *tx)
>> iowrite32(TSNEP_CONTROL_TX_ENABLE, tx->addr + TSNEP_CONTROL);
>> }
>>
>> +static bool tsnep_xdp_xmit_back(struct tsnep_adapter *adapter,
>> + struct xdp_buff *xdp,
>> + struct netdev_queue *tx_nq, struct tsnep_tx *tx)
>> +{
>> + struct xdp_frame *xdpf = xdp_convert_buff_to_frame(xdp);
>> + bool xmit;
>> +
>> + if (unlikely(!xdpf))
>> + return false;
>> +
>> + __netif_tx_lock(tx_nq, smp_processor_id());
>> +
>> + /* Avoid transmit queue timeout since we share it with the slow path */
>> + txq_trans_cond_update(tx_nq);
>> +
>> + xmit = tsnep_xdp_xmit_frame_ring(xdpf, tx, TSNEP_TX_TYPE_XDP_TX);
>> +
>
> Again the trans_cond_update should be after the xmit and only if it is
> not indicating it completed the transmit.
tsnep_xdp_xmit_frame_ring() only adds xpdf to the descriptor ring, so it
cannot complete the transmit. Therefore and in line with your previous
comment trans_cond_update() should be called here if xpdf is
successfully placed in the descriptor ring. Is that right?
>> + __netif_tx_unlock(tx_nq);
>> +
>> + return xmit;
>> +}
>> +
>> static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget)
>> {
>> struct tsnep_tx_entry *entry;
>> @@ -983,6 +1009,62 @@ static int tsnep_rx_refill(struct tsnep_rx *rx, int count, bool reuse)
>> return i;
>> }
>>
>> +static bool tsnep_xdp_run_prog(struct tsnep_rx *rx, struct bpf_prog *prog,
>> + struct xdp_buff *xdp, int *status,
>> + struct netdev_queue *tx_nq, struct tsnep_tx *tx)
>> +{
>> + unsigned int length;
>> + unsigned int sync;
>> + u32 act;
>> +
>> + length = xdp->data_end - xdp->data_hard_start - XDP_PACKET_HEADROOM;
>> +
>> + act = bpf_prog_run_xdp(prog, xdp);
>> +
>> + /* Due xdp_adjust_tail: DMA sync for_device cover max len CPU touch */
>> + sync = xdp->data_end - xdp->data_hard_start - XDP_PACKET_HEADROOM;
>> + sync = max(sync, length);
>> +
>> + switch (act) {
>> + case XDP_PASS:
>> + return false;
>> + case XDP_TX:
>> + if (!tsnep_xdp_xmit_back(rx->adapter, xdp, tx_nq, tx))
>> + goto out_failure;
>> + *status |= TSNEP_XDP_TX;
>> + return true;
>> + case XDP_REDIRECT:
>> + if (xdp_do_redirect(rx->adapter->netdev, xdp, prog) < 0)
>> + goto out_failure;
>> + *status |= TSNEP_XDP_REDIRECT;
>> + return true;
>> + default:
>> + bpf_warn_invalid_xdp_action(rx->adapter->netdev, prog, act);
>> + fallthrough;
>> + case XDP_ABORTED:
>> +out_failure:
>> + trace_xdp_exception(rx->adapter->netdev, prog, act);
>> + fallthrough;
>> + case XDP_DROP:
>> + page_pool_put_page(rx->page_pool, virt_to_head_page(xdp->data),
>> + sync, true);
>> + return true;
>> + }
>> +}
>> +
>> +static void tsnep_finalize_xdp(struct tsnep_adapter *adapter, int status,
>> + struct netdev_queue *tx_nq, struct tsnep_tx *tx)
>> +{
>> + if (status & TSNEP_XDP_TX) {
>> + __netif_tx_lock(tx_nq, smp_processor_id());
>> + tsnep_xdp_xmit_flush(tx);
>> + __netif_tx_unlock(tx_nq);
>> + }
>> +
>> + if (status & TSNEP_XDP_REDIRECT)
>> + xdp_do_flush();
>> +}
>> +
>> static struct sk_buff *tsnep_build_skb(struct tsnep_rx *rx, struct page *page,
>> int length)
>> {
>> @@ -1018,15 +1100,29 @@ static int tsnep_rx_poll(struct tsnep_rx *rx, struct napi_struct *napi,
>> int budget)
>> {
>> struct device *dmadev = rx->adapter->dmadev;
>> - int desc_available;
>> - int done = 0;
>> enum dma_data_direction dma_dir;
>> struct tsnep_rx_entry *entry;
>> + struct netdev_queue *tx_nq;
>> + struct bpf_prog *prog;
>> + struct xdp_buff xdp;
>> struct sk_buff *skb;
>> + struct tsnep_tx *tx;
>> + int desc_available;
>> + int xdp_status = 0;
>> + int done = 0;
>> int length;
>>
>> desc_available = tsnep_rx_desc_available(rx);
>> dma_dir = page_pool_get_dma_dir(rx->page_pool);
>> + prog = READ_ONCE(rx->adapter->xdp_prog);
>> + if (prog) {
>> + int queue = smp_processor_id() % rx->adapter->num_tx_queues;
>> +
>
> As I mentioned before. Take a look at how this was addressed in
> skb_tx_hash. The modulus division is really expensive.
>
> Also does this make sense. I am assuming you have a 1:1 Tx to Rx
> mapping for your queues don't you? If so it might make more sense to
> use the Tx queue that you clean in this queue pair.
Sounds reasonable. I will work on that.
Gerhard
Powered by blists - more mailing lists