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: <PH0PR11MB5830D7F46EE359FF9EF59CEAD89B9@PH0PR11MB5830.namprd11.prod.outlook.com> Date: Wed, 12 Apr 2023 15:37:01 +0000 From: "Song, Yoong Siang" <yoong.siang.song@...el.com> To: "Brouer, Jesper" <brouer@...hat.com>, "bpf@...r.kernel.org" <bpf@...r.kernel.org> CC: "Brouer, Jesper" <brouer@...hat.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Stanislav Fomichev <sdf@...gle.com>, "martin.lau@...nel.org" <martin.lau@...nel.org>, "ast@...nel.org" <ast@...nel.org>, "daniel@...earbox.net" <daniel@...earbox.net>, "Lobakin, Aleksander" <aleksander.lobakin@...el.com>, "Zaremba, Larysa" <larysa.zaremba@...el.com>, "xdp-hints@...-project.net" <xdp-hints@...-project.net>, "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "Ong, Boon Leong" <boon.leong.ong@...el.com>, "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>, "pabeni@...hat.com" <pabeni@...hat.com>, "Brandeburg, Jesse" <jesse.brandeburg@...el.com>, "kuba@...nel.org" <kuba@...nel.org>, "edumazet@...gle.com" <edumazet@...gle.com>, "john.fastabend@...il.com" <john.fastabend@...il.com>, "hawk@...nel.org" <hawk@...nel.org>, "davem@...emloft.net" <davem@...emloft.net> Subject: RE: [PATCH bpf-next V3 5/6] igc: add XDP hints kfuncs for RX timestamp On Wednesday, April 12, 2023 7:41 PM, Song Yoong Siang wrote: >On Thursday, March 23, 2023 12:02 AM , Jesper Dangaard Brouer ><brouer@...hat.com> wrote: >>The NIC hardware RX timestamping mechanism adds an optional tailored >>header before the MAC header containing packet reception time. Optional >>depending on RX descriptor TSIP status bit (IGC_RXDADV_STAT_TSIP). In >>case this bit is set driver does offset adjustments to packet data start and >extracts the timestamp. >> >>The timestamp need to be extracted before invoking the XDP bpf_prog, >>because this area just before the packet is also accessible by XDP via >>data_meta context pointer (and helper bpf_xdp_adjust_meta). Thus, an >>XDP bpf_prog can potentially overwrite this and corrupt data that we >>want to extract with the new kfunc for reading the timestamp. >> >>Signed-off-by: Jesper Dangaard Brouer <brouer@...hat.com> Tested-by: Song Yoong Siang <yoong.siang.song@...el.com> Tested Rx HWTS metadata on I226-LM (rev 04) NIC. Below are the detail of test steps and result. 1. Run xdp_hw_metadata tool. @DUT: sudo ./xdp_hw_metadata eth0 2. Enable Rx HWTS for all incoming packets. Note: This step should not needed, so it should be a HWTS enablement bug on igc driver. I will take a look on it. @DUT: sudo hwstamp_ctl -i eth0 -r 1 3. Set the ptp clock time from the system time using testptp tool. @DUT: sudo ./testptp -d /dev/ptp0 -s 4. Send UDP packet with 9091 port from link partner immediately after step 3. @LinkPartner: echo -n xdp | nc -u -q1 <Destination IPv4 addr> 9091 Result: xsk_ring_cons__peek: 1 0x559288636880: rx_desc[2]->addr=100000000202000 addr=202100 comp_addr=202000 rx_hash: 0x0E550050 rx_timestamp: 1677762906850259640 (sec:1677762906.8503) XDP RX-time: 1677762906850374981 (sec:1677762906.8504) delta sec:0.0001 (115.341 usec) AF_XDP time: 1677762906850396571 (sec:1677762906.8504) delta sec:0.0000 (21.590 usec) 0x559288636880: complete idx=514 addr=202000 >>--- >> drivers/net/ethernet/intel/igc/igc.h | 1 + >> drivers/net/ethernet/intel/igc/igc_main.c | 20 ++++++++++++++++++++ >> 2 files changed, 21 insertions(+) >> >>diff --git a/drivers/net/ethernet/intel/igc/igc.h >>b/drivers/net/ethernet/intel/igc/igc.h >>index bc67a52e47e8..29941734f1a1 100644 >>--- a/drivers/net/ethernet/intel/igc/igc.h >>+++ b/drivers/net/ethernet/intel/igc/igc.h >>@@ -503,6 +503,7 @@ struct igc_rx_buffer { struct igc_xdp_buff { >> struct xdp_buff xdp; >> union igc_adv_rx_desc *rx_desc; >>+ ktime_t rx_ts; /* data indication bit IGC_RXDADV_STAT_TSIP */ >> }; >> >> struct igc_q_vector { >>diff --git a/drivers/net/ethernet/intel/igc/igc_main.c >>b/drivers/net/ethernet/intel/igc/igc_main.c >>index a78d7e6bcfd6..f66285c85444 100644 >>--- a/drivers/net/ethernet/intel/igc/igc_main.c >>+++ b/drivers/net/ethernet/intel/igc/igc_main.c >>@@ -2539,6 +2539,7 @@ static int igc_clean_rx_irq(struct igc_q_vector >>*q_vector, const int budget) >> if (igc_test_staterr(rx_desc, IGC_RXDADV_STAT_TSIP)) { >> timestamp = igc_ptp_rx_pktstamp(q_vector->adapter, >> pktbuf); >>+ ctx.rx_ts = timestamp; >> pkt_offset = IGC_TS_HDR_LEN; >> size -= IGC_TS_HDR_LEN; >> } >>@@ -2727,6 +2728,7 @@ static int igc_clean_rx_irq_zc(struct >>igc_q_vector *q_vector, const int budget) >> if (igc_test_staterr(desc, IGC_RXDADV_STAT_TSIP)) { >> timestamp = igc_ptp_rx_pktstamp(q_vector->adapter, >> bi->xdp->data); >>+ ctx->rx_ts = timestamp; >> >> bi->xdp->data += IGC_TS_HDR_LEN; >> >>@@ -6481,6 +6483,23 @@ u32 igc_rd32(struct igc_hw *hw, u32 reg) >> return value; >> } >> >>+static int igc_xdp_rx_timestamp(const struct xdp_md *_ctx, u64 >>+*timestamp) { >>+ const struct igc_xdp_buff *ctx = (void *)_ctx; >>+ >>+ if (igc_test_staterr(ctx->rx_desc, IGC_RXDADV_STAT_TSIP)) { >>+ *timestamp = ctx->rx_ts; >>+ >>+ return 0; >>+ } >>+ >>+ return -ENODATA; >>+} >>+ >>+const struct xdp_metadata_ops igc_xdp_metadata_ops = { > >Hi Jesper, > >Since igc_xdp_metadata_ops is used on igc_main.c only, we can make it static to >avoid following build warning: > >drivers/net/ethernet/intel/igc/igc_main.c:6499:31: warning: symbol >'igc_xdp_metadata_ops' was not declared. Should it be static? > >Thanks & Regards >Siang > >>+ .xmo_rx_timestamp = igc_xdp_rx_timestamp, >>+}; >>+ >> /** >> * igc_probe - Device Initialization Routine >> * @pdev: PCI device information struct @@ -6554,6 +6573,7 @@ static >>int igc_probe(struct pci_dev *pdev, >> hw->hw_addr = adapter->io_addr; >> >> netdev->netdev_ops = &igc_netdev_ops; >>+ netdev->xdp_metadata_ops = &igc_xdp_metadata_ops; >> igc_ethtool_set_ops(netdev); >> netdev->watchdog_timeo = 5 * HZ; >> >>
Powered by blists - more mailing lists