Skip to contentSkip to conversation

Configuring traffic classes via new hardware offload mechanism in tc/mqprio

PATCH In review Amritha Nambiar 8 participants 17 messages May 19, 2017 – Jul 26, 2017

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover Configuring traffic classes via new hardware offload mechanism in tc/mqprio

The following series introduces a new harware offload mode in tc/mqprio where the TCs, the queue configurations and bandwidth rate limits are offloaded to the hardware. The i40e driver enables the new mqprio hardware offload mechanism factoring the TCs, queue configuration and bandwidth rates by creating HW channel VSIs.

In this mode, the priority to traffic class mapping and the user specified queue ranges are used to configure the traffic class when the 'hw' option is set to 2. This is achieved by creating HW channels(VSI). A new channel is created for each of the traffic class configuration offloaded via mqprio framework except for the first TC (TC0) which is for the main VSI. TC0 for the main VSI is also reconfigured as per user provided queue parameters. Finally, bandwidth rate limits are set on these traffic classes through the mqprio offload framework by sending these rates in addition to the number of TCs and the queue configurations.

Example:

# tc qdisc add dev eth0 root mqprio num_tc 2  map 0 0 0 0 1 1 1 1\
  queues 4@0 4@4 min_rate 0Mbit 0Mbit max_rate 55Mbit 60Mbit hw 2

To dump the bandwidth rates:

# tc qdisc show dev eth0
  qdisc mqprio 804a: root  tc 2 map 0 0 0 0 1 1 1 1 0 0 0 0 0 0 0 0
               queues:(0:3) (4:7)
               min rates:0bit 0bit
               max rates:55Mbit 60Mbit

---

Amritha Nambiar (4):
[next-queue]net: mqprio: Introduce new hardware offload mode in mqprio for offloading full TC configurations [next-queue]net: i40e: Add infrastructure for queue channel support with the TCs and queue configurations offloaded via mqprio scheduler [next-queue]net: i40e: Enable mqprio full offload mode in the i40e driver for configuring TCs and queue mapping [next-queue]net: i40e: Add support to set max bandwidth rates for TCs offloaded via tc/mqprio

 drivers/net/ethernet/intel/i40e/i40e.h         |   42 +
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c |    6 
 drivers/net/ethernet/intel/i40e/i40e_main.c    | 1365 +++++++++++++++++++++---
 drivers/net/ethernet/intel/i40e/i40e_txrx.h    |    2 
 include/linux/netdevice.h                      |    2 
 include/net/pkt_cls.h                          |    7 
 include/uapi/linux/pkt_sched.h                 |   13 
 net/sched/sch_mqprio.c                         |  169 +++
 8 files changed, 1449 insertions(+), 157 deletions(-)
#1 lore raw

PATCH 1/4 net: mqprio: Introduce new hardware offload mode in mqprio for offloading full TC configurations

This patch introduces a new hardware offload mode in mqprio which makes full use of the mqprio options, the TCs, the queue configurations and the bandwidth rates for the TCs. This is achieved by setting the value 2 for the "hw" option. This new offload mode supports new attributes for traffic class such as minimum and maximum values for bandwidth rate limits.

Introduces a new datastructure 'tc_mqprio_qopt_offload' for offloading mqprio queue options and use this to be shared between the kernel and device driver. This contains a copy of the exisiting datastructure for mqprio queue options. This new datastructure can be extended when adding new attributes for traffic class such as bandwidth rate limits. The existing datastructure for mqprio queue options will be shared between the kernel and userspace.

This patch enables configuring additional attributes associated with a traffic class such as minimum and maximum bandwidth rates and can be offloaded to the hardware in the new offload mode. The min and max limits for bandwidth rates are provided by the user along with the the TCs and the queue configurations when creating the mqprio qdisc.

Example:

# tc qdisc add dev eth0 root mqprio num_tc 2  map 0 0 0 0 1 1 1 1\
  queues 4@0 4@4 min_rate 0Mbit 0Mbit max_rate 55Mbit 60Mbit hw 2

To dump the bandwidth rates:

# tc qdisc show dev eth0
qdisc mqprio 804a: root  tc 2 map 0 0 0 0 1 1 1 1 0 0 0 0 0 0 0 0
             queues:(0:3) (4:7)
             min rates:0bit 0bit
             max rates:55Mbit 60Mbit
---
 include/linux/netdevice.h      |    2 
 include/net/pkt_cls.h          |    7 ++
 include/uapi/linux/pkt_sched.h |   13 +++
 net/sched/sch_mqprio.c         |  169 +++++++++++++++++++++++++++++++++++++---
 4 files changed, 180 insertions(+), 11 deletions(-)
include/linux/netdevice.h +2 -0
index 0150b2d..17b9066 100644
@@ -779,6 +779,7 @@ enum {
 	TC_SETUP_CLSFLOWER,
 	TC_SETUP_MATCHALL,
 	TC_SETUP_CLSBPF,
+	TC_SETUP_MQPRIO_EXT,
 };
 
 struct tc_cls_u32_offload;
@@ -791,6 +792,7 @@ struct tc_to_netdev {
 		struct tc_cls_matchall_offload *cls_mall;
 		struct tc_cls_bpf_offload *cls_bpf;
 		struct tc_mqprio_qopt *mqprio;
+		struct tc_mqprio_qopt_offload *mqprio_qopt;
 	};
 	bool egress_dev;
 };
include/net/pkt_cls.h +7 -0
index 2c213a6..5ab8052 100644
@@ -549,6 +549,13 @@ struct tc_cls_bpf_offload {
 	u32 gen_flags;
 };
 
+struct tc_mqprio_qopt_offload {
+	/* struct tc_mqprio_qopt must always be the first element */
+	struct tc_mqprio_qopt qopt;
+	u32	flags;
+	u64	min_rate[TC_QOPT_MAX_QUEUE];
+	u64	max_rate[TC_QOPT_MAX_QUEUE];
+};
 
 /* This structure holds cookie structure that is passed from user
  * to the kernel for actions and classifiers
include/uapi/linux/pkt_sched.h +13 -0
index 099bf55..cf2a146 100644
@@ -620,6 +620,7 @@ struct tc_drr_stats {
 enum {
 	TC_MQPRIO_HW_OFFLOAD_NONE,	/* no offload requested */
 	TC_MQPRIO_HW_OFFLOAD_TCS,	/* offload TCs, no queue counts */
+	TC_MQPRIO_HW_OFFLOAD,		/* fully supported offload */
 	__TC_MQPRIO_HW_OFFLOAD_MAX
 };
 
@@ -633,6 +634,18 @@ struct tc_mqprio_qopt {
 	__u16	offset[TC_QOPT_MAX_QUEUE];
 };
 
+#define TC_MQPRIO_F_MIN_RATE  0x1
+#define TC_MQPRIO_F_MAX_RATE  0x2
+
+enum {
+	TCA_MQPRIO_UNSPEC,
+	TCA_MQPRIO_MIN_RATE64,
+	TCA_MQPRIO_MAX_RATE64,
+	__TCA_MQPRIO_MAX,
+};
+
+#define TCA_MQPRIO_MAX (__TCA_MQPRIO_MAX - 1)
+
 /* SFB */
 
 enum {
net/sched/sch_mqprio.c +158 -11

234 lines not on this page · raw message

#2 lore raw

Re: [Intel-wired-lan] [PATCH 1/4] [next-queue]net: mqprio: Introduce new hardware offload mode in mqprio for offloading full TC configurations

Quoted 42 lines · Amritha Nambiar

This patch introduces a new hardware offload mode in mqprio which makes full use of the mqprio options, the TCs, the queue configurations and the bandwidth rates for the TCs. This is achieved by setting the value 2 for the "hw" option. This new offload mode supports new attributes for traffic class such as minimum and maximum values for bandwidth rate limits.

Introduces a new datastructure 'tc_mqprio_qopt_offload' for offloading mqprio queue options and use this to be shared between the kernel and device driver. This contains a copy of the exisiting datastructure for mqprio queue options. This new datastructure can be extended when adding new attributes for traffic class such as bandwidth rate limits. The existing datastructure for mqprio queue options will be shared between the kernel and userspace.

This patch enables configuring additional attributes associated with a traffic class such as minimum and maximum bandwidth rates and can be offloaded to the hardware in the new offload mode. The min and max limits for bandwidth rates are provided by the user along with the the TCs and the queue configurations when creating the mqprio qdisc.

Example:

# tc qdisc add dev eth0 root mqprio num_tc 2  map 0 0 0 0 1 1 1 1\
  queues 4 at 0 4 at 4 min_rate 0Mbit 0Mbit max_rate 55Mbit 60Mbit hw 2

To dump the bandwidth rates:

# tc qdisc show dev eth0
qdisc mqprio 804a: root  tc 2 map 0 0 0 0 1 1 1 1 0 0 0 0 0 0 0 0
             queues:(0:3) (4:7)
             min rates:0bit 0bit
             max rates:55Mbit 60Mbit
---
 include/linux/netdevice.h      |    2
 include/net/pkt_cls.h          |    7 ++
 include/uapi/linux/pkt_sched.h |   13 +++
 net/sched/sch_mqprio.c         |  169 +++++++++++++++++++++++++++++++++++++---
 4 files changed, 180 insertions(+), 11 deletions(-)

[...]

Quoted 19 lines
net/sched/sch_mqprio.c +3 -0
index 0a4cf27..6457ec9 100644
@@ -18,10 +18,13 @@
 #include <net/netlink.h>
 #include <net/pkt_sched.h>
 #include <net/sch_generic.h>
+#include <net/pkt_cls.h>
 
 struct mqprio_sched {
        struct Qdisc            **qdiscs;
        int hw_offload;
+       u32 flags;
+       u64 min_rate[TC_QOPT_MAX_QUEUE], max_rate[TC_QOPT_MAX_QUEUE];
 };
 
 static void mqprio_destroy(struct Qdisc *sch)

net/sched/sch_mqprio.c static void mqprio_destroy(struct Qdisc *sch) · line 45

Quoted 7 lines
net/sched/sch_mqprio.c +1 -3
@@ -39,10 +42,21 @@ static void mqprio_destroy(struct Qdisc *sch)
        }
 
        if (priv->hw_offload && dev->netdev_ops->ndo_setup_tc) {
-               struct tc_mqprio_qopt offload = { 0 };
-               struct tc_to_netdev tc = { .type = TC_SETUP_MQPRIO,
-                                          { .mqprio = &offload } };
+               struct tc_mqprio_qopt_offload offload = { 0 };

So this is currently throwing a warning when I pull these patches and build this with gcc 6.2. I think the correct setup is "offload = {{ 0 }}" in order to indicate that we are initializing the inner structure and all other data to 0.

Quoted 17 lines
net/sched/sch_mqprio.c +13 -0
+               struct tc_to_netdev tc = { 0 };
 
+               switch (priv->hw_offload) {
+               case TC_MQPRIO_HW_OFFLOAD_TCS:
+                       tc.type = TC_SETUP_MQPRIO;
+                       tc.mqprio = &offload.qopt;
+                       break;
+               case TC_MQPRIO_HW_OFFLOAD:
+                       tc.type = TC_SETUP_MQPRIO_EXT;
+                       tc.mqprio_qopt = &offload;
+                       break;
+               default:
+                       return;
+               }
                dev->netdev_ops->ndo_setup_tc(dev, sch->handle, 0, &tc);
        } else {
                netdev_set_num_tc(dev, 0);
#3 lore raw

PATCH 2/4 net: i40e: Add infrastructure for queue channel support with the TCs and queue configurations offloaded via mqprio scheduler

In reply to Amritha Nambiar

This patch sets up the infrastructure for offloading TCs and queue configurations to the hardware by creating HW channels(VSI). A new channel is created for each of the traffic class configuration offloaded via mqprio framework except for the first TC (TC0). TC0 for the main VSI is also reconfigured as per user provided queue parameters. Queue counts that are not power-of-2 are handled by reconfiguring RSS by reprogramming LUTs using the queue count value. This patch also handles configuring the TX rings for the channels, setting up the RX queue map for channel.

Also, the channels so created are removed and all the queue configuration is set to default when the qdisc is detached from the root of the device.

---
 drivers/net/ethernet/intel/i40e/i40e.h      |   36 +
 drivers/net/ethernet/intel/i40e/i40e_main.c |  740 +++++++++++++++++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_txrx.h |    2 
 3 files changed, 771 insertions(+), 7 deletions(-)
drivers/net/ethernet/intel/i40e/i40e.h +36 -0
index 395ca94..0915b02 100644
@@ -330,6 +330,24 @@ struct i40e_flex_pit {
 	u8 pit_index;
 };
 
+struct i40e_channel {
+	struct list_head list;
+	bool initialized;
+	u8 type;
+	u16 vsi_number;
+	u16 stat_counter_idx;
+	u16 base_queue;
+	u16 num_queue_pairs; /* Requested by user */
+	u16 allowed_queue_pairs;
+	u16 seid;
+
+	u8 enabled_tc;
+	struct i40e_aqc_vsi_properties_data info;
+
+	/* track this channel belongs to which VSI */
+	struct i40e_vsi *parent_vsi;
+};
+
 /* struct that defines the Ethernet device */
 struct i40e_pf {
 	struct pci_dev *pdev;
@@ -442,6 +460,7 @@ struct i40e_pf {
 #define I40E_FLAG_CLIENT_L2_CHANGE		BIT_ULL(56)
 #define I40E_FLAG_WOL_MC_MAGIC_PKT_WAKE		BIT_ULL(57)
 #define I40E_FLAG_LEGACY_RX			BIT_ULL(58)
+#define I40E_FLAG_TC_MQPRIO			BIT_ULL(59)
 
 	struct i40e_client_instance *cinst;
 	bool stat_offsets_loaded;
@@ -523,6 +542,9 @@ struct i40e_pf {
 	u32 ioremap_len;
 	u32 fd_inv;
 	u16 phy_led_val;
+
+#define I40E_MAX_QUEUES_PER_CH	64
+	u16 override_q_count;
 };
 
 /**
@@ -684,6 +706,16 @@ struct i40e_vsi {
 	bool current_isup;	/* Sync 'link up' logging */
 	enum i40e_aq_link_speed current_speed;	/* Sync link speed logging */
 
+	/* channel specific fields */
+	u16 cnt_q_avail; /* num of queues available for channel usage */
+	u16 orig_rss_size;
+	u16 current_rss_size;
+
+	/* keeps track of next_base_queue to be used for channel setup */
+	atomic_t next_base_queue;
+
+	struct list_head ch_list;
+
 	void *priv;	/* client driver data reference. */
 
 	/* VSI specific handlers */
@@ -716,6 +748,9 @@ struct i40e_q_vector {
 	bool arm_wb_state;
 #define ITR_COUNTDOWN_START 100
 	u8 itr_countdown;	/* when 0 should adjust ITR */
+
+	/* Following field(s) are specific to channel usage */
+	bool is_an_atq;
 } ____cacheline_internodealigned_in_smp;
 
 /* lan device */
@@ -972,4 +1007,5 @@ i40e_status i40e_get_npar_bw_setting(struct i40e_pf *pf);
 i40e_status i40e_set_npar_bw_setting(struct i40e_pf *pf);
 i40e_status i40e_commit_npar_bw_setting(struct i40e_pf *pf);
 void i40e_print_link_message(struct i40e_vsi *vsi, bool isup);
+int i40e_create_queue_channel(struct i40e_vsi *vsi, struct i40e_channel *ch);
 #endif /* _I40E_H_ */
drivers/net/ethernet/intel/i40e/i40e_main.c +733 -7

775 lines not on this page · raw message

drivers/net/ethernet/intel/i40e/i40e_txrx.h +2 -0
index f5de511..02e1e84 100644
@@ -420,6 +420,8 @@ struct i40e_ring {
 					 * i40e_clean_rx_ring_irq() is called
 					 * for this ring.
 					 */
+
+	struct i40e_channel *ch;
 } ____cacheline_internodealigned_in_smp;
 
 static inline bool ring_uses_build_skb(struct i40e_ring *ring)
#4 lore raw

Re: [Intel-wired-lan] [PATCH 2/4] [next-queue]net: i40e: Add infrastructure for queue channel support with the TCs and queue configurations offloaded via mqprio scheduler

Quoted 24 lines · Amritha Nambiar

This patch sets up the infrastructure for offloading TCs and queue configurations to the hardware by creating HW channels(VSI). A new channel is created for each of the traffic class configuration offloaded via mqprio framework except for the first TC (TC0). TC0 for the main VSI is also reconfigured as per user provided queue parameters. Queue counts that are not power-of-2 are handled by reconfiguring RSS by reprogramming LUTs using the queue count value. This patch also handles configuring the TX rings for the channels, setting up the RX queue map for channel.

Also, the channels so created are removed and all the queue configuration is set to default when the qdisc is detached from the root of the device.

---
 drivers/net/ethernet/intel/i40e/i40e.h      |   36 +
 drivers/net/ethernet/intel/i40e/i40e_main.c |  740 +++++++++++++++++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_txrx.h |    2
 3 files changed, 771 insertions(+), 7 deletions(-)
drivers/net/ethernet/intel/i40e/i40e.h +0 -0
index 395ca94..0915b02 100644

[...]

Quoted 4 lines
drivers/net/ethernet/intel/i40e/i40e_main.c +0 -0
index 8d1d3b85..e1bea45 100644

[...]

Quoted 53 lines
drivers/net/ethernet/intel/i40e/i40e_main.c +53 -0
+/**
+ * i40e_create_queue_channel - function to create channel
+ * @vsi: VSI to be configured
+ * @ch: ptr to channel (it contains channel specific params)
+ *
+ * This function creates channel (VSI) using num_queues specified by user,
+ * reconfigs RSS if needed.
+ **/
+int i40e_create_queue_channel(struct i40e_vsi *vsi,
+                             struct i40e_channel *ch)
+{
+       struct i40e_pf *pf = vsi->back;
+       bool reconfig_rss;
+       int err;
+
+       if (!ch)
+               return -EINVAL;
+
+       if (!ch->num_queue_pairs) {
+               dev_err(&pf->pdev->dev, "Invalid num_queues requested: %d\n",
+                       ch->num_queue_pairs);
+               return -EINVAL;
+       }
+
+       /* validate user requested num_queues for channel */
+       err = i40e_validate_num_queues(pf, ch->num_queue_pairs, vsi,
+                                      &reconfig_rss);
+       if (err) {
+               dev_info(&pf->pdev->dev, "Failed to validate num_queues (%d)\n",
+                        ch->num_queue_pairs);
+               return -EINVAL;
+       }
+
+       /* By default we are in VEPA mode, if this is the first VF/VMDq
+        * VSI to be added switch to VEB mode.
+        */
+       if ((!(pf->flags & I40E_FLAG_VEB_MODE_ENABLED)) ||
+           (!i40e_is_any_channel(vsi))) {
+               if (!is_power_of_2(vsi->tc_config.tc_info[0].qcount)) {
+                       dev_info(&pf->pdev->dev,
+                                "Failed to create channel. Override queues (%u) not power of 2\n",
+                                vsi->tc_config.tc_info[0].qcount);
+                       return -EINVAL;
+               }
+
+               if (vsi->type == I40E_VSI_SRIOV) {
+                       if (!(pf->flags & I40E_FLAG_VEB_MODE_ENABLED)) {
+                               dev_info(&pf->pdev->dev,
+                                        "Expected to be VEB mode by this time\n");
+                               return -EINVAL;
+                       }
+               }
+               if (!(pf->flags & I40E_FLAG_VEB_MODE_ENABLED)) {

drivers/net/ethernet/intel/i40e/i40e_main.c static int i40e_vsi_config_tc(struct i40e_vsi *vsi, u8 enabled_tc) · line 5683

Quoted 10 lines
drivers/net/ethernet/intel/i40e/i40e_main.c +10 -0
+                       pf->flags |= I40E_FLAG_VEB_MODE_ENABLED;
+
+                       if (vsi->type == I40E_VSI_MAIN) {
+                               if (pf->flags & I40E_FLAG_TC_MQPRIO)
+                                       i40e_do_reset(pf,
+                                       BIT_ULL(__I40E_PF_RESET_REQUESTED),
+                                                     true);
+                               else
+                                       i40e_do_reset_safe(pf,
+                                       BIT_ULL(__I40E_PF_RESET_REQUESTED));

So these BIT_ULL lines are triggering a check in checkpatch, and I have to say I don't really like this as it really is messed up in terms of formatting.

If nothing else you might want to look at defining a macro that replaces the line. That way you could still represent the same data without having to resort to misaligning things to make it under 80 characters.

#5 lore raw

PATCH 2/4 net: i40e: Add infrastructure for queue channel support with the TCs and queue configurations offloaded via mqprio scheduler

Quoted 104 lines · Alexander Duyck
Quoted 24 lines · Amritha Nambiar

This patch sets up the infrastructure for offloading TCs and queue configurations to the hardware by creating HW channels(VSI). A new channel is created for each of the traffic class configuration offloaded via mqprio framework except for the first TC (TC0). TC0 for the main VSI is also reconfigured as per user provided queue parameters. Queue counts that are not power-of-2 are handled by reconfiguring RSS by reprogramming LUTs using the queue count value. This patch also handles configuring the TX rings for the channels, setting up the RX queue map for channel.

Also, the channels so created are removed and all the queue configuration is set to default when the qdisc is detached from the root of the device.

---
  drivers/net/ethernet/intel/i40e/i40e.h      |   36 +
  drivers/net/ethernet/intel/i40e/i40e_main.c |  740 +++++++++++++++++++++++++++
  drivers/net/ethernet/intel/i40e/i40e_txrx.h |    2
  3 files changed, 771 insertions(+), 7 deletions(-)
drivers/net/ethernet/intel/i40e/i40e.h +0 -0
index 395ca94..0915b02 100644

[...]

Quoted 4 lines
drivers/net/ethernet/intel/i40e/i40e_main.c +0 -0
index 8d1d3b85..e1bea45 100644

[...]

Quoted 53 lines
drivers/net/ethernet/intel/i40e/i40e_main.c +53 -0
+/**
+ * i40e_create_queue_channel - function to create channel
+ * @vsi: VSI to be configured
+ * @ch: ptr to channel (it contains channel specific params)
+ *
+ * This function creates channel (VSI) using num_queues specified by user,
+ * reconfigs RSS if needed.
+ **/
+int i40e_create_queue_channel(struct i40e_vsi *vsi,
+                             struct i40e_channel *ch)
+{
+       struct i40e_pf *pf = vsi->back;
+       bool reconfig_rss;
+       int err;
+
+       if (!ch)
+               return -EINVAL;
+
+       if (!ch->num_queue_pairs) {
+               dev_err(&pf->pdev->dev, "Invalid num_queues requested: %d\n",
+                       ch->num_queue_pairs);
+               return -EINVAL;
+       }
+
+       /* validate user requested num_queues for channel */
+       err = i40e_validate_num_queues(pf, ch->num_queue_pairs, vsi,
+                                      &reconfig_rss);
+       if (err) {
+               dev_info(&pf->pdev->dev, "Failed to validate num_queues (%d)\n",
+                        ch->num_queue_pairs);
+               return -EINVAL;
+       }
+
+       /* By default we are in VEPA mode, if this is the first VF/VMDq
+        * VSI to be added switch to VEB mode.
+        */
+       if ((!(pf->flags & I40E_FLAG_VEB_MODE_ENABLED)) ||
+           (!i40e_is_any_channel(vsi))) {
+               if (!is_power_of_2(vsi->tc_config.tc_info[0].qcount)) {
+                       dev_info(&pf->pdev->dev,
+                                "Failed to create channel. Override queues (%u) not power of 2\n",
+                                vsi->tc_config.tc_info[0].qcount);
+                       return -EINVAL;
+               }
+
+               if (vsi->type == I40E_VSI_SRIOV) {
+                       if (!(pf->flags & I40E_FLAG_VEB_MODE_ENABLED)) {
+                               dev_info(&pf->pdev->dev,
+                                        "Expected to be VEB mode by this time\n");
+                               return -EINVAL;
+                       }
+               }
+               if (!(pf->flags & I40E_FLAG_VEB_MODE_ENABLED)) {

drivers/net/ethernet/intel/i40e/i40e_main.c static int i40e_vsi_config_tc(struct i40e_vsi *vsi, u8 enabled_tc) · line 5683

Quoted 10 lines
drivers/net/ethernet/intel/i40e/i40e_main.c +10 -0
+                       pf->flags |= I40E_FLAG_VEB_MODE_ENABLED;
+
+                       if (vsi->type == I40E_VSI_MAIN) {
+                               if (pf->flags & I40E_FLAG_TC_MQPRIO)
+                                       i40e_do_reset(pf,
+                                       BIT_ULL(__I40E_PF_RESET_REQUESTED),
+                                                     true);
+                               else
+                                       i40e_do_reset_safe(pf,
+                                       BIT_ULL(__I40E_PF_RESET_REQUESTED));

So these BIT_ULL lines are triggering a check in checkpatch, and I have to say I don't really like this as it really is messed up in terms of formatting.

Quoted 4 lines

If nothing else you might want to look at defining a macro that replaces the line. That way you could still represent the same data without having to resort to misaligning things to make it under 80 characters.

Agree, we will get it fixed thru' macro

Quoted 5 lines

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

#6 lore raw

PATCH 3/4 net: i40e: Enable mqprio full offload mode in the i40e driver for configuring TCs and queue mapping

In reply to Amritha Nambiar

The i40e driver is modified to enable the new mqprio hardware offload mode and factor the TCs and queue configuration by creating channel VSIs. In this mode, the priority to traffic class mapping and the user specified queue ranges are used to configure the traffic classes when the 'hw' option is set to 2.

Example:

# tc qdisc add dev eth0 root mqprio num_tc 4\
  map 0 0 0 0 1 2 2 3 queues 2@0 2@2 1@4 1@5 hw 2
# tc qdisc show dev eth0
qdisc mqprio 8038: root  tc 4 map 0 0 0 0 1 2 2 3 0 0 0 0 0 0 0 0
             queues:(0:1) (2:3) (4:4) (5:5)

The HW channels created are removed and all the queue configuration is set to default when the qdisc is detached from the root of the device.

#tc qdisc del dev eth0 root

This patch also disables setting up channels via ethtool (ethtool -L) when the TCs are confgured using mqprio scheduler.

---
 drivers/net/ethernet/intel/i40e/i40e.h         |    4 
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c |    6 
 drivers/net/ethernet/intel/i40e/i40e_main.c    |  311 ++++++++++++++++++++++--
 3 files changed, 292 insertions(+), 29 deletions(-)
drivers/net/ethernet/intel/i40e/i40e.h +4 -0
index 0915b02..a62f65a 100644
@@ -54,6 +54,8 @@
 #include <linux/clocksource.h>
 #include <linux/net_tstamp.h>
 #include <linux/ptp_clock_kernel.h>
+#include <net/pkt_cls.h>
+
 #include "i40e_type.h"
 #include "i40e_prototype.h"
 #include "i40e_client.h"
@@ -685,6 +687,7 @@ struct i40e_vsi {
 	enum i40e_vsi_type type;  /* VSI type, e.g., LAN, FCoE, etc */
 	s16 vf_id;		/* Virtual function ID for SRIOV VSIs */
 
+	struct tc_mqprio_qopt_offload mqprio_qopt; /* queue parameters */
 	struct i40e_tc_configuration tc_config;
 	struct i40e_aqc_vsi_properties_data info;
 
@@ -710,6 +713,7 @@ struct i40e_vsi {
 	u16 cnt_q_avail; /* num of queues available for channel usage */
 	u16 orig_rss_size;
 	u16 current_rss_size;
+	bool reconfig_rss;
 
 	/* keeps track of next_base_queue to be used for channel setup */
 	atomic_t next_base_queue;
drivers/net/ethernet/intel/i40e/i40e_ethtool.c +6 -0
index 3d58762..ab52979 100644
@@ -3841,6 +3841,12 @@ static int i40e_set_channels(struct net_device *dev,
 	if (vsi->type != I40E_VSI_MAIN)
 		return -EINVAL;
 
+	/* We do not support setting channels via ethtool when TCs are
+	 * configured through mqprio
+	 */
+	if (pf->flags & I40E_FLAG_TC_MQPRIO)
+		return -EINVAL;
+
 	/* verify they are not requesting separate vectors */
 	if (!count || ch->rx_count || ch->tx_count)
 		return -EINVAL;
drivers/net/ethernet/intel/i40e/i40e_main.c +282 -29

428 lines not on this page · raw message

#7 lore raw

Re: [Intel-wired-lan] [PATCH 3/4] [next-queue]net: i40e: Enable mqprio full offload mode in the i40e driver for configuring TCs and queue mapping

Quoted 339 lines · Amritha Nambiar

The i40e driver is modified to enable the new mqprio hardware offload mode and factor the TCs and queue configuration by creating channel VSIs. In this mode, the priority to traffic class mapping and the user specified queue ranges are used to configure the traffic classes when the 'hw' option is set to 2.

Example:

# tc qdisc add dev eth0 root mqprio num_tc 4\
  map 0 0 0 0 1 2 2 3 queues 2 at 0 2 at 2 1 at 4 1 at 5 hw 2
# tc qdisc show dev eth0
qdisc mqprio 8038: root  tc 4 map 0 0 0 0 1 2 2 3 0 0 0 0 0 0 0 0
             queues:(0:1) (2:3) (4:4) (5:5)

The HW channels created are removed and all the queue configuration is set to default when the qdisc is detached from the root of the device.

#tc qdisc del dev eth0 root

This patch also disables setting up channels via ethtool (ethtool -L) when the TCs are confgured using mqprio scheduler.

---
 drivers/net/ethernet/intel/i40e/i40e.h         |    4
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c |    6
 drivers/net/ethernet/intel/i40e/i40e_main.c    |  311 ++++++++++++++++++++++--
 3 files changed, 292 insertions(+), 29 deletions(-)
drivers/net/ethernet/intel/i40e/i40e.h +4 -0
index 0915b02..a62f65a 100644
@@ -54,6 +54,8 @@
 #include <linux/clocksource.h>
 #include <linux/net_tstamp.h>
 #include <linux/ptp_clock_kernel.h>
+#include <net/pkt_cls.h>
+
 #include "i40e_type.h"
 #include "i40e_prototype.h"
 #include "i40e_client.h"
@@ -685,6 +687,7 @@ struct i40e_vsi {
        enum i40e_vsi_type type;  /* VSI type, e.g., LAN, FCoE, etc */
        s16 vf_id;              /* Virtual function ID for SRIOV VSIs */
 
+       struct tc_mqprio_qopt_offload mqprio_qopt; /* queue parameters */
        struct i40e_tc_configuration tc_config;
        struct i40e_aqc_vsi_properties_data info;
 
@@ -710,6 +713,7 @@ struct i40e_vsi {
        u16 cnt_q_avail; /* num of queues available for channel usage */
        u16 orig_rss_size;
        u16 current_rss_size;
+       bool reconfig_rss;
 
        /* keeps track of next_base_queue to be used for channel setup */
        atomic_t next_base_queue;
drivers/net/ethernet/intel/i40e/i40e_ethtool.c +6 -0
index 3d58762..ab52979 100644
@@ -3841,6 +3841,12 @@ static int i40e_set_channels(struct net_device *dev,
        if (vsi->type != I40E_VSI_MAIN)
                return -EINVAL;
 
+       /* We do not support setting channels via ethtool when TCs are
+        * configured through mqprio
+        */
+       if (pf->flags & I40E_FLAG_TC_MQPRIO)
+               return -EINVAL;
+
        /* verify they are not requesting separate vectors */
        if (!count || ch->rx_count || ch->tx_count)
                return -EINVAL;
drivers/net/ethernet/intel/i40e/i40e_main.c +167 -6
index e1bea45..7f61d4f 100644
@@ -68,6 +68,7 @@ static int i40e_reset(struct i40e_pf *pf);
 static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired);
 static void i40e_fdir_sb_setup(struct i40e_pf *pf);
 static int i40e_veb_get_bw_info(struct i40e_veb *veb);
+static int i40e_vsi_config_rss(struct i40e_vsi *vsi);
 
 /* i40e_pci_tbl - PCI Device ID Table
  *
@@ -1560,6 +1561,105 @@ static int i40e_set_mac(struct net_device *netdev, void *p)
 }
 
 /**
+ * i40e_vsi_setup_queue_map_mqprio - Prepares VSI tc_config to have queue
+ * configurations based on MQPRIO options.
+ * @vsi: the VSI being configured,
+ * @ctxt: VSI context structure
+ * @enabled_tc: number of traffic classes to enable
+ **/
+static int i40e_vsi_setup_queue_map_mqprio(struct i40e_vsi *vsi,
+                                          struct i40e_vsi_context *ctxt,
+                                          u8 enabled_tc)
+{
+       u8 netdev_tc = 0, offset = 0;
+       u16 qcount = 0, max_qcount, qmap, sections = 0;
+       int i, override_q, pow, num_qps, ret;
+
+       if (vsi->type != I40E_VSI_MAIN)
+               return -EINVAL;
+
+       sections = I40E_AQ_VSI_PROP_QUEUE_MAP_VALID;
+       sections |= I40E_AQ_VSI_PROP_SCHED_VALID;
+
+       vsi->tc_config.numtc = vsi->mqprio_qopt.qopt.num_tc;
+       vsi->tc_config.enabled_tc = enabled_tc ? enabled_tc : 1;
+
+       num_qps = vsi->mqprio_qopt.qopt.count[0];
+
+       /* find the next higher power-of-2 of num queue pairs */
+       pow = ilog2(num_qps);
+       if (!is_power_of_2(num_qps))
+               pow++;
+
+       qmap = (offset << I40E_AQ_VSI_TC_QUE_OFFSET_SHIFT) |
+               (pow << I40E_AQ_VSI_TC_QUE_NUMBER_SHIFT);
+
+       /* Setup queue offset/count for all TCs for given VSI */
+       max_qcount = vsi->mqprio_qopt.qopt.count[0];
+
+       for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++) {
+               /* See if the given TC is enabled for the given VSI */
+               if (vsi->tc_config.enabled_tc & BIT(i)) {
+                       offset = vsi->mqprio_qopt.qopt.offset[i];
+                       qcount = vsi->mqprio_qopt.qopt.count[i];
+
+                       if (qcount > max_qcount)
+                               max_qcount = qcount;
+
+                       vsi->tc_config.tc_info[i].qoffset = offset;
+                       vsi->tc_config.tc_info[i].qcount = qcount;
+                       vsi->tc_config.tc_info[i].netdev_tc = netdev_tc++;
+
+               } else {
+                       /* TC is not enabled so set the offset to
+                        * default queue and allocate one queue
+                        * for the given TC.
+                        */
+                       vsi->tc_config.tc_info[i].qoffset = 0;
+                       vsi->tc_config.tc_info[i].qcount = 1;
+                       vsi->tc_config.tc_info[i].netdev_tc = 0;
+               }
+       }
+
+       /* Set actual Tx/Rx queue pairs */
+       vsi->num_queue_pairs = offset + qcount;
+
+       /* Setup queue TC[0].qmap for given VSI context */
+       ctxt->info.tc_mapping[0] = cpu_to_le16(qmap);
+
+       ctxt->info.mapping_flags |=
+                                       cpu_to_le16(I40E_AQ_VSI_QUE_MAP_CONTIG);
+
+       ctxt->info.queue_mapping[0] = cpu_to_le16(vsi->base_queue);
+       ctxt->info.valid_sections |= cpu_to_le16(sections);
+
+       /* Reconfigure RSS for main VSI with max queue count */
+       vsi->rss_size = max_qcount;
+
+       ret = i40e_vsi_config_rss(vsi);
+       if (ret) {
+               dev_info(&vsi->back->pdev->dev,
+                        "Failed to reconfig rss for num_queues (%u)\n",
+                        max_qcount);
+               return ret;
+       }
+       vsi->reconfig_rss = true;
+       dev_dbg(&vsi->back->pdev->dev,
+               "Reconfigured rss with num_queues (%u)\n", max_qcount);
+
+       /* Find queue count available for channel VSIs and starting offset
+        * for channel VSIs
+        */
+       override_q = vsi->mqprio_qopt.qopt.count[0];
+       if (override_q && (override_q < vsi->num_queue_pairs)) {
+               vsi->cnt_q_avail = vsi->num_queue_pairs - override_q;
+               atomic_set(&vsi->next_base_queue, override_q);
+       }
+
+       return 0;
+}
+
+/**
  * i40e_vsi_setup_queue_map - Setup a VSI queue map based on enabled_tc
  * @vsi: the VSI being setup
  * @ctxt: VSI context structure
@@ -1597,7 +1697,7 @@ static void i40e_vsi_setup_queue_map(struct i40e_vsi *vsi,
                        numtc = 1;
                }
        } else {
-               /* At least TC0 is enabled in case of non-DCB case */
+               /* At least TC0 is enabled in non-DCB, non-MQPRIO case */
                numtc = 1;
        }
 
@@ -3150,6 +3250,7 @@ static void i40e_vsi_config_dcb_rings(struct i40e_vsi *vsi)
                        rx_ring->dcb_tc = 0;
                        tx_ring->dcb_tc = 0;
                }
+               return;
        }
 
        for (n = 0; n < I40E_MAX_TRAFFIC_CLASS; n++) {
@@ -4777,6 +4878,25 @@ static u8 i40e_dcb_get_enabled_tc(struct i40e_dcbx_config *dcbcfg)
 }
 
 /**
+ * i40e_mqprio_get_enabled_tc - Get enabled traffic classes
+ * @pf: PF being queried
+ *
+ * Query the current MQPRIO configuration and return the number of
+ * traffic classes enabled.
+ **/
+static u8 i40e_mqprio_get_enabled_tc(struct i40e_pf *pf)
+{
+       struct i40e_vsi *vsi = pf->vsi[pf->lan_vsi];
+       u8 num_tc = vsi->mqprio_qopt.qopt.num_tc;
+       u8 enabled_tc = 1, i;
+
+       for (i = 1; i < num_tc; i++)
+               enabled_tc |= BIT(i);
+
+       return enabled_tc;
+}
+
+/**
  * i40e_pf_get_num_tc - Get enabled traffic classes for PF
  * @pf: PF being queried
  *
@@ -4789,7 +4909,10 @@ static u8 i40e_pf_get_num_tc(struct i40e_pf *pf)
        u8 num_tc = 0;
        struct i40e_dcbx_config *dcbcfg = &hw->local_dcbx_config;
 
-       /* If DCB is not enabled then always in single TC */
+       if (pf->flags & I40E_FLAG_TC_MQPRIO)
+               return pf->vsi[pf->lan_vsi]->mqprio_qopt.qopt.num_tc;
+
+       /* If neither MQPRIO nor DCB is enabled, then always in single TC */
        if (!(pf->flags & I40E_FLAG_DCB_ENABLED))
                return 1;
 
@@ -4818,7 +4941,12 @@ static u8 i40e_pf_get_num_tc(struct i40e_pf *pf)
  **/
 static u8 i40e_pf_get_tc_map(struct i40e_pf *pf)
 {
-       /* If DCB is not enabled for this PF then just return default TC */
+       if (pf->flags & I40E_FLAG_TC_MQPRIO)
+               return i40e_mqprio_get_enabled_tc(pf);
+
+       /* If neither MQPRIO nor DCB is enabled for this PF then just return
+        * default TC
+        */
        if (!(pf->flags & I40E_FLAG_DCB_ENABLED))
                return I40E_DEFAULT_TRAFFIC_CLASS;
 
@@ -4912,6 +5040,10 @@ static int i40e_vsi_configure_bw_alloc(struct i40e_vsi *vsi, u8 enabled_tc,
        for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++)
                bw_data.tc_bw_credits[i] = bw_share[i];
 
+       if ((vsi->back->flags & I40E_FLAG_TC_MQPRIO) ||
+           !vsi->mqprio_qopt.qopt.hw)
+               return 0;
+
        ret = i40e_aq_config_vsi_tc_bw(&vsi->back->hw, vsi->seid, &bw_data,
                                       NULL);
        if (ret) {
@@ -4970,6 +5102,9 @@ static void i40e_vsi_config_netdev_tc(struct i40e_vsi *vsi, u8 enabled_tc)
                                        vsi->tc_config.tc_info[i].qoffset);
        }
 
+       if (pf->flags & I40E_FLAG_TC_MQPRIO)
+               return;
+
        /* Assign UP2TC map for the VSI */
        for (i = 0; i < I40E_MAX_USER_PRIORITY; i++) {
                /* Get the actual TC# for the UP */
@@ -5020,7 +5155,8 @@ static int i40e_vsi_config_tc(struct i40e_vsi *vsi, u8 enabled_tc)
        int i;
 
        /* Check if enabled_tc is same as existing or new TCs */
-       if (vsi->tc_config.enabled_tc == enabled_tc)
+       if (vsi->tc_config.enabled_tc == enabled_tc &&
+           vsi->mqprio_qopt.qopt.hw != TC_MQPRIO_HW_OFFLOAD)
                return ret;
 
        /* Enable ETS TCs with equal BW Share for now across all VSIs */
@@ -5043,7 +5179,30 @@ static int i40e_vsi_config_tc(struct i40e_vsi *vsi, u8 enabled_tc)
        ctxt.vf_num = 0;
        ctxt.uplink_seid = vsi->uplink_seid;
        ctxt.info = vsi->info;
-       i40e_vsi_setup_queue_map(vsi, &ctxt, enabled_tc, false);
+
+       if (vsi->back->flags & I40E_FLAG_TC_MQPRIO) {
+               ret = i40e_vsi_setup_queue_map_mqprio(vsi, &ctxt, enabled_tc);
+               if (ret)
+                       goto out;
+
+       } else {
+               i40e_vsi_setup_queue_map(vsi, &ctxt, enabled_tc, false);
+       }
+
+       /* On destroying the qdisc, reset vsi->rss_size, as number of enabled
+        * queues changed.
+        */
+       if (!vsi->mqprio_qopt.qopt.hw && vsi->reconfig_rss) {
+               vsi->rss_size = min_t(int, vsi->back->alloc_rss_size,
+                                     vsi->num_queue_pairs);
+               ret = i40e_vsi_config_rss(vsi);
+               if (ret) {
+                       dev_info(&vsi->back->pdev->dev,
+                                "Failed to reconfig rss for num_queues\n");
+                       return ret;
+               }
+               vsi->reconfig_rss = false;
+       }
 
        if (vsi->back->flags & I40E_FLAG_IWARP_ENABLED) {
                ctxt.info.valid_sections |=
@@ -5051,7 +5210,9 @@ static int i40e_vsi_config_tc(struct i40e_vsi *vsi, u8 enabled_tc)
                ctxt.info.queueing_opt_flags |= I40E_AQ_VSI_QUE_OPT_TCP_ENA;
        }
 
-       /* Update the VSI after updating the VSI queue-mapping information */
+       /* Update the VSI after updating the VSI queue-mapping
+        * information
+        */
        ret = i40e_aq_update_vsi_params(&vsi->back->hw, &ctxt, NULL);
        if (ret) {
                dev_info(&vsi->back->pdev->dev,

drivers/net/ethernet/intel/i40e/i40e_main.c void i40e_down(struct i40e_vsi *vsi) · line 6337

Quoted 9 lines
drivers/net/ethernet/intel/i40e/i40e_main.c +6 -0
@@ -6168,48 +6329,142 @@ void i40e_down(struct i40e_vsi *vsi)
 }
 
 /**
+ * i40e_validate_mqprio_queue_mapping - validate queue mapping info
+ * @vsi: the VSI being configured
+ * @mqprio_qopt: queue parametrs
+ **/
+int i40e_validate_mqprio_queue_mapping(struct i40e_vsi *vsi,
+                               struct tc_mqprio_qopt_offload *mqprio_qopt)

So I see a few issues with this. From what I can tell this can be static. Second is that it is triggering a check in checkpatch for unaligned parameters.

I would recommend using the gcc coding style if needed so that you could move the "static int" into a separate line if needed in order to avoid issues with the line being too long. In addition you might look at reducing the length of the function name if you still have issues with being over 80 characters. Maybe something like i40e_validate_mqprio_qopt instead of calling out the mapping and all that since it seems like most of this is just about validating the qopt fields provided in the mqprio_qopt structure anyway.

Quoted 181 lines
drivers/net/ethernet/intel/i40e/i40e_main.c +109 -23
+{
+       int i;
+
+       if ((mqprio_qopt->qopt.offset[0] != 0) ||
+           (mqprio_qopt->qopt.num_tc < 1))
+               return -EINVAL;
+
+       for (i = 0; ; i++) {
+               if (!mqprio_qopt->qopt.count[i])
+                       return -EINVAL;
+
+               if (mqprio_qopt->min_rate[i] || mqprio_qopt->max_rate[i])
+                       return -EINVAL;
+
+               if (i >= mqprio_qopt->qopt.num_tc - 1)
+                       break;
+
+               if (mqprio_qopt->qopt.offset[i + 1] !=
+                   (mqprio_qopt->qopt.offset[i] + mqprio_qopt->qopt.count[i]))
+                       return -EINVAL;
+       }
+
+       if (vsi->num_queue_pairs <
+           (mqprio_qopt->qopt.offset[i] + mqprio_qopt->qopt.count[i])) {
+               return -EINVAL;
+       }
+
+       return 0;
+}
+
+/**
  * i40e_setup_tc - configure multiple traffic classes
  * @netdev: net device to configure
- * @tc: number of traffic classes to enable
+ * @tc: pointer to struct tc_to_netdev
  **/
-static int i40e_setup_tc(struct net_device *netdev, u8 tc)
+static int i40e_setup_tc(struct net_device *netdev, struct tc_to_netdev *tc)
 {
        struct i40e_netdev_priv *np = netdev_priv(netdev);
        struct i40e_vsi *vsi = np->vsi;
        struct i40e_pf *pf = vsi->back;
-       u8 enabled_tc = 0;
+       u8 enabled_tc = 0, num_tc = 0, hw = 0;
        int ret = -EINVAL;
        int i;
 
-       /* Check if DCB enabled to continue */
-       if (!(pf->flags & I40E_FLAG_DCB_ENABLED)) {
-               netdev_info(netdev, "DCB is not enabled for adapter\n");
-               goto exit;
+       if (tc->type == TC_SETUP_MQPRIO) {
+               hw = tc->mqprio->hw;
+               num_tc = tc->mqprio->num_tc;
+       } else if (tc->type == TC_SETUP_MQPRIO_EXT) {
+               hw = tc->mqprio_qopt->qopt.hw;
+               num_tc = tc->mqprio_qopt->qopt.num_tc;
        }
 
-       /* Check if MFP enabled */
-       if (pf->flags & I40E_FLAG_MFP_ENABLED) {
-               netdev_info(netdev, "Configuring TC not supported in MFP mode\n");
-               goto exit;
+       if (!hw) {
+               pf->flags &= ~I40E_FLAG_TC_MQPRIO;
+               if (tc->type == TC_SETUP_MQPRIO_EXT)
+                       memcpy(&vsi->mqprio_qopt, tc->mqprio_qopt,
+                              sizeof(*tc->mqprio_qopt));
+               goto config_tc;
        }
 
-       /* Check whether tc count is within enabled limit */
-       if (tc > i40e_pf_get_num_tc(pf)) {
-               netdev_info(netdev, "TC count greater than enabled on link for adapter\n");
-               goto exit;
+       switch (hw) {
+       case TC_MQPRIO_HW_OFFLOAD_TCS:
+               pf->flags &= ~I40E_FLAG_TC_MQPRIO;
+               /* Check if DCB enabled to continue */
+               if (!(pf->flags & I40E_FLAG_DCB_ENABLED)) {
+                       netdev_info(netdev,
+                                   "DCB is not enabled for adapter\n");
+                       goto exit;
+               }
+
+               /* Check if MFP enabled */
+               if (pf->flags & I40E_FLAG_MFP_ENABLED) {
+                       netdev_info(netdev,
+                                   "Configuring TC not supported in MFP mode\n");
+                       goto exit;
+               }
+
+               /* Check whether tc count is within enabled limit */
+               if (num_tc > i40e_pf_get_num_tc(pf)) {
+                       netdev_info(netdev,
+                                   "TC count greater than enabled on link for adapter\n");
+                       goto exit;
+               }
+               break;
+       case TC_MQPRIO_HW_OFFLOAD:
+               if (pf->flags & I40E_FLAG_DCB_ENABLED) {
+                       netdev_info(netdev,
+                                   "Full offload of TC Mqprio options is not supported when DCB is enabled\n");
+                       goto exit;
+               }
+
+               /* Check if MFP enabled */
+               if (pf->flags & I40E_FLAG_MFP_ENABLED) {
+                       netdev_info(netdev,
+                                   "Configuring TC not supported in MFP mode\n");
+                       goto exit;
+               }
+
+               ret = i40e_validate_mqprio_queue_mapping(vsi,
+                                                        tc->mqprio_qopt);
+               if (ret)
+                       goto exit;
+
+               memcpy(&vsi->mqprio_qopt, tc->mqprio_qopt,
+                      sizeof(*tc->mqprio_qopt));
+
+               pf->flags |= I40E_FLAG_TC_MQPRIO;
+               pf->flags &= ~I40E_FLAG_DCB_ENABLED;
+
+               break;
+       default:
+               return -EINVAL;
        }
 
+config_tc:
        /* Generate TC map for number of tc requested */
-       for (i = 0; i < tc; i++)
+       for (i = 0; i < num_tc; i++)
                enabled_tc |= BIT(i);
 
        /* Requesting same TC configuration as already enabled */
-       if (enabled_tc == vsi->tc_config.enabled_tc)
+       if (enabled_tc == vsi->tc_config.enabled_tc &&
+           hw != TC_MQPRIO_HW_OFFLOAD)
                return 0;
 
        /* Quiesce VSI queues */
        i40e_quiesce_vsi(vsi);
 
+       if (!hw && !(pf->flags & I40E_FLAG_TC_MQPRIO))
+               i40e_remove_queue_channel(vsi);
+
        /* Configure VSI for enabled TCs */
        ret = i40e_vsi_config_tc(vsi, enabled_tc);
        if (ret) {
@@ -6229,8 +6484,11 @@ static int i40e_setup_tc(struct net_device *netdev, u8 tc)
 
        /* Unquiesce VSI */
        i40e_unquiesce_vsi(vsi);
+       return ret;
 
 exit:
+       /* Reset the configuration data */
+       memset(&vsi->tc_config, 0, sizeof(vsi->tc_config));
        i40e_unquiesce_vsi(vsi);
        return ret;
 }
@@ -6238,12 +6496,7 @@ static int i40e_setup_tc(struct net_device *netdev, u8 tc)
 static int __i40e_setup_tc(struct net_device *netdev, u32 handle, __be16 proto,
                           struct tc_to_netdev *tc)
 {
-       if (tc->type != TC_SETUP_MQPRIO)
-               return -EINVAL;
-
-       tc->mqprio->hw = TC_MQPRIO_HW_OFFLOAD_TCS;
-
-       return i40e_setup_tc(netdev, tc->mqprio->num_tc);
+       return i40e_setup_tc(netdev, tc);
 }
 
 /**

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

#8 lore raw

PATCH 4/4 net: i40e: Add support to set max bandwidth rates for TCs offloaded via tc/mqprio

In reply to Amritha Nambiar

This patch enables setting up maximum Tx rates for the traffic classes in i40e. The maximum rate offloaded to the hardware through the mqprio framework is configured for the VSI. Configuring minimum Tx rate limit is not supported in the device. The minimum usable value for Tx rate is 50Mbps.

Example:

# tc qdisc add dev eth0 root mqprio num_tc 2  map 0 0 0 0 1 1 1 1\
  queues 4@0 4@4 min_rate 0Mbit 0Mbit max_rate 55Mbit 60Mbit hw 2

To dump the bandwidth rates:

# tc qdisc show dev eth0
qdisc mqprio 804a: root  tc 2 map 0 0 0 0 1 1 1 1 0 0 0 0 0 0 0 0
             queues:(0:3) (4:7)
             min rates:0bit 0bit
             max rates:55Mbit 60Mbit
---
 drivers/net/ethernet/intel/i40e/i40e.h      |    2 +
 drivers/net/ethernet/intel/i40e/i40e_main.c |  102 ++++++++++++++++++++++++++-
 2 files changed, 100 insertions(+), 4 deletions(-)
drivers/net/ethernet/intel/i40e/i40e.h +2 -0
index a62f65a..83a060d 100644
@@ -346,6 +346,8 @@ struct i40e_channel {
 	u8 enabled_tc;
 	struct i40e_aqc_vsi_properties_data info;
 
+	u32 max_tx_rate;
+
 	/* track this channel belongs to which VSI */
 	struct i40e_vsi *parent_vsi;
 };
drivers/net/ethernet/intel/i40e/i40e_main.c +98 -4

145 lines not on this page · raw message

#9 lore raw
In reply to Amritha Nambiar
Quoted 6 lines · Amritha Nambiar

The following series introduces a new harware offload mode in tc/mqprio where the TCs, the queue configurations and bandwidth rate limits are offloaded to the hardware. The i40e driver enables the new mqprio hardware offload mechanism factoring the TCs, queue configuration and bandwidth rates by creating HW channel VSIs.

nice work, fix your time stamp and line wrapping though.

Quoted 14 lines

In this mode, the priority to traffic class mapping and the user specified queue ranges are used to configure the traffic class when the 'hw' option is set to 2. This is achieved by creating HW channels(VSI). A new channel is created for each of the traffic class configuration offloaded via mqprio framework except for the first TC (TC0) which is for the main VSI. TC0 for the main VSI is also reconfigured as per user provided queue parameters. Finally, bandwidth rate limits are set on these traffic classes through the mqprio offload framework by sending these rates in addition to the number of TCs and the queue configurations.
Example:

# tc qdisc add dev eth0 root mqprio num_tc 2  map 0 0 0 0 1 1 1 1\
  queues 4 at 0 4 at 4 min_rate 0Mbit 0Mbit max_rate 55Mbit 60Mbit hw 2
Quoted 8 lines

To dump the bandwidth rates:

# tc qdisc show dev eth0
  qdisc mqprio 804a: root  tc 2 map 0 0 0 0 1 1 1 1 0 0 0 0 0 0 0 0
               queues:(0:3) (4:7)
               min rates:0bit 0bit
               max rates:55Mbit 60Mbit

Looks reasonable to me thanks. Previously, rate limits were being set via dcbnl but I guess this interface is slightly nicer in that it puts all configuration in one spot. IMO it would be nice to push dcbnl users over to this.

Thanks,
.John

#10 lore raw
In reply to Amritha Nambiar
Quoted 1 line · Amritha Nambiar

The following series introduces a new harware offload mode in tc/mqprio

Wait, we have already a HW QoS model introduced by John F and Co couple of years ago, right?

Please elaborate in few sentence if you are extending it/replacing it, why we want to do that and what are the implications on existing applications UAPI wise.

Below you just say in the new mode Qos is configured with knobs XYZ -- this is way not enough

Quoted 7 lines

where the TCs, the queue configurations and bandwidth rate limits are offloaded to the hardware.

The i40e driver enables the new mqprio hardware offload mechanism factoring the TCs, queue configuration and bandwidth rates by creating HW channel VSIs.

In this mode, the priority to traffic class mapping and the user specified queue ranges are used to configure the traffic class when the 'hw' option is set to 2. This is achieved by creating HW channels(VSI). A new channel is created for each of the traffic class configuration offloaded via mqprio framework except for the first TC (TC0) which is for the main VSI. TC0 for the main VSI is also reconfigured as per user provided queue parameters. Finally, bandwidth rate limits are set on these traffic classes through the mqprio offload framework by sending these rates in addition to the number of TCs and the queue configurations.

#11 lore raw
Quoted 6 lines · Or Gerlitz
Quoted 1 line · Amritha Nambiar

The following series introduces a new harware offload mode in tc/mqprio

Wait, we have already a HW QoS model introduced by John F and Co couple of years ago, right?

I assume you are referring to the ETS portion of DCBX? If so then yes we have something there, but there is a fairly high level of complexity and dependencies in order to enable that. What we have in mind doesn't really fit with DCBX and the problem is the spec calls out that you really have to have support for DCBX in order to make use of ETS. What we are trying to do here is provide a lightweight way of doing this configuration similar to how mqprio was already providing a lightweight way of enabling multiple traffic classes.

Quoted 3 lines

Please elaborate in few sentence if you are extending it/replacing it, why we want to do that and what are the implications on existing applications UAPI wise.

This is meant to be an extension of the existing structure. It can be ignored by the driver and instead only have the basic hw offload supported. In such a case the command will still return success but any rate limits and queue configuration can be ignored. In the case of the current implementation the queue configuration was already being ignored so we opted to re-purpose the "hw" flag so that if you pass a value greater than 1 and the driver doesn't recognize the value it has the option to just ignore the extra bits it doesn't recognize and return 1 as it did before for the hw flag in mqprio.

Quoted 2 lines

Below you just say in the new mode Qos is configured with knobs XYZ -- this is way not enough

Can you clarify what you are asking for here? Amritha included an example in patch 1 of a command line that enabled 2 traffic classes with rate limits. Is there something more specific you are looking for?

Thanks.

- Alex

#12 lore raw
Quoted 8 lines · Alexander Duyck
Quoted 6 lines · Or Gerlitz
Quoted 1 line · Amritha Nambiar

The following series introduces a new harware offload mode in tc/mqprio

Wait, we have already a HW QoS model introduced by John F and Co couple of years ago, right?

Quoted 8 lines

I assume you are referring to the ETS portion of DCBX? If so then yes we have something there, but there is a fairly high level of complexity and dependencies in order to enable that. What we have in mind doesn't really fit with DCBX and the problem is the spec calls out that you really have to have support for DCBX in order to make use of ETS. What we are trying to do here is provide a lightweight way of doing this configuration similar to how mqprio was already providing a lightweight way of enabling multiple traffic classes.

UAPI wise, we are talking on DCB, not DCBX, right? the X-portion comes into play if some user-space entity run LLDP traffic and calls into the kernel to configure (say) ETS. If there are some issues to use the existing user-space tool (lldpad tool/daemon) to do DCB without X, one can/should fix them or introduce another/simpler tool that in a lightweight manner only configures things w.o exchanging.

So to clarify, the essence of this series is introducing a 2nd way to configure ETS and rate limit?

Quoted 4 lines
Quoted 3 lines

Please elaborate in few sentence if you are extending it/replacing it, why we want to do that and what are the implications on existing applications UAPI wise.

Quoted 9 lines

This is meant to be an extension of the existing structure. It can be ignored by the driver and instead only have the basic hw offload supported. In such a case the command will still return success but any rate limits and queue configuration can be ignored. In the case of the current implementation the queue configuration was already being ignored so we opted to re-purpose the "hw" flag so that if you pass a value greater than 1 and the driver doesn't recognize the value it has the option to just ignore the extra bits it doesn't recognize and return 1 as it did before for the hw flag in mqprio.

So the user asked to configure something and the kernel returned success but the config was not plugged to the hw? sounds to me like possible (probable) source of troubles and complaints.

Quoted 6 lines
Quoted 2 lines

Below you just say in the new mode Qos is configured with knobs XYZ -- this is way not enough

Can you clarify what you are asking for here? Amritha included an example in patch 1 of a command line that enabled 2 traffic classes with rate limits. Is there something more specific you are looking for?

you were referring to the questions I posted, so we can continue the discussion from here.

#13 lore raw
Quoted 29 lines · Or Gerlitz
Quoted 11 lines · Alexander Duyck
Quoted 8 lines · Or Gerlitz
Quoted 2 lines · Amritha Nambiar

The following series introduces a new harware offload mode in tc/mqprio

Wait, we have already a HW QoS model introduced by John F and Co couple of years ago, right?

Quoted 8 lines

I assume you are referring to the ETS portion of DCBX? If so then yes we have something there, but there is a fairly high level of complexity and dependencies in order to enable that. What we have in mind doesn't really fit with DCBX and the problem is the spec calls out that you really have to have support for DCBX in order to make use of ETS. What we are trying to do here is provide a lightweight way of doing this configuration similar to how mqprio was already providing a lightweight way of enabling multiple traffic classes.

UAPI wise, we are talking on DCB, not DCBX, right? the X-portion comes into play if some user-space entity run LLDP traffic and calls into the kernel to configure (say) ETS. If there are some issues to use the existing user-space tool (lldpad tool/daemon) to do DCB without X, one can/should fix them or introduce another/simpler tool that in a lightweight manner only configures things w.o exchanging.

Quoted 2 lines

So to clarify, the essence of this series is introducing a 2nd way to configure ETS and rate limit?

Sort of. Basically the idea is we can use several different approaches to enable queue configuration and rate limits. So we are adding two pieces of functionality.

The first block allows for configuring queue counts and layout. Historically DCB/DCBX hasn't allowed us to specify that.

The second bit is that we added support for rate limiting. I am actually basing it on what we had for SR-IOV rate limiting as that is how this is working in i40e. However the basic attributes we are adding should work for most ETS type use cases though it might need to use the min rates instead of the max rates as we do in i40e.

Quoted 17 lines
Quoted 7 lines
Quoted 5 lines

Please elaborate in few sentence if you are extending it/replacing it, why we want to do that and what are the implications on existing applications UAPI wise.

Quoted 9 lines

This is meant to be an extension of the existing structure. It can be ignored by the driver and instead only have the basic hw offload supported. In such a case the command will still return success but any rate limits and queue configuration can be ignored. In the case of the current implementation the queue configuration was already being ignored so we opted to re-purpose the "hw" flag so that if you pass a value greater than 1 and the driver doesn't recognize the value it has the option to just ignore the extra bits it doesn't recognize and return 1 as it did before for the hw flag in mqprio.

Quoted 3 lines

So the user asked to configure something and the kernel returned success but the config was not plugged to the hw? sounds to me like possible (probable) source of troubles and complaints.

That is possible, however the issue already existed. The queue configuration could be specified with the mqprio configuration and be totally ignored. I opted for just maintaining the existing behavior and moving forward and providing some input via the ability to report what "version" of the hardware offload we are supporting.

The plan is that legacy devices can fall back into the setup where they support mode 1, however if they support hw mode 2 then we will fail to initialize if we don't support something that is being requested.

Quoted 16 lines
Quoted 9 lines
Quoted 4 lines

Below you just say in the new mode Qos is configured with knobs XYZ -- this is way not enough

Can you clarify what you are asking for here? Amritha included an example in patch 1 of a command line that enabled 2 traffic classes with rate limits. Is there something more specific you are looking for?

you were referring to the questions I posted, so we can continue the discussion from here.
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

#14 lore raw
In reply to Amritha Nambiar
Quoted 22 lines · Amritha Nambiar

The following series introduces a new harware offload mode in tc/mqprio where the TCs, the queue configurations and bandwidth rate limits are offloaded to the hardware.
The i40e driver enables the new mqprio hardware offload mechanism factoring the TCs, queue configuration and bandwidth rates by creating HW channel VSIs.?

In this mode, the priority to traffic class mapping and the user specified queue ranges are used to configure the traffic class when the 'hw' option is set to 2. This is achieved by creating HW channels(VSI). A new channel is created for each of the traffic class configuration offloaded via mqprio framework except for the first TC (TC0) which is for the main VSI. TC0 for the main VSI is also reconfigured as per user provided queue parameters. Finally, bandwidth rate limits are set on these traffic classes through the mqprio offload framework by sending these rates in addition to the number of TCs and the queue configurations.

Example:

# tc qdisc add dev eth0 root mqprio num_tc 2??map 0 0 0 0 1 1 1 1\
? queues 4 at 0 4 at 4 min_rate 0Mbit 0Mbit max_rate 55Mbit 60Mbit hw 2
Quoted 8 lines

To dump the bandwidth rates:

# tc qdisc show dev eth0
? qdisc mqprio 804a: root??tc 2 map 0 0 0 0 1 1 1 1 0 0 0 0 0 0 0 0
???????????????queues:(0:3) (4:7)
???????????????min rates:0bit 0bit
???????????????max rates:55Mbit 60Mbit

This was meant to be sent out as an RFC, but apparently that did not get
conveyed when these were sent out Friday.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20170522/d42362ce/attachment.asc>

#15 lore raw
Quoted 4 lines · Jeff Kirsher
Quoted 3 lines · Amritha Nambiar

The following series introduces a new harware offload mode in tc/mqprio where the TCs, the queue configurations and bandwidth rate limits are offloaded to the hardware.

...

Quoted 2 lines

This was meant to be sent out as an RFC, but apparently that did not get conveyed when these were sent out Friday.

I am looking at tc/mqprio and dcb with the purpose of implementing

- Forwarding and Queuing Enhancements for Time-Sensitive Streams (FQTSS)

    802.1Q-2014 Clause 34

- Scheduled Traffic (time based scheduling)

    P802.1Qbv

using the HW capabilities of the i210. This series looks like a promising avenue for these features.

My question is, did series go anywhere?

I didn't see any follow ups on netdev, but maybe I missed something.

Thanks,
Richard

#16 lore raw
Quoted 17 lines · Richard Cochran
Quoted 4 lines · Jeff Kirsher
Quoted 3 lines · Amritha Nambiar

The following series introduces a new harware offload mode in tc/mqprio where the TCs, the queue configurations and bandwidth rate limits are offloaded to the hardware.

...

Quoted 2 lines

This was meant to be sent out as an RFC, but apparently that did not get conveyed when these were sent out Friday.

I am looking at tc/mqprio and dcb with the purpose of implementing

- Forwarding and Queuing Enhancements for Time-Sensitive Streams (FQTSS)

    802.1Q-2014 Clause 34

- Scheduled Traffic (time based scheduling)

Quoted 8 lines
    P802.1Qbv

using the HW capabilities of the i210. This series looks like a promising avenue for these features.

My question is, did series go anywhere?

I didn't see any follow ups on netdev, but maybe I missed something.

I have submitted another series as non-RFC for next-queue maintained by Jeff Kirsher. This is going to come through the next-queue tree. You can follow the new series "[PATCH 0/6] Configuring traffic classes via new hardware offload mechanism in tc/mqprio" at
https://www.mail-archive.com/netdev at vger.kernel.org/msg177390.html

Quoted 6 lines

Thanks,
Richard