[PATCH net-next 0/3][pull request] Intel Wired LAN Driver Updates 2023-01-25 (igc)

[PATCH net-next 0/3][pull request] Intel Wired LAN Driver Updates 2023-01-25 (igc)

[Tony Nguyen]

This series contains updates to igc driver only.

Muhammad adds tracking and reporting of QBV config errors.

Tan adds support for configuring max SDU for each Tx queue.

Sasha removes check for alternate media as only one media type is
supported.

The following are changes since commit f5be9caf7bf022ab550f62ea68f1c1bb8f5287ee:
  net: ethtool: fix NULL pointer dereference in pause_prepare_data()
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 1GbE

Muhammad Husaini Zulkifli (1):
  igc: Add qbv_config_change_errors counter

Sasha Neftin (1):
  igc: Clean up and optimize watchdog task

Tan Tee Min (1):
  igc: offload queue max SDU from tc-taprio

 drivers/net/ethernet/intel/igc/igc.h         |  4 +-
 drivers/net/ethernet/intel/igc/igc_ethtool.c |  1 +
 drivers/net/ethernet/intel/igc/igc_main.c    | 62 ++++++++++++++------
 drivers/net/ethernet/intel/igc/igc_tsn.c     |  6 ++
 4 files changed, 54 insertions(+), 19 deletions(-)

-- 
2.38.1

[PATCH net-next 1/3] igc: Add qbv_config_change_errors counter

[Tony Nguyen]

From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>

Add ConfigChangeError(qbv_config_change_errors) when user try to set the
AdminBaseTime to past value while the current GCL is still running.

The ConfigChangeError counter should not be increased when a gate control
list is scheduled into the future.

User can use "ethtool -S <interface> | grep qbv_config_change_errors"
command to check the counter values.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Tested-by: Naama Meir <naamax.meir@linux.intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igc/igc.h         | 1 +
 drivers/net/ethernet/intel/igc/igc_ethtool.c | 1 +
 drivers/net/ethernet/intel/igc/igc_main.c    | 1 +
 drivers/net/ethernet/intel/igc/igc_tsn.c     | 6 ++++++
 4 files changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index df3e26c0cf01..79cc99af4317 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -185,6 +185,7 @@ struct igc_adapter {
 	ktime_t base_time;
 	ktime_t cycle_time;
 	bool qbv_enable;
+	u32 qbv_config_change_errors;
 
 	/* OS defined structs */
 	struct pci_dev *pdev;
diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index 5a26a7805ef8..0e2cb00622d1 100644
--- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
+++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
@@ -67,6 +67,7 @@ static const struct igc_stats igc_gstrings_stats[] = {
 	IGC_STAT("rx_hwtstamp_cleared", rx_hwtstamp_cleared),
 	IGC_STAT("tx_lpi_counter", stats.tlpic),
 	IGC_STAT("rx_lpi_counter", stats.rlpic),
+	IGC_STAT("qbv_config_change_errors", qbv_config_change_errors),
 };
 
 #define IGC_NETDEV_STAT(_net_stat) { \
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index e86b15efaeb8..20bcf9c4e310 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -6029,6 +6029,7 @@ static int igc_tsn_clear_schedule(struct igc_adapter *adapter)
 
 	adapter->base_time = 0;
 	adapter->cycle_time = NSEC_PER_SEC;
+	adapter->qbv_config_change_errors = 0;
 
 	for (i = 0; i < adapter->num_tx_queues; i++) {
 		struct igc_ring *ring = adapter->tx_ring[i];
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
index a386c8d61dbf..b38c1c7569a0 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.c
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
@@ -239,6 +239,12 @@ static int igc_tsn_enable_offload(struct igc_adapter *adapter)
 		s64 n = div64_s64(ktime_sub_ns(systim, base_time), cycle);
 
 		base_time = ktime_add_ns(base_time, (n + 1) * cycle);
+
+		/* Increase the counter if scheduling into the past while
+		 * Gate Control List (GCL) is running.
+		 */
+		if (rd32(IGC_BASET_H) || rd32(IGC_BASET_L))
+			adapter->qbv_config_change_errors++;
 	} else {
 		/* According to datasheet section 7.5.2.9.3.3, FutScdDis bit
 		 * has to be configured before the cycle time and base time.
-- 
2.38.1

Re: [PATCH net-next 1/3] igc: Add qbv_config_change_errors counter

[Leon Romanovsky]

On Wed, Jan 25, 2023 at 01:27:00PM -0800, Tony Nguyen wrote:
> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> 
> Add ConfigChangeError(qbv_config_change_errors) when user try to set the
> AdminBaseTime to past value while the current GCL is still running.
> 
> The ConfigChangeError counter should not be increased when a gate control
> list is scheduled into the future.
> 
> User can use "ethtool -S <interface> | grep qbv_config_change_errors"
> command to check the counter values.
> 
> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> Tested-by: Naama Meir <naamax.meir@linux.intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  drivers/net/ethernet/intel/igc/igc.h         | 1 +
>  drivers/net/ethernet/intel/igc/igc_ethtool.c | 1 +
>  drivers/net/ethernet/intel/igc/igc_main.c    | 1 +
>  drivers/net/ethernet/intel/igc/igc_tsn.c     | 6 ++++++
>  4 files changed, 9 insertions(+)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

[PATCH net-next 2/3] igc: offload queue max SDU from tc-taprio

[Tony Nguyen]

From: Tan Tee Min <tee.min.tan@linux.intel.com>

Add support for configuring the max SDU for each Tx queue.
If not specified, keep the default.

All link speeds have been tested with this implementation.
No performance issue observed.

How to test:

1) Configure the tc with max-sdu

tc qdisc replace dev $IFACE parent root handle 100 taprio \
    num_tc 4 \
    map 0 1 2 3 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 S 0xF 1000000 \
    max-sdu 1500 1498 0 0 0 0 0 0 0 0 0 0 0 0 0 0 \
    flags 0x2 \
    txtime-delay 0

2) Use network statistic to watch the tx queue packet to see if
packet able to go out or drop.

Signed-off-by: Tan Tee Min <tee.min.tan@linux.intel.com>
Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Tested-by: Naama Meir <naamax.meir@linux.intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igc/igc.h      |  1 +
 drivers/net/ethernet/intel/igc/igc_main.c | 44 +++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index 79cc99af4317..c0c00b8bd8d8 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -99,6 +99,7 @@ struct igc_ring {
 
 	u32 start_time;
 	u32 end_time;
+	u32 max_sdu;
 
 	/* CBS parameters */
 	bool cbs_enable;                /* indicates if CBS is enabled */
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 20bcf9c4e310..811f842a20a4 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -1508,6 +1508,7 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb,
 	__le32 launch_time = 0;
 	u32 tx_flags = 0;
 	unsigned short f;
+	u32 max_sdu = 0;
 	ktime_t txtime;
 	u8 hdr_len = 0;
 	int tso = 0;
@@ -1527,6 +1528,14 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb,
 		return NETDEV_TX_BUSY;
 	}
 
+	if (tx_ring->max_sdu > 0) {
+		max_sdu = tx_ring->max_sdu +
+			  (skb_vlan_tagged(skb) ? VLAN_HLEN : 0);
+
+		if (skb->len > max_sdu)
+			goto skb_drop;
+	}
+
 	if (!tx_ring->launchtime_enable)
 		goto done;
 
@@ -1606,6 +1615,12 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb,
 	dev_kfree_skb_any(first->skb);
 	first->skb = NULL;
 
+	return NETDEV_TX_OK;
+
+skb_drop:
+	dev_kfree_skb_any(skb);
+	skb = NULL;
+
 	return NETDEV_TX_OK;
 }
 
@@ -6036,6 +6051,7 @@ static int igc_tsn_clear_schedule(struct igc_adapter *adapter)
 
 		ring->start_time = 0;
 		ring->end_time = NSEC_PER_SEC;
+		ring->max_sdu = 0;
 	}
 
 	return 0;
@@ -6119,6 +6135,16 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter,
 		}
 	}
 
+	for (i = 0; i < adapter->num_tx_queues; i++) {
+		struct igc_ring *ring = adapter->tx_ring[i];
+		struct net_device *dev = adapter->netdev;
+
+		if (qopt->max_sdu[i])
+			ring->max_sdu = qopt->max_sdu[i] + dev->hard_header_len;
+		else
+			ring->max_sdu = 0;
+	}
+
 	return 0;
 }
 
@@ -6206,12 +6232,30 @@ static int igc_tsn_enable_cbs(struct igc_adapter *adapter,
 	return igc_tsn_offload_apply(adapter);
 }
 
+static int igc_tsn_query_caps(struct tc_query_caps_base *base)
+{
+	switch (base->type) {
+	case TC_SETUP_QDISC_TAPRIO: {
+		struct tc_taprio_caps *caps = base->caps;
+
+		caps->supports_queue_max_sdu = true;
+
+		return 0;
+	}
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static int igc_setup_tc(struct net_device *dev, enum tc_setup_type type,
 			void *type_data)
 {
 	struct igc_adapter *adapter = netdev_priv(dev);
 
 	switch (type) {
+	case TC_QUERY_CAPS:
+		return igc_tsn_query_caps(type_data);
+
 	case TC_SETUP_QDISC_TAPRIO:
 		return igc_tsn_enable_qbv_scheduling(adapter, type_data);
 
-- 
2.38.1

Re: [PATCH net-next 2/3] igc: offload queue max SDU from tc-taprio

[Leon Romanovsky]

On Wed, Jan 25, 2023 at 01:27:01PM -0800, Tony Nguyen wrote:
> From: Tan Tee Min <tee.min.tan@linux.intel.com>
> 
> Add support for configuring the max SDU for each Tx queue.
> If not specified, keep the default.
> 
> All link speeds have been tested with this implementation.
> No performance issue observed.
> 
> How to test:
> 
> 1) Configure the tc with max-sdu
> 
> tc qdisc replace dev $IFACE parent root handle 100 taprio \
>     num_tc 4 \
>     map 0 1 2 3 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 S 0xF 1000000 \
>     max-sdu 1500 1498 0 0 0 0 0 0 0 0 0 0 0 0 0 0 \
>     flags 0x2 \
>     txtime-delay 0
> 
> 2) Use network statistic to watch the tx queue packet to see if
> packet able to go out or drop.
> 
> Signed-off-by: Tan Tee Min <tee.min.tan@linux.intel.com>
> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> Tested-by: Naama Meir <naamax.meir@linux.intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  drivers/net/ethernet/intel/igc/igc.h      |  1 +
>  drivers/net/ethernet/intel/igc/igc_main.c | 44 +++++++++++++++++++++++
>  2 files changed, 45 insertions(+)

<...>

> +skb_drop:
> +	dev_kfree_skb_any(skb);
> +	skb = NULL;

Why do you set skb to be NULL?

> +
>  	return NETDEV_TX_OK;
>  }
>  

Thanks

RE: [PATCH net-next 2/3] igc: offload queue max SDU from tc-taprio

[Zulkifli, Muhammad Husaini]

Hi Leon,

Sorry for the late reply. Replied inline.

> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Thursday, 26 January, 2023 6:51 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Cc: davem@davemloft.net; kuba@kernel.org; pabeni@redhat.com;
> edumazet@google.com; Tan Tee Min <tee.min.tan@linux.intel.com>;
> netdev@vger.kernel.org; Neftin, Sasha <sasha.neftin@intel.com>; Zulkifli,
> Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; Naama Meir
> <naamax.meir@linux.intel.com>
> Subject: Re: [PATCH net-next 2/3] igc: offload queue max SDU from tc-taprio
> 
> On Wed, Jan 25, 2023 at 01:27:01PM -0800, Tony Nguyen wrote:
> > From: Tan Tee Min <tee.min.tan@linux.intel.com>
> >
> > Add support for configuring the max SDU for each Tx queue.
> > If not specified, keep the default.
> >
> > All link speeds have been tested with this implementation.
> > No performance issue observed.
> >
> > How to test:
> >
> > 1) Configure the tc with max-sdu
> >
> > tc qdisc replace dev $IFACE parent root handle 100 taprio \
> >     num_tc 4 \
> >     map 0 1 2 3 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 S 0xF 1000000 \
> >     max-sdu 1500 1498 0 0 0 0 0 0 0 0 0 0 0 0 0 0 \
> >     flags 0x2 \
> >     txtime-delay 0
> >
> > 2) Use network statistic to watch the tx queue packet to see if packet
> > able to go out or drop.
> >
> > Signed-off-by: Tan Tee Min <tee.min.tan@linux.intel.com>
> > Signed-off-by: Muhammad Husaini Zulkifli
> > <muhammad.husaini.zulkifli@intel.com>
> > Tested-by: Naama Meir <naamax.meir@linux.intel.com>
> > Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> > ---
> >  drivers/net/ethernet/intel/igc/igc.h      |  1 +
> >  drivers/net/ethernet/intel/igc/igc_main.c | 44
> > +++++++++++++++++++++++
> >  2 files changed, 45 insertions(+)
> 
> <...>
> 
> > +skb_drop:
> > +	dev_kfree_skb_any(skb);
> > +	skb = NULL;
> 
> Why do you set skb to be NULL?

IMHO, dev_kfree_skb_any(skb) should be enough to free the buffer.
I will remove it on the next submission.

Thanks,
Husaini

> 
> > +
> >  	return NETDEV_TX_OK;
> >  }
> >
> 
> Thanks

[PATCH net-next 3/3] igc: Clean up and optimize watchdog task

[Tony Nguyen]

From: Sasha Neftin <sasha.neftin@intel.com>

i225/i226 parts used only one media type copper. The copper media type is
not replaceable. Clean up the code accordingly, and remove the obsolete
media replacement and reset options.

Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
Tested-by: Naama Meir <naamax.meir@linux.intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igc/igc.h      |  2 --
 drivers/net/ethernet/intel/igc/igc_main.c | 17 -----------------
 2 files changed, 19 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index c0c00b8bd8d8..34aebf00a512 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -294,8 +294,6 @@ extern char igc_driver_name[];
 #define IGC_FLAG_PTP			BIT(8)
 #define IGC_FLAG_WOL_SUPPORTED		BIT(8)
 #define IGC_FLAG_NEED_LINK_UPDATE	BIT(9)
-#define IGC_FLAG_MEDIA_RESET		BIT(10)
-#define IGC_FLAG_MAS_ENABLE		BIT(12)
 #define IGC_FLAG_HAS_MSIX		BIT(13)
 #define IGC_FLAG_EEE			BIT(14)
 #define IGC_FLAG_VLAN_PROMISC		BIT(15)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 811f842a20a4..13088b5bcf5b 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -5561,25 +5561,8 @@ static void igc_watchdog_task(struct work_struct *work)
 				mod_timer(&adapter->phy_info_timer,
 					  round_jiffies(jiffies + 2 * HZ));
 
-			/* link is down, time to check for alternate media */
-			if (adapter->flags & IGC_FLAG_MAS_ENABLE) {
-				if (adapter->flags & IGC_FLAG_MEDIA_RESET) {
-					schedule_work(&adapter->reset_task);
-					/* return immediately */
-					return;
-				}
-			}
 			pm_schedule_suspend(netdev->dev.parent,
 					    MSEC_PER_SEC * 5);
-
-		/* also check for alternate media here */
-		} else if (!netif_carrier_ok(netdev) &&
-			   (adapter->flags & IGC_FLAG_MAS_ENABLE)) {
-			if (adapter->flags & IGC_FLAG_MEDIA_RESET) {
-				schedule_work(&adapter->reset_task);
-				/* return immediately */
-				return;
-			}
 		}
 	}
 
-- 
2.38.1

Re: [PATCH net-next 3/3] igc: Clean up and optimize watchdog task

[Leon Romanovsky]

On Wed, Jan 25, 2023 at 01:27:02PM -0800, Tony Nguyen wrote:
> From: Sasha Neftin <sasha.neftin@intel.com>
> 
> i225/i226 parts used only one media type copper. The copper media type is
> not replaceable. Clean up the code accordingly, and remove the obsolete
> media replacement and reset options.
> 
> Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
> Tested-by: Naama Meir <naamax.meir@linux.intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  drivers/net/ethernet/intel/igc/igc.h      |  2 --
>  drivers/net/ethernet/intel/igc/igc_main.c | 17 -----------------
>  2 files changed, 19 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>