Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Kishen Maloor <kishen.maloor@intel.com>
To: "Jesper Dangaard Brouer" <jbrouer@redhat.com>,
	bpf@vger.kernel.org, netdev@vger.kernel.org, hawk@kernel.org,
	magnus.karlsson@intel.com, "Björn Töpel" <bjorn@kernel.org>
Cc: <brouer@redhat.com>, Jithu Joseph <jithu.joseph@intel.com>
Subject: Re: [RFC bpf-next 5/5] samples/bpf/xdpsock_user.c: Launchtime/TXTIME API usage
Date: Thu, 19 Aug 2021 15:32:35 -0400	[thread overview]
Message-ID: <d15fdcb4-9b5f-ebf4-f5fe-5b16f06f2afc@intel.com> (raw)
In-Reply-To: <4ea898db-563c-851b-c3da-9389abcb83ac@redhat.com>

On 8/18/21 4:54 AM, Jesper Dangaard Brouer wrote:
> 
> On 03/08/2021 19.10, Kishen Maloor wrote:
>> diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
>> index 3fd2f6a0d1eb..a0fd3d5414ba 100644
>> --- a/samples/bpf/xdpsock_user.c
>> +++ b/samples/bpf/xdpsock_user.c
> [...]
>> @@ -741,6 +745,8 @@ static inline u16 udp_csum(u32 saddr, u32 daddr, u32 len,
>>     #define ETH_FCS_SIZE 4
>>   +#define MD_SIZE (sizeof(struct xdp_user_tx_metadata))
>> +
>>   #define PKT_HDR_SIZE (sizeof(struct ethhdr) + sizeof(struct iphdr) + \
>>                 sizeof(struct udphdr))
>>   @@ -798,8 +804,10 @@ static void gen_eth_hdr_data(void)
>>     static void gen_eth_frame(struct xsk_umem_info *umem, u64 addr)
>>   {
>> -    memcpy(xsk_umem__get_data(umem->buffer, addr), pkt_data,
>> -           PKT_SIZE);
>> +    if (opt_launch_time)
>> +        memcpy(xsk_umem__get_data(umem->buffer, addr) + MD_SIZE, pkt_data, PKT_SIZE);
>> +    else
>> +        memcpy(xsk_umem__get_data(umem->buffer, addr), pkt_data, PKT_SIZE);
>>   }
>>   
> 
> I imagined that AF_XDP 'addr' would still point to the start of the packet data, and that metadata area was access via a negative offset from 'addr'.
> 

There is currently no kernel "infrastructure" on the TX path which factors in the concept of XDP metadata, so the application needs to make place for it. (For e.g., XDP_PACKET_HEADROOM has no de facto role on the TX path AFAIK).

xsk_umem__get_data() just returns the UMEM chunk at the user supplied 'addr' and applications need to write both the XDP packet and any accompanying metadata into this (raw) buffer.

In doing so, it places that metadata right ahead of the XDP packet (much like how that's structured in the RX path), and further plugs (addr + offset_to_XDP_packet (the metadata size, in other words)) into the TX descriptor 'addr' so that lower layers (e.g. the driver) can access the XDP packet as always.
(Note also that the TX descriptor 'len' would exclude the md size)

> Maybe I misunderstood the code, but it looks like 'addr' (xsk_umem__get_data(umem->buffer, addr)) points to metadata area, is this correct?
> 

No, more specifically, it is the user supplied UMEM chunk 'addr'. 

> (and to skip this the code does + MD_SIZE, before memcpy)

Since the packet is written (by the application) to immediately follow the metadata (both stored on the chunk), the code does that (+ MD_SIZE).

> 
> One problem/challenge with AF_XDP is that we don't have room in struct xdp_desc to store info on the size of the metadata area.  Bjørn came up with the idea of having btf_id as last member (access able via minus 4 bytes), as this tells the kernel the size of metadata area.

Yes, the RFC follows this idea and hence btf_id is the last member of struct xdp_user_tx_metadata.

> 
> Maybe you have come up with a better solution?
> (of making the metadata area size dynamic)
> 
> --Jesper
> 


      reply	other threads:[~2021-08-19 19:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-03 17:10 [RFC bpf-next 0/5] SO_TXTIME support in AF_XDP Kishen Maloor
2021-08-03 17:10 ` [RFC bpf-next 1/5] net: xdp: " Kishen Maloor
2021-08-03 17:10 ` [RFC bpf-next 2/5] libbpf: " Kishen Maloor
2021-08-06 23:08   ` Andrii Nakryiko
2021-08-18  9:49   ` Jesper Dangaard Brouer
2021-08-19 19:32     ` Kishen Maloor
2021-08-03 17:10 ` [RFC bpf-next 3/5] igc: Launchtime support in XDP Tx ZC path Kishen Maloor
2021-08-05 17:53   ` Kishen Maloor
2021-08-03 17:10 ` [RFC bpf-next 4/5] samples/bpf/xdpsock_user.c: Make get_nsecs() generic Kishen Maloor
2021-08-03 17:10 ` [RFC bpf-next 5/5] samples/bpf/xdpsock_user.c: Launchtime/TXTIME API usage Kishen Maloor
2021-08-18  8:54   ` Jesper Dangaard Brouer
2021-08-19 19:32     ` Kishen Maloor [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d15fdcb4-9b5f-ebf4-f5fe-5b16f06f2afc@intel.com \
    --to=kishen.maloor@intel.com \
    --cc=bjorn@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=hawk@kernel.org \
    --cc=jbrouer@redhat.com \
    --cc=jithu.joseph@intel.com \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    --subject='Re: [RFC bpf-next 5/5] samples/bpf/xdpsock_user.c: Launchtime/TXTIME API usage' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).