netdev - Re: [PATCH net] igb: Fix XDP with PTP enabled

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <871rbbhh90.fsf@kurt>
Date:   Thu, 15 Apr 2021 14:19:55 +0200
From:   Kurt Kanzenbach <kurt@...utronix.de>
To:     Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
Cc:     Jesse Brandeburg <jesse.brandeburg@...el.com>,
        Tony Nguyen <anthony.l.nguyen@...el.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        Sven Auhagen <sven.auhagen@...eatech.de>,
        intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
        bpf@...r.kernel.org,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Lorenzo Bianconi <lorenzo@...nel.org>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Richard Cochran <richardcochran@...il.com>
Subject: Re: [PATCH net] igb: Fix XDP with PTP enabled

On Thu Apr 15 2021, Lorenzo Bianconi wrote:
> [...]
>> @@ -8683,7 +8676,10 @@ static int igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)
>>  	while (likely(total_packets < budget)) {
>>  		union e1000_adv_rx_desc *rx_desc;
>>  		struct igb_rx_buffer *rx_buffer;
>> +		ktime_t timestamp = 0;
>> +		int pkt_offset = 0;
>>  		unsigned int size;
>> +		void *pktbuf;
>>  
>>  		/* return some buffers to hardware, one at a time is too slow */
>>  		if (cleaned_count >= IGB_RX_BUFFER_WRITE) {
>> @@ -8703,15 +8699,22 @@ static int igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)
>>  		dma_rmb();
>>  
>>  		rx_buffer = igb_get_rx_buffer(rx_ring, size, &rx_buf_pgcnt);
>> +		pktbuf = page_address(rx_buffer->page) + rx_buffer->page_offset;
>> +
>> +		/* pull rx packet timestamp if available */
>> +		if (igb_test_staterr(rx_desc, E1000_RXDADV_STAT_TSIP)) {
>> +			timestamp = igb_ptp_rx_pktstamp(rx_ring->q_vector,
>> +							pktbuf);
>> +			pkt_offset += IGB_TS_HDR_LEN;
>> +			size -= IGB_TS_HDR_LEN;
>> +		}
>>  
>>  		/* retrieve a buffer from the ring */
>>  		if (!skb) {
>> -			unsigned int offset = igb_rx_offset(rx_ring);
>> -			unsigned char *hard_start;
>> -
>> -			hard_start = page_address(rx_buffer->page) +
>> -				     rx_buffer->page_offset - offset;
>> -			xdp_prepare_buff(&xdp, hard_start, offset, size, true);
>> +			xdp.data = pktbuf + pkt_offset;
>> +			xdp.data_end = xdp.data + size;
>> +			xdp.data_meta = xdp.data;
>> +			xdp.data_hard_start = pktbuf - igb_rx_offset(rx_ring);
>
> in order to keep it aligned with other xdp drivers, I guess you can do something like:
>
> 			unsigned char *hard_start = pktbuf - igb_rx_offset(rx_ring);
> 			unsigned int offset = pkt_offset + igb_rx_offset(rx_ring);
>
> 			xdp_prepare_buff(&xdp, hard_start, offset, size, true);

This should work as well. I just kept it in sync with the igc driver,
because it doesn't use xdp_prepare_buff() either.

>
> Probably the compiler will optimize it.

Most likely.

Thanks,
Kurt

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)