Re: [Intel-wired-lan] Intel-wired-lan Digest, Vol 405, Issue 6

["Neftin, Sasha" <sasha.neftin@intel.com>]

On 1/24/2023 16:46, Zulkifli, Muhammad Husaini wrote:
> Hi Sasha,
> 
>> -----Original Message-----
>> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
>> intel-wired-lan-request@osuosl.org
>> Sent: Tuesday, 10 January, 2023 1:49 PM
>> To: intel-wired-lan@osuosl.org
>> Subject: Intel-wired-lan Digest, Vol 405, Issue 6
>>
>> Send Intel-wired-lan mailing list submissions to
>> 	intel-wired-lan@osuosl.org
>>
>> To subscribe or unsubscribe via the World Wide Web, visit
>> 	https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>> or, via email, send a message with subject or body 'help' to
>> 	intel-wired-lan-request@osuosl.org
>>
>> You can reach the person managing the list at
>> 	intel-wired-lan-owner@osuosl.org
>>
>> When replying, please edit your Subject line so it is more specific than "Re:
>> Contents of Intel-wired-lan digest..."
>>
>>
>> Today's Topics:
>>
>>     1. [PATCH 1/1] ice: WiP support for BIG TCP packets
>>        (Pawel Chmielewski)
>>     2. Re: [PATCH 1/1] ice: WiP support for BIG TCP packets
>>        (Alexander H Duyck)
>>     3. [tnguy-net-queue:dev-queue] BUILD SUCCESS
>>        4cb425d20a6ddbf9fd40989c31f5c6f8f304dc35 (kernel test robot)
>>     4. [PATCH net v1 1/1] igc: Add ndo_tx_timeout support (Sasha Neftin)
>>
>>
>> ----------------------------------------------------------------------
>>
>> Message: 1
>> Date: Mon,  9 Jan 2023 17:18:33 +0100
>> From: Pawel Chmielewski <pawel.chmielewski@intel.com>
>> To: netdev@vger.kernel.org
>> Cc: intel-wired-lan@osuosl.org, Pawel Chmielewski
>> 	<pawel.chmielewski@intel.com>
>> Subject: [Intel-wired-lan] [PATCH 1/1] ice: WiP support for BIG TCP
>> 	packets
>> Message-ID: <20230109161833.223510-1-pawel.chmielewski@intel.com>
>>
>> This patch is a proof of concept for testing BIG TCP feature in ice driver.
>> Please see letter below.
>>
>> Signed-off-by: Pawel Chmielewski <pawel.chmielewski@intel.com>
>> ---
>> Hi All
>> I'm writing on the list, as you may be able to provide me some feedback.
>> I want to enable BIG TCP feature in intel ice drive, but I think I'm missing
>> something.
>> In the code itself, I've set 128k as a maximum tso size for the netif, and
>> added stripping the HBH option from the header.
>> For testing purposes, gso_max_size & gro_max_size were set to 128k and
>> mtu to 9000.
>> I've assumed that the ice tso offload will do the rest of the job.
>> However- while running netperf TCP_RR and TCP_STREAM tests, I saw that
>> only up to ~20% of the transmitted test packets have the specified size.
>> Other packets to be transmitted, appear from the stack as splitted.
>>
>> I've been running the following testcases:
>> netperf -t TCP_RR -H 2001:db8:0:f101::1  -- -r80000,80000 -O
>> MIN_LATENCY,P90_LATENCY,P99_LATENCY,THROUGHPUT
>> netperf -l-1 -t TCP_STREAM -H 2001:db8:0:f101::1  -- -m 128K -O
>> MIN_LATENCY,P90_LATENCY,P99_LATENCY,THROUGHPUT
>> I suspected a shrinking tcp window size, but sniffing with tcpdump showed
>> rather big scaling factor (usually 128x).
>> Apart from using netperf, I also tried a simple IPv6 user space application
>> (with SO_SNDBUF option set to 192k and TCP_WINDOW_CLAMP to 96k) -
>> similar results.
>>
>> I'd be very grateful for any feedback/suggestions
>>
>> Pawel
>> ---
>>   drivers/net/ethernet/intel/ice/ice_main.c | 4 ++++
>> drivers/net/ethernet/intel/ice/ice_txrx.c | 9 +++++++++
>>   2 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
>> b/drivers/net/ethernet/intel/ice/ice_main.c
>> index 2b23b4714a26..4e657820e55d 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>> @@ -48,6 +48,8 @@ static DEFINE_IDA(ice_aux_ida);
>> DEFINE_STATIC_KEY_FALSE(ice_xdp_locking_key);
>>   EXPORT_SYMBOL(ice_xdp_locking_key);
>>
>> +#define ICE_MAX_TSO_SIZE 131072
>> +
>>   /**
>>    * ice_hw_to_dev - Get device pointer from the hardware structure
>>    * @hw: pointer to the device HW structure @@ -3422,6 +3424,8 @@ static
>> void ice_set_netdev_features(struct net_device *netdev)
>>   	 * be changed at runtime
>>   	 */
>>   	netdev->hw_features |= NETIF_F_RXFCS;
>> +
>> +	netif_set_tso_max_size(netdev, ICE_MAX_TSO_SIZE);
>>   }
>>
>>   /**
>> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c
>> b/drivers/net/ethernet/intel/ice/ice_txrx.c
>> index 086f0b3ab68d..7e0ac483cad9 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_txrx.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
>> @@ -23,6 +23,9 @@
>>   #define FDIR_DESC_RXDID 0x40
>>   #define ICE_FDIR_CLEAN_DELAY 10
>>
>> +#define HBH_HDR_SIZE sizeof(struct hop_jumbo_hdr) #define
>> HBH_OFFSET
>> +ETH_HLEN + sizeof(struct ipv6hdr)
>> +
>>   /**
>>    * ice_prgm_fdir_fltr - Program a Flow Director filter
>>    * @vsi: VSI to send dummy packet
>> @@ -2300,6 +2303,12 @@ ice_xmit_frame_ring(struct sk_buff *skb, struct
>> ice_tx_ring *tx_ring)
>>
>>   	ice_trace(xmit_frame_ring, tx_ring, skb);
>>
>> +	if (ipv6_has_hopopt_jumbo(skb)) {
>> +		memmove(skb->data + HBH_HDR_SIZE, skb->data,
>> HBH_OFFSET);
>> +		__skb_pull(skb, HBH_HDR_SIZE);
>> +		skb_reset_mac_header(skb);
>> +	}
>> +
>>   	count = ice_xmit_desc_count(skb);
>>   	if (ice_chk_linearize(skb, count)) {
>>   		if (__skb_linearize(skb))
>> --
>> 2.37.3
>>
>>
>>
>> ------------------------------
>>
>> Message: 2
>> Date: Mon, 09 Jan 2023 10:22:22 -0800
>> From: Alexander H Duyck <alexander.duyck@gmail.com>
>> To: Pawel Chmielewski <pawel.chmielewski@intel.com>,
>> 	netdev@vger.kernel.org
>> Cc: intel-wired-lan@osuosl.org
>> Subject: Re: [Intel-wired-lan] [PATCH 1/1] ice: WiP support for BIG
>> 	TCP packets
>> Message-ID:
>> <9b68abc5e8613e02207e9c0c3619b1b07bc5bb8c.camel@gmail.com>
>> Content-Type: text/plain; charset="UTF-8"
>>
>> On Mon, 2023-01-09 at 17:18 +0100, Pawel Chmielewski wrote:
>>> This patch is a proof of concept for testing BIG TCP feature in ice driver.
>>> Please see letter below.
>>>
>>> Signed-off-by: Pawel Chmielewski <pawel.chmielewski@intel.com>
>>> ---
>>> Hi All
>>> I'm writing on the list, as you may be able to provide me some feedback.
>>> I want to enable BIG TCP feature in intel ice drive, but I think I'm
>>> missing something.
>>> In the code itself, I've set 128k as a maximum tso size for the netif,
>>> and added stripping the HBH option from the header.
>>> For testing purposes, gso_max_size & gro_max_size were set to 128k and
>>> mtu to 9000.
>>> I've assumed that the ice tso offload will do the rest of the job.
>>> However- while running netperf TCP_RR and TCP_STREAM tests,
>>> I saw that only up to ~20% of the transmitted test packets have
>>> the specified size.
>>> Other packets to be transmitted, appear from the stack as splitted.
>>>
>>> I've been running the following testcases:
>>> netperf -t TCP_RR -H 2001:db8:0:f101::1  -- -r80000,80000 -O
>> MIN_LATENCY,P90_LATENCY,P99_LATENCY,THROUGHPUT
>>> netperf -l-1 -t TCP_STREAM -H 2001:db8:0:f101::1  -- -m 128K -O
>> MIN_LATENCY,P90_LATENCY,P99_LATENCY,THROUGHPUT
>>> I suspected a shrinking tcp window size, but sniffing with tcpdump showed
>> rather big scaling factor (usually 128x).
>>> Apart from using netperf, I also tried a simple IPv6 user space application
>>> (with SO_SNDBUF option set to 192k and TCP_WINDOW_CLAMP to 96k) -
>> similar results.
>>>
>>> I'd be very grateful for any feedback/suggestions
>>>
>>> Pawel
>>> ---
>>>   drivers/net/ethernet/intel/ice/ice_main.c | 4 ++++
>>>   drivers/net/ethernet/intel/ice/ice_txrx.c | 9 +++++++++
>>>   2 files changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
>> b/drivers/net/ethernet/intel/ice/ice_main.c
>>> index 2b23b4714a26..4e657820e55d 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>>> @@ -48,6 +48,8 @@ static DEFINE_IDA(ice_aux_ida);
>>>   DEFINE_STATIC_KEY_FALSE(ice_xdp_locking_key);
>>>   EXPORT_SYMBOL(ice_xdp_locking_key);
>>>
>>> +#define ICE_MAX_TSO_SIZE 131072
>>> +
>>>   /**
>>>    * ice_hw_to_dev - Get device pointer from the hardware structure
>>>    * @hw: pointer to the device HW structure
>>> @@ -3422,6 +3424,8 @@ static void ice_set_netdev_features(struct
>> net_device *netdev)
>>>   	 * be changed at runtime
>>>   	 */
>>>   	netdev->hw_features |= NETIF_F_RXFCS;
>>> +
>>> +	netif_set_tso_max_size(netdev, ICE_MAX_TSO_SIZE);
>>>   }
>>>
>>>   /**
>>> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c
>> b/drivers/net/ethernet/intel/ice/ice_txrx.c
>>> index 086f0b3ab68d..7e0ac483cad9 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_txrx.c
>>> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
>>> @@ -23,6 +23,9 @@
>>>   #define FDIR_DESC_RXDID 0x40
>>>   #define ICE_FDIR_CLEAN_DELAY 10
>>>
>>> +#define HBH_HDR_SIZE sizeof(struct hop_jumbo_hdr)
>>> +#define HBH_OFFSET ETH_HLEN + sizeof(struct ipv6hdr)
>>> +
>>>   /**
>>>    * ice_prgm_fdir_fltr - Program a Flow Director filter
>>>    * @vsi: VSI to send dummy packet
>>> @@ -2300,6 +2303,12 @@ ice_xmit_frame_ring(struct sk_buff *skb, struct
>> ice_tx_ring *tx_ring)
>>>
>>>   	ice_trace(xmit_frame_ring, tx_ring, skb);
>>>
>>> +	if (ipv6_has_hopopt_jumbo(skb)) {
>>> +		memmove(skb->data + HBH_HDR_SIZE, skb->data,
>> HBH_OFFSET);
>>> +		__skb_pull(skb, HBH_HDR_SIZE);
>>> +		skb_reset_mac_header(skb);
>>> +	}
>>> +
>>>   	count = ice_xmit_desc_count(skb);
>>>   	if (ice_chk_linearize(skb, count)) {
>>>   		if (__skb_linearize(skb))
>>
>> Your removal code here is forgetting to handle the network header. As a
>> result your frames will be pointer mangled in terms of header location.
>>
>> You might be better off using ipv6_hopopt_jumbo_remove() rather than
>> just coding your own bit to remove it.
>>
>>
>> ------------------------------
>>
>> Message: 3
>> Date: Tue, 10 Jan 2023 13:10:09 +0800
>> From: kernel test robot <lkp@intel.com>
>> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
>> Subject: [Intel-wired-lan] [tnguy-net-queue:dev-queue] BUILD SUCCESS
>> 	4cb425d20a6ddbf9fd40989c31f5c6f8f304dc35
>> Message-ID: <63bcf331.kDz0I6QwE35Zcf1P%lkp@intel.com>
>> Content-Type: text/plain; charset=us-ascii
>>
>> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-
>> queue.git dev-queue
>> branch HEAD: 4cb425d20a6ddbf9fd40989c31f5c6f8f304dc35  ice: fix out-of-
>> bounds KASAN warning in virtchnl
>>
>> elapsed time: 723m
>>
>> configs tested: 84
>> configs skipped: 2
>>
>> The following configs have been built successfully.
>> More configs may be tested in the coming days.
>>
>> gcc tested configs:
>> x86_64                            allnoconfig
>> arc                                 defconfig
>> alpha                               defconfig
>> powerpc                           allnoconfig
>> um                             i386_defconfig
>> um                           x86_64_defconfig
>> alpha                            allyesconfig
>> s390                             allmodconfig
>> m68k                             allmodconfig
>> arc                              allyesconfig
>> s390                                defconfig
>> m68k                             allyesconfig
>> x86_64                              defconfig
>> s390                             allyesconfig
>> i386                                defconfig
>> arm                                 defconfig
>> i386                             allyesconfig
>> x86_64               randconfig-a011-20230109
>> x86_64               randconfig-a013-20230109
>> i386                 randconfig-a011-20230109
>> x86_64               randconfig-a012-20230109
>> sh                               allmodconfig
>> i386                 randconfig-a013-20230109
>> x86_64                           rhel-8.3-bpf
>> x86_64                               rhel-8.3
>> x86_64               randconfig-a014-20230109
>> x86_64               randconfig-a016-20230109
>> x86_64               randconfig-a015-20230109
>> ia64                             allmodconfig
>> x86_64                           allyesconfig
>> i386                 randconfig-a012-20230109
>> x86_64                    rhel-8.3-kselftests
>> x86_64                           rhel-8.3-syz
>> x86_64                         rhel-8.3-kunit
>> x86_64                          rhel-8.3-func
>> i386                 randconfig-a014-20230109
>> x86_64                           rhel-8.3-kvm
>> i386                 randconfig-a016-20230109
>> i386                 randconfig-a015-20230109
>> mips                             allyesconfig
>> powerpc                          allmodconfig
>> arm64                            allyesconfig
>> arm                              allyesconfig
>> riscv                randconfig-r042-20230109
>> s390                 randconfig-r044-20230109
>> arm                  randconfig-r046-20230108
>> arc                  randconfig-r043-20230108
>> arc                  randconfig-r043-20230109
>> i386                          randconfig-c001
>> arm                        trizeps4_defconfig
>> arm                         s3c6400_defconfig
>> sh                               alldefconfig
>> ia64                                defconfig
>> riscv                    nommu_virt_defconfig
>> riscv                          rv32_defconfig
>> riscv                    nommu_k210_defconfig
>> riscv                             allnoconfig
>> i386                   debian-10.3-kselftests
>> i386                              debian-10.3
>> m68k                           sun3_defconfig
>> parisc                generic-32bit_defconfig
>> sh                     sh7710voipgw_defconfig
>> mips                            gpr_defconfig
>>
>> clang tested configs:
>> i386                 randconfig-a004-20230109
>> x86_64               randconfig-a003-20230109
>> x86_64               randconfig-a002-20230109
>> x86_64                          rhel-8.3-rust
>> x86_64               randconfig-a004-20230109
>> hexagon              randconfig-r045-20230109
>> i386                 randconfig-a002-20230109
>> x86_64               randconfig-a005-20230109
>> i386                 randconfig-a003-20230109
>> x86_64               randconfig-a001-20230109
>> arm                  randconfig-r046-20230109
>> riscv                randconfig-r042-20230108
>> x86_64               randconfig-a006-20230109
>> hexagon              randconfig-r041-20230108
>> i386                 randconfig-a006-20230109
>> i386                 randconfig-a001-20230109
>> hexagon              randconfig-r041-20230109
>> i386                 randconfig-a005-20230109
>> hexagon              randconfig-r045-20230108
>> s390                 randconfig-r044-20230108
>> x86_64                        randconfig-k001
>>
>> --
>> 0-DAY CI Kernel Test Service
>> https://01.org/lkp
>>
>>
>> ------------------------------
>>
>> Message: 4
>> Date: Tue, 10 Jan 2023 07:48:41 +0200
>> From: Sasha Neftin <sasha.neftin@intel.com>
>> To: intel-wired-lan@lists.osuosl.org
>> Subject: [Intel-wired-lan] [PATCH net v1 1/1] igc: Add ndo_tx_timeout
>> 	support
>> Message-ID: <20230110054841.1857688-1-sasha.neftin@intel.com>
>>
>> On some platforms, 100/1000/2500 speeds seem to have sometimes
>> problems
>> reporting false positive tx unit hang during stressful UDP traffic. Likely
>> other Intel drivers introduce responses to a tx hang. Update the 'tx hang'
>> comparator with the comparison of the head and tail of ring pointers and
>> restore the tx_timeout_factor to the previous value (one).
>>
>> This can be test by using netperf or iperf3 applications.
>> Example:
>> iperf3 -s -p 5001
>> iperf3 -c 192.168.0.2 --udp -p 5001 --time 600 -b 0
>>
>> netserver -p 16604
>> netperf -H 192.168.0.2 -l 600 -p 16604 -t UDP_STREAM -- -m 64000
>>
>> Fixes: b27b8dc77b5e ("igc: Increase timeout value for Speed 100/1000/2500")
>> Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
>> ---
>>   drivers/net/ethernet/intel/igc/igc_main.c | 25 +++++++++++++++++++++--
>>   1 file changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
>> b/drivers/net/ethernet/intel/igc/igc_main.c
>> index 13088b5bcf5b..b1031d5b32bc 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_main.c
>> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
>> @@ -2957,7 +2957,9 @@ static bool igc_clean_tx_irq(struct igc_q_vector
>> *q_vector, int napi_budget)
>>   		if (tx_buffer->next_to_watch &&
>>   		    time_after(jiffies, tx_buffer->time_stamp +
>>   		    (adapter->tx_timeout_factor * HZ)) &&
>> -		    !(rd32(IGC_STATUS) & IGC_STATUS_TXOFF)) {
>> +		    !(rd32(IGC_STATUS) & IGC_STATUS_TXOFF) &&
>> +		    (rd32(IGC_TDH(tx_ring->reg_idx)) !=
>> +		    readl(tx_ring->tail))) {
>>   			/* detected Tx unit hang */
>>   			netdev_err(tx_ring->netdev,
>>   				   "Detected Tx Unit Hang\n"
>> @@ -5083,6 +5085,24 @@ static int igc_change_mtu(struct net_device
>> *netdev, int new_mtu)
>>   	return 0;
>>   }
>>
>> +/**
>> + * igc_tx_timeout - Respond to a Tx Hang
>> + * @netdev: network interface device structure
>> + * @txqueue: queue number that timed out
>> + **/
>> +static void igc_tx_timeout(struct net_device *netdev,
>> +			   unsigned int __always_unused txqueue)
>> +{
>> +	struct igc_adapter *adapter = netdev_priv(netdev);
>> +	struct igc_hw *hw = &adapter->hw;
>> +
>> +	/* Do the reset outside of interrupt context */
>> +	adapter->tx_timeout_count++;
>> +	schedule_work(&adapter->reset_task);
>> +	wr32(IGC_EICS,
>> +	     (adapter->eims_enable_mask & ~adapter->eims_other));
>> +}
>> +
>>   /**
>>    * igc_get_stats64 - Get System Network Statistics
>>    * @netdev: network interface device structure
>> @@ -5510,7 +5530,7 @@ static void igc_watchdog_task(struct work_struct
>> *work)
>>   			case SPEED_100:
>>   			case SPEED_1000:
>>   			case SPEED_2500:
>> -				adapter->tx_timeout_factor = 7;
>> +				adapter->tx_timeout_factor = 1;
> 
> I was running more test with UDP packets with changing of Qbv Gates for TSN Mode.
> The tx timeout factor appears to need to stay at 7 in order to avoid the hanging issue
> for some extremely high traffic situation gate switching. Given that it had no impact
> on the first head and tail pointer problem as per what you resolved in igc_clean_tx_irq(),
> I advise keeping this number.
Hello Muhammad,
I have a concern. Increasing the tx_timeout_factor will delay the 
'.ndo_tx_timeout' report to the 'netdev' and lead to a stuck controller. 
I saw it on production boards (few CRB board with iperf3 tests).Let's 
leave it 1 for now. I would suggest understanding your TSN corner cases 
and directly discussing the solution with Dima and me(Intel).
Sasha
> 
>>   				break;
>>   			}
>>
>> @@ -6352,6 +6372,7 @@ static const struct net_device_ops igc_netdev_ops
>> = {
>>   	.ndo_set_rx_mode	= igc_set_rx_mode,
>>   	.ndo_set_mac_address	= igc_set_mac,
>>   	.ndo_change_mtu		= igc_change_mtu,
>> +	.ndo_tx_timeout		= igc_tx_timeout,
>>   	.ndo_get_stats64	= igc_get_stats64,
>>   	.ndo_fix_features	= igc_fix_features,
>>   	.ndo_set_features	= igc_set_features,
>> --
>> 2.25.1
>>
>>
>>
>> ------------------------------
>>
>> Subject: Digest Footer
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>>
>>
>> ------------------------------
>>
>> End of Intel-wired-lan Digest, Vol 405, Issue 6
>> ***********************************************
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan