| Message ID | 20241216064720.931522-10-faizal.abdul.rahim@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | igc: Add support for Frame Preemption feature in IGC | expand |
On Mon, Dec 16, 2024 at 01:47:20AM -0500, Faizal Rahim wrote: > Implemented "ethtool --include-statistics --show-mm" callback for IGC. > > Tested preemption scenario to check preemption statistics: > 1) Trigger verification handshake on both boards: > $ sudo ethtool --set-mm enp1s0 pmac-enabled on > $ sudo ethtool --set-mm enp1s0 tx-enabled on > $ sudo ethtool --set-mm enp1s0 verify-enabled on > 2) Set preemptible or express queue in taprio for tx board: > $ sudo tc qdisc replace dev enp1s0 parent root handle 100 taprio \ > num_tc 4 map 0 1 2 3 0 0 0 0 0 0 0 0 0 0 0 0 \ > queues 1@0 1@1 1@2 1@3 base-time 0 sched-entry S F 100000 \ > fp E E P P Hmm, the prio_tc_map pattern changed since the last time I looked at igc examples? It was in decreasing order before? How do you handle backwards compatibility with the Tx ring strict priority default configuration? I haven't downloaded the entire set locally, will do so later. > 3) Send large size packets on preemptible queue > 4) Send small size packets on express queue to preempt packets in > preemptible queue > 5) Show preemption statistics on the receiving board: > $ ethtool --include-statistics --show-mm enp1s0 > MAC Merge layer state for enp1s0: > pMAC enabled: on > TX enabled: on > TX active: on > TX minimum fragment size: 252 > RX minimum fragment size: 252 > Verify enabled: on > Verify time: 128 > Max verify time: 128 > Verification status: SUCCEEDED > Statistics: > MACMergeFrameAssErrorCount: 0 > MACMergeFrameSmdErrorCount: 0 > MACMergeFrameAssOkCount: 511 > MACMergeFragCountRx: 764 > MACMergeFragCountTx: 0 > MACMergeHoldCount: 0 > > Co-developed-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> > Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> > Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com> > --- > drivers/net/ethernet/intel/igc/igc_ethtool.c | 40 ++++++++++++++++++++ > drivers/net/ethernet/intel/igc/igc_regs.h | 19 ++++++++++ > 2 files changed, 59 insertions(+) > > diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c > index 16aa6e4e1727..90a9dbb0d901 100644 > --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c > +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c > @@ -1835,6 +1835,45 @@ static int igc_ethtool_set_mm(struct net_device *netdev, > return igc_tsn_offload_apply(adapter); > } > > +/** > + * igc_ethtool_get_frame_ass_error - Get the frame assembly error count. > + * @dev: Pointer to the net_device structure. > + * @return: The count of frame assembly errors. I may be wrong, but I think the syntax for kernel-doc is "Returns: " > + */ > +static u64 igc_ethtool_get_frame_ass_error(struct net_device *dev) > +{ > + struct igc_adapter *adapter = netdev_priv(dev); > + u32 ooo_smdc, ooo_frame_cnt, ooo_frag_cnt; /* Out of order statistics */ > + struct igc_hw *hw = &adapter->hw; > + u32 miss_frame_frag_cnt; > + u32 reg_value; > + > + reg_value = rd32(IGC_PRMEXPRCNT); > + ooo_smdc = reg_value & IGC_PRMEXPRCNT_OOO_SMDC; > + ooo_frame_cnt = (reg_value & IGC_PRMEXPRCNT_OOO_FRAME_CNT) > + >> IGC_PRMEXPRCNT_OOO_FRAME_CNT_SHIFT; > + ooo_frag_cnt = (reg_value & IGC_PRMEXPRCNT_OOO_FRAG_CNT) > + >> IGC_PRMEXPRCNT_OOO_FRAG_CNT_SHIFT; > + miss_frame_frag_cnt = (reg_value & IGC_PRMEXPRCNT_MISS_FRAME_FRAG_CNT) > + >> IGC_PRMEXPRCNT_MISS_FRAME_FRAG_CNT_SHIFT; Candidates for FIELD_GET()? > + > + return ooo_smdc + ooo_frame_cnt + ooo_frag_cnt + miss_frame_frag_cnt; > +} > + > +static void igc_ethtool_get_mm_stats(struct net_device *dev, > + struct ethtool_mm_stats *stats) > +{ > + struct igc_adapter *adapter = netdev_priv(dev); > + struct igc_hw *hw = &adapter->hw; > + > + stats->MACMergeFrameAssErrorCount = igc_ethtool_get_frame_ass_error(dev); > + stats->MACMergeFrameSmdErrorCount = 0; /* Not available in IGC */ > + stats->MACMergeFrameAssOkCount = rd32(IGC_PRMPTDRCNT); > + stats->MACMergeFragCountRx = rd32(IGC_PRMEVNTRCNT); > + stats->MACMergeFragCountTx = rd32(IGC_PRMEVNTTCNT); > + stats->MACMergeHoldCount = 0; /* Not available in IGC */ Don't report counters as zero when in reality you don't know. Just don't assign values to these. mm_prepare_data() -> ethtool_stats_init() presets them to 0xffffffffffffffff (ETHTOOL_STAT_NOT_SET), and mm_put_stats() -> mm_put_stat() detects whether they are still equal to this value, and if they are, does not report netlink attributes for them. > +} > + > static int igc_ethtool_get_link_ksettings(struct net_device *netdev, > struct ethtool_link_ksettings *cmd) > { > @@ -2124,6 +2163,7 @@ static const struct ethtool_ops igc_ethtool_ops = { > .get_channels = igc_ethtool_get_channels, > .get_mm = igc_ethtool_get_mm, > .set_mm = igc_ethtool_set_mm, > + .get_mm_stats = igc_ethtool_get_mm_stats, > .set_channels = igc_ethtool_set_channels, > .get_priv_flags = igc_ethtool_get_priv_flags, > .set_priv_flags = igc_ethtool_set_priv_flags, > diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h b/drivers/net/ethernet/intel/igc/igc_regs.h > index 12ddc5793651..f40946cce35a 100644 > --- a/drivers/net/ethernet/intel/igc/igc_regs.h > +++ b/drivers/net/ethernet/intel/igc/igc_regs.h > @@ -222,6 +222,25 @@ > > #define IGC_FTQF(_n) (0x059E0 + (4 * (_n))) /* 5-tuple Queue Fltr */ > > +/* Time sync registers - preemption statistics */ > +#define IGC_PRMEVNTTCNT 0x04298 /* TX Preemption event counter */ > +#define IGC_PRMEVNTRCNT 0x0429C /* RX Preemption event counter */ > +#define IGC_PRMPTDRCNT 0x04284 /* Good RX Preempted Packets */ > + > + /* Preemption Exception Counter */ > +#define IGC_PRMEXPRCNT 0x042A0 > +/* Received out of order packets with SMD-C and NOT ReumeRx */ > +#define IGC_PRMEXPRCNT_OOO_SMDC 0x000000FF > +/* Received out of order packets with SMD-C and wrong Frame CNT */ > +#define IGC_PRMEXPRCNT_OOO_FRAME_CNT 0x0000FF00 > +#define IGC_PRMEXPRCNT_OOO_FRAME_CNT_SHIFT 8 > +/* Received out of order packets with SMD-C and wrong Frag CNT */ > +#define IGC_PRMEXPRCNT_OOO_FRAG_CNT 0x00FF0000 > +#define IGC_PRMEXPRCNT_OOO_FRAG_CNT_SHIFT 16 > +/* Received packets with SMD-S and ReumeRx */ What is ReumeRx? > +#define IGC_PRMEXPRCNT_MISS_FRAME_FRAG_CNT 0xFF000000 > +#define IGC_PRMEXPRCNT_MISS_FRAME_FRAG_CNT_SHIFT 24 > + > /* Transmit Scheduling Registers */ > #define IGC_TQAVCTRL 0x3570 > #define IGC_TXQCTL(_n) (0x3344 + 0x4 * (_n)) > -- > 2.25.1 > >
On 17/12/2024 12:05 am, Vladimir Oltean wrote: > On Mon, Dec 16, 2024 at 01:47:20AM -0500, Faizal Rahim wrote: >> Implemented "ethtool --include-statistics --show-mm" callback for IGC. >> >> Tested preemption scenario to check preemption statistics: >> 1) Trigger verification handshake on both boards: >> $ sudo ethtool --set-mm enp1s0 pmac-enabled on >> $ sudo ethtool --set-mm enp1s0 tx-enabled on >> $ sudo ethtool --set-mm enp1s0 verify-enabled on >> 2) Set preemptible or express queue in taprio for tx board: >> $ sudo tc qdisc replace dev enp1s0 parent root handle 100 taprio \ >> num_tc 4 map 0 1 2 3 0 0 0 0 0 0 0 0 0 0 0 0 \ >> queues 1@0 1@1 1@2 1@3 base-time 0 sched-entry S F 100000 \ >> fp E E P P > > Hmm, the prio_tc_map pattern changed since the last time I looked at igc > examples? It was in decreasing order before? How do you handle backwards > compatibility with the Tx ring strict priority default configuration? > I haven't downloaded the entire set locally, will do so later. > I tested like this for i226: CMD=( "tc qdisc replace dev $IFACE parent root handle 100 taprio " "num_tc 4 " "map 3 2 1 0 3 3 3 3 3 3 3 3 3 3 3 3 " "queues 1@0 1@1 1@2 1@3 " "base-time $BASE " "${SCHED_ENTRY[*]} " "flags 0x1 " "txtime-delay $TXTIME_DELAY " "clockid CLOCK_TAI " But I mistakenly copied the mapping from a different scenario where socket priority -> tc -> hw_queue mapping is not important to my test objective in that scenario. I'll update the description to use decreasing order then. >> + >> + return ooo_smdc + ooo_frame_cnt + ooo_frag_cnt + miss_frame_frag_cnt; >> +} >> + >> +static void igc_ethtool_get_mm_stats(struct net_device *dev, >> + struct ethtool_mm_stats *stats) >> +{ >> + struct igc_adapter *adapter = netdev_priv(dev); >> + struct igc_hw *hw = &adapter->hw; >> + >> + stats->MACMergeFrameAssErrorCount = igc_ethtool_get_frame_ass_error(dev); >> + stats->MACMergeFrameSmdErrorCount = 0; /* Not available in IGC */ >> + stats->MACMergeFrameAssOkCount = rd32(IGC_PRMPTDRCNT); >> + stats->MACMergeFragCountRx = rd32(IGC_PRMEVNTRCNT); >> + stats->MACMergeFragCountTx = rd32(IGC_PRMEVNTTCNT); >> + stats->MACMergeHoldCount = 0; /* Not available in IGC */ > > Don't report counters as zero when in reality you don't know. > > Just don't assign values to these. mm_prepare_data() -> ethtool_stats_init() > presets them to 0xffffffffffffffff (ETHTOOL_STAT_NOT_SET), and > mm_put_stats() -> mm_put_stat() detects whether they are still equal to > this value, and if they are, does not report netlink attributes for them. > Got it. >> diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h b/drivers/net/ethernet/intel/igc/igc_regs.h >> index 12ddc5793651..f40946cce35a 100644 >> --- a/drivers/net/ethernet/intel/igc/igc_regs.h >> +++ b/drivers/net/ethernet/intel/igc/igc_regs.h >> @@ -222,6 +222,25 @@ >> >> #define IGC_FTQF(_n) (0x059E0 + (4 * (_n))) /* 5-tuple Queue Fltr */ >> >> +/* Time sync registers - preemption statistics */ >> +#define IGC_PRMEVNTTCNT 0x04298 /* TX Preemption event counter */ >> +#define IGC_PRMEVNTRCNT 0x0429C /* RX Preemption event counter */ >> +#define IGC_PRMPTDRCNT 0x04284 /* Good RX Preempted Packets */ >> + >> + /* Preemption Exception Counter */ >> +#define IGC_PRMEXPRCNT 0x042A0 >> +/* Received out of order packets with SMD-C and NOT ReumeRx */ >> +#define IGC_PRMEXPRCNT_OOO_SMDC 0x000000FF >> +/* Received out of order packets with SMD-C and wrong Frame CNT */ >> +#define IGC_PRMEXPRCNT_OOO_FRAME_CNT 0x0000FF00 >> +#define IGC_PRMEXPRCNT_OOO_FRAME_CNT_SHIFT 8 >> +/* Received out of order packets with SMD-C and wrong Frag CNT */ >> +#define IGC_PRMEXPRCNT_OOO_FRAG_CNT 0x00FF0000 >> +#define IGC_PRMEXPRCNT_OOO_FRAG_CNT_SHIFT 16 >> +/* Received packets with SMD-S and ReumeRx */ > > What is ReumeRx? Resume receive. It’s a typo in the i226 documentation that I (shamelessly) copied into the code without checking properly. It's meant to indicate that an RX flag in the i226 firmware is set. I’ll remove the 'ResumeRX' part since it adds confusion.
diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c index 16aa6e4e1727..90a9dbb0d901 100644 --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c @@ -1835,6 +1835,45 @@ static int igc_ethtool_set_mm(struct net_device *netdev, return igc_tsn_offload_apply(adapter); } +/** + * igc_ethtool_get_frame_ass_error - Get the frame assembly error count. + * @dev: Pointer to the net_device structure. + * @return: The count of frame assembly errors. + */ +static u64 igc_ethtool_get_frame_ass_error(struct net_device *dev) +{ + struct igc_adapter *adapter = netdev_priv(dev); + u32 ooo_smdc, ooo_frame_cnt, ooo_frag_cnt; /* Out of order statistics */ + struct igc_hw *hw = &adapter->hw; + u32 miss_frame_frag_cnt; + u32 reg_value; + + reg_value = rd32(IGC_PRMEXPRCNT); + ooo_smdc = reg_value & IGC_PRMEXPRCNT_OOO_SMDC; + ooo_frame_cnt = (reg_value & IGC_PRMEXPRCNT_OOO_FRAME_CNT) + >> IGC_PRMEXPRCNT_OOO_FRAME_CNT_SHIFT; + ooo_frag_cnt = (reg_value & IGC_PRMEXPRCNT_OOO_FRAG_CNT) + >> IGC_PRMEXPRCNT_OOO_FRAG_CNT_SHIFT; + miss_frame_frag_cnt = (reg_value & IGC_PRMEXPRCNT_MISS_FRAME_FRAG_CNT) + >> IGC_PRMEXPRCNT_MISS_FRAME_FRAG_CNT_SHIFT; + + return ooo_smdc + ooo_frame_cnt + ooo_frag_cnt + miss_frame_frag_cnt; +} + +static void igc_ethtool_get_mm_stats(struct net_device *dev, + struct ethtool_mm_stats *stats) +{ + struct igc_adapter *adapter = netdev_priv(dev); + struct igc_hw *hw = &adapter->hw; + + stats->MACMergeFrameAssErrorCount = igc_ethtool_get_frame_ass_error(dev); + stats->MACMergeFrameSmdErrorCount = 0; /* Not available in IGC */ + stats->MACMergeFrameAssOkCount = rd32(IGC_PRMPTDRCNT); + stats->MACMergeFragCountRx = rd32(IGC_PRMEVNTRCNT); + stats->MACMergeFragCountTx = rd32(IGC_PRMEVNTTCNT); + stats->MACMergeHoldCount = 0; /* Not available in IGC */ +} + static int igc_ethtool_get_link_ksettings(struct net_device *netdev, struct ethtool_link_ksettings *cmd) { @@ -2124,6 +2163,7 @@ static const struct ethtool_ops igc_ethtool_ops = { .get_channels = igc_ethtool_get_channels, .get_mm = igc_ethtool_get_mm, .set_mm = igc_ethtool_set_mm, + .get_mm_stats = igc_ethtool_get_mm_stats, .set_channels = igc_ethtool_set_channels, .get_priv_flags = igc_ethtool_get_priv_flags, .set_priv_flags = igc_ethtool_set_priv_flags, diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h b/drivers/net/ethernet/intel/igc/igc_regs.h index 12ddc5793651..f40946cce35a 100644 --- a/drivers/net/ethernet/intel/igc/igc_regs.h +++ b/drivers/net/ethernet/intel/igc/igc_regs.h @@ -222,6 +222,25 @@ #define IGC_FTQF(_n) (0x059E0 + (4 * (_n))) /* 5-tuple Queue Fltr */ +/* Time sync registers - preemption statistics */ +#define IGC_PRMEVNTTCNT 0x04298 /* TX Preemption event counter */ +#define IGC_PRMEVNTRCNT 0x0429C /* RX Preemption event counter */ +#define IGC_PRMPTDRCNT 0x04284 /* Good RX Preempted Packets */ + + /* Preemption Exception Counter */ +#define IGC_PRMEXPRCNT 0x042A0 +/* Received out of order packets with SMD-C and NOT ReumeRx */ +#define IGC_PRMEXPRCNT_OOO_SMDC 0x000000FF +/* Received out of order packets with SMD-C and wrong Frame CNT */ +#define IGC_PRMEXPRCNT_OOO_FRAME_CNT 0x0000FF00 +#define IGC_PRMEXPRCNT_OOO_FRAME_CNT_SHIFT 8 +/* Received out of order packets with SMD-C and wrong Frag CNT */ +#define IGC_PRMEXPRCNT_OOO_FRAG_CNT 0x00FF0000 +#define IGC_PRMEXPRCNT_OOO_FRAG_CNT_SHIFT 16 +/* Received packets with SMD-S and ReumeRx */ +#define IGC_PRMEXPRCNT_MISS_FRAME_FRAG_CNT 0xFF000000 +#define IGC_PRMEXPRCNT_MISS_FRAME_FRAG_CNT_SHIFT 24 + /* Transmit Scheduling Registers */ #define IGC_TQAVCTRL 0x3570 #define IGC_TXQCTL(_n) (0x3344 + 0x4 * (_n))
Implemented "ethtool --include-statistics --show-mm" callback for IGC. Tested preemption scenario to check preemption statistics: 1) Trigger verification handshake on both boards: $ sudo ethtool --set-mm enp1s0 pmac-enabled on $ sudo ethtool --set-mm enp1s0 tx-enabled on $ sudo ethtool --set-mm enp1s0 verify-enabled on 2) Set preemptible or express queue in taprio for tx board: $ sudo tc qdisc replace dev enp1s0 parent root handle 100 taprio \ num_tc 4 map 0 1 2 3 0 0 0 0 0 0 0 0 0 0 0 0 \ queues 1@0 1@1 1@2 1@3 base-time 0 sched-entry S F 100000 \ fp E E P P 3) Send large size packets on preemptible queue 4) Send small size packets on express queue to preempt packets in preemptible queue 5) Show preemption statistics on the receiving board: $ ethtool --include-statistics --show-mm enp1s0 MAC Merge layer state for enp1s0: pMAC enabled: on TX enabled: on TX active: on TX minimum fragment size: 252 RX minimum fragment size: 252 Verify enabled: on Verify time: 128 Max verify time: 128 Verification status: SUCCEEDED Statistics: MACMergeFrameAssErrorCount: 0 MACMergeFrameSmdErrorCount: 0 MACMergeFrameAssOkCount: 511 MACMergeFragCountRx: 764 MACMergeFragCountTx: 0 MACMergeHoldCount: 0 Co-developed-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com> --- drivers/net/ethernet/intel/igc/igc_ethtool.c | 40 ++++++++++++++++++++ drivers/net/ethernet/intel/igc/igc_regs.h | 19 ++++++++++ 2 files changed, 59 insertions(+)