From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dvora Fuxbrumer Date: Tue, 2 Mar 2021 10:21:34 +0200 Subject: [Intel-wired-lan] [PATCH net-next v6 5/9] igc: Refactor Rx timestamp handling In-Reply-To: <20210210215848.24514-6-vedang.patel@intel.com> References: <20210210215848.24514-1-vedang.patel@intel.com> <20210210215848.24514-6-vedang.patel@intel.com> Message-ID: <17441c4a-ebc5-b8c5-a924-e9e5bae94a36@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: intel-wired-lan@osuosl.org List-ID: On 10/02/2021 23:58, Vedang Patel wrote: > From: Andre Guedes > > Refactor the Rx timestamp handling in preparation to land > XDP support. > > Rx timestamps are put in the Rx buffer by hardware, before the packet > data. When creating the xdp buffer, we will need to check the Rx > descriptor to determine if the buffer contains timestamp information > and consider the offset when setting xdp.data. > > The Rx descriptor check is already done in igc_construct_skb(). To > avoid code duplication, this patch moves the timestamp handling to > igc_clean_rx_irq() so both skb and xdp paths can reuse it. > > Signed-off-by: Andre Guedes > Reviewed-by: Maciej Fijalkowski > Signed-off-by: Vedang Patel > --- > drivers/net/ethernet/intel/igc/igc.h | 3 +-- > drivers/net/ethernet/intel/igc/igc_main.c | 30 +++++++++++++++-------- > drivers/net/ethernet/intel/igc/igc_ptp.c | 25 ++++++++++--------- > 3 files changed, 34 insertions(+), 24 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h > index fbd8c414c3c8..72544b5e747e 100644 > --- a/drivers/net/ethernet/intel/igc/igc.h > +++ b/drivers/net/ethernet/intel/igc/igc.h > @@ -547,8 +547,7 @@ void igc_ptp_init(struct igc_adapter *adapter); > void igc_ptp_reset(struct igc_adapter *adapter); > void igc_ptp_suspend(struct igc_adapter *adapter); > void igc_ptp_stop(struct igc_adapter *adapter); > -void igc_ptp_rx_pktstamp(struct igc_q_vector *q_vector, u32 *va, > - struct sk_buff *skb); > +ktime_t igc_ptp_rx_pktstamp(struct igc_adapter *adapter, u32 *buf); > int igc_ptp_set_ts_config(struct net_device *netdev, struct ifreq *ifr); > int igc_ptp_get_ts_config(struct net_device *netdev, struct ifreq *ifr); > void igc_ptp_tx_hang(struct igc_adapter *adapter); > diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c > index d1fc5baadf9b..5f77f1f43a7c 100644 > --- a/drivers/net/ethernet/intel/igc/igc_main.c > +++ b/drivers/net/ethernet/intel/igc/igc_main.c > @@ -1582,10 +1582,11 @@ static struct sk_buff *igc_build_skb(struct igc_ring *rx_ring, > > static struct sk_buff *igc_construct_skb(struct igc_ring *rx_ring, > struct igc_rx_buffer *rx_buffer, > - union igc_adv_rx_desc *rx_desc, > - unsigned int size) > + unsigned int size, int pkt_offset, > + ktime_t timestamp) > { > - void *va = page_address(rx_buffer->page) + rx_buffer->page_offset; > + void *va = page_address(rx_buffer->page) + rx_buffer->page_offset + > + pkt_offset; > unsigned int truesize = igc_get_rx_frame_truesize(rx_ring, size); > unsigned int headlen; > struct sk_buff *skb; > @@ -1598,11 +1599,8 @@ static struct sk_buff *igc_construct_skb(struct igc_ring *rx_ring, > if (unlikely(!skb)) > return NULL; > > - if (unlikely(igc_test_staterr(rx_desc, IGC_RXDADV_STAT_TSIP))) { > - igc_ptp_rx_pktstamp(rx_ring->q_vector, va, skb); > - va += IGC_TS_HDR_LEN; > - size -= IGC_TS_HDR_LEN; > - } > + if (timestamp) > + skb_hwtstamps(skb)->hwtstamp = timestamp; > > /* Determine available headroom for copy */ > headlen = size; > @@ -1896,6 +1894,8 @@ static int igc_clean_rx_irq(struct igc_q_vector *q_vector, const int budget) > while (likely(total_packets < budget)) { > union igc_adv_rx_desc *rx_desc; > struct igc_rx_buffer *rx_buffer; > + ktime_t timestamp = 0; > + int pkt_offset = 0; > unsigned int size; > > /* return some buffers to hardware, one at a time is too slow */ > @@ -1917,14 +1917,24 @@ static int igc_clean_rx_irq(struct igc_q_vector *q_vector, const int budget) > > rx_buffer = igc_get_rx_buffer(rx_ring, size); > > + if (igc_test_staterr(rx_desc, IGC_RXDADV_STAT_TSIP)) { > + void *pktbuf = page_address(rx_buffer->page) + > + rx_buffer->page_offset; > + > + timestamp = igc_ptp_rx_pktstamp(q_vector->adapter, > + pktbuf); > + pkt_offset = IGC_TS_HDR_LEN; > + size -= IGC_TS_HDR_LEN; > + } > + > /* retrieve a buffer from the ring */ > if (skb) > igc_add_rx_frag(rx_ring, rx_buffer, skb, size); > else if (ring_uses_build_skb(rx_ring)) > skb = igc_build_skb(rx_ring, rx_buffer, rx_desc, size); > else > - skb = igc_construct_skb(rx_ring, rx_buffer, > - rx_desc, size); > + skb = igc_construct_skb(rx_ring, rx_buffer, size, > + pkt_offset, timestamp); > > /* exit if we failed to retrieve a buffer */ > if (!skb) { > diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c > index b6a640bfc991..7301a7b3fadc 100644 > --- a/drivers/net/ethernet/intel/igc/igc_ptp.c > +++ b/drivers/net/ethernet/intel/igc/igc_ptp.c > @@ -153,20 +153,20 @@ static void igc_ptp_systim_to_hwtstamp(struct igc_adapter *adapter, > > /** > * igc_ptp_rx_pktstamp - Retrieve timestamp from Rx packet buffer > - * @q_vector: Pointer to interrupt specific structure > - * @va: Pointer to address containing Rx buffer > - * @skb: Buffer containing timestamp and packet > + * @adapter: Pointer to adapter the packet buffer belongs to > + * @buf: Pointer to packet buffer > * > * This function retrieves the timestamp saved in the beginning of packet > * buffer. While two timestamps are available, one in timer0 reference and the > * other in timer1 reference, this function considers only the timestamp in > * timer0 reference. > + * > + * Returns timestamp value. > */ > -void igc_ptp_rx_pktstamp(struct igc_q_vector *q_vector, u32 *va, > - struct sk_buff *skb) > +ktime_t igc_ptp_rx_pktstamp(struct igc_adapter *adapter, u32 *buf) > { > - struct igc_adapter *adapter = q_vector->adapter; > - u64 regval; > + ktime_t timestamp; > + u32 secs, nsecs; > int adjust; > > /* Timestamps are saved in little endian at the beginning of the packet > @@ -178,9 +178,10 @@ void igc_ptp_rx_pktstamp(struct igc_q_vector *q_vector, u32 *va, > * SYSTIML holds the nanoseconds part while SYSTIMH holds the seconds > * part of the timestamp. > */ > - regval = le32_to_cpu(va[2]); > - regval |= (u64)le32_to_cpu(va[3]) << 32; > - igc_ptp_systim_to_hwtstamp(adapter, skb_hwtstamps(skb), regval); > + nsecs = le32_to_cpu(buf[2]); > + secs = le32_to_cpu(buf[3]); > + > + timestamp = ktime_set(secs, nsecs); > > /* Adjust timestamp for the RX latency based on link speed */ > switch (adapter->link_speed) { > @@ -201,8 +202,8 @@ void igc_ptp_rx_pktstamp(struct igc_q_vector *q_vector, u32 *va, > netdev_warn_once(adapter->netdev, "Imprecise timestamp\n"); > break; > } > - skb_hwtstamps(skb)->hwtstamp = > - ktime_sub_ns(skb_hwtstamps(skb)->hwtstamp, adjust); > + > + return ktime_sub_ns(timestamp, adjust); > } > > static void igc_ptp_disable_rx_timestamp(struct igc_adapter *adapter) > Tested-by: Dvora Fuxbrumer