[PATCH net 0/6] hinic: BugFixes

[PATCH net 0/6] hinic: BugFixes

[Luo bin]

Fix a number of bugs which have been present since the first commit.

The bugs fixed in these patchs are hardly exposed unless given
very specific conditions.

Luo bin (6):
  hinic: fix process of long length skb without frags
  hinic: fix a bug of waitting for IO stopped
  hinic: fix the bug of clearing event queue
  hinic: fix out-of-order excution in arm cpu
  hinic: fix wrong para of wait_for_completion_timeout
  hinic: fix wrong value of MIN_SKB_LEN

 .../net/ethernet/huawei/hinic/hinic_ethtool.c |  1 +
 .../net/ethernet/huawei/hinic/hinic_hw_cmdq.c |  5 +-
 .../net/ethernet/huawei/hinic/hinic_hw_dev.c  | 51 +------------------
 .../net/ethernet/huawei/hinic/hinic_hw_eqs.c  | 27 +++++++---
 .../net/ethernet/huawei/hinic/hinic_hw_mgmt.c |  5 +-
 .../net/ethernet/huawei/hinic/hinic_main.c    |  1 +
 drivers/net/ethernet/huawei/hinic/hinic_rx.c  |  3 ++
 drivers/net/ethernet/huawei/hinic/hinic_tx.c  | 15 ++++--
 drivers/net/ethernet/huawei/hinic/hinic_tx.h  |  2 +-
 9 files changed, 46 insertions(+), 64 deletions(-)

-- 
2.17.1

[PATCH net 1/6] hinic: fix process of long length skb without frags

[Luo bin]

some tool such as pktgen can build an illegal skb with long
length but no fragments, which is unsupported for hw, so
drop it

Signed-off-by: Luo bin <luobin9@huawei.com>
---
 drivers/net/ethernet/huawei/hinic/hinic_ethtool.c |  1 +
 drivers/net/ethernet/huawei/hinic/hinic_main.c    |  1 +
 drivers/net/ethernet/huawei/hinic/hinic_tx.c      | 13 +++++++++----
 drivers/net/ethernet/huawei/hinic/hinic_tx.h      |  2 +-
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
index 966aea949c0b..dac157bc06a7 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
@@ -582,6 +582,7 @@ static struct hinic_stats hinic_tx_queue_stats[] = {
 	HINIC_TXQ_STAT(tx_wake),
 	HINIC_TXQ_STAT(tx_dropped),
 	HINIC_TXQ_STAT(big_frags_pkts),
+	HINIC_TXQ_STAT(frag_len_overflow),
 };
 
 #define HINIC_RXQ_STAT(_stat_item) { \
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_main.c b/drivers/net/ethernet/huawei/hinic/hinic_main.c
index 13560975c103..a9bee70bd6c7 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_main.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_main.c
@@ -107,6 +107,7 @@ static void update_tx_stats(struct hinic_dev *nic_dev, struct hinic_txq *txq)
 	nic_tx_stats->tx_wake += tx_stats.tx_wake;
 	nic_tx_stats->tx_dropped += tx_stats.tx_dropped;
 	nic_tx_stats->big_frags_pkts += tx_stats.big_frags_pkts;
+	nic_tx_stats->frag_len_overflow += tx_stats.frag_len_overflow;
 	u64_stats_update_end(&nic_tx_stats->syncp);
 
 	hinic_txq_clean_stats(txq);
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_tx.c b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
index 0e13d1c7e474..3c6762086fff 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_tx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
@@ -45,9 +45,10 @@
 
 #define HW_CONS_IDX(sq)                 be16_to_cpu(*(u16 *)((sq)->hw_ci_addr))
 
-#define MIN_SKB_LEN                     17
+#define MIN_SKB_LEN			17
+#define HINIC_GSO_MAX_SIZE		65536
 
-#define	MAX_PAYLOAD_OFFSET	        221
+#define	MAX_PAYLOAD_OFFSET		221
 #define TRANSPORT_OFFSET(l4_hdr, skb)	((u32)((l4_hdr) - (skb)->data))
 
 union hinic_l3 {
@@ -84,6 +85,7 @@ void hinic_txq_clean_stats(struct hinic_txq *txq)
 	txq_stats->tx_wake = 0;
 	txq_stats->tx_dropped = 0;
 	txq_stats->big_frags_pkts = 0;
+	txq_stats->frag_len_overflow = 0;
 	u64_stats_update_end(&txq_stats->syncp);
 }
 
@@ -106,6 +108,7 @@ void hinic_txq_get_stats(struct hinic_txq *txq, struct hinic_txq_stats *stats)
 		stats->tx_wake = txq_stats->tx_wake;
 		stats->tx_dropped = txq_stats->tx_dropped;
 		stats->big_frags_pkts = txq_stats->big_frags_pkts;
+		stats->frag_len_overflow = txq_stats->frag_len_overflow;
 	} while (u64_stats_fetch_retry(&txq_stats->syncp, start));
 	u64_stats_update_end(&stats->syncp);
 }
@@ -440,7 +443,6 @@ static int hinic_tx_offload(struct sk_buff *skb, struct hinic_sq_task *task,
 			     vlan_tag >> VLAN_PRIO_SHIFT);
 		offload |= TX_OFFLOAD_VLAN;
 	}
-
 	if (offload)
 		hinic_task_set_l2hdr(task, skb_network_offset(skb));
 
@@ -488,11 +490,14 @@ netdev_tx_t hinic_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 		txq->txq_stats.big_frags_pkts++;
 		u64_stats_update_end(&txq->txq_stats.syncp);
 	}
-
 	if (nr_sges > txq->max_sges) {
 		netdev_err(netdev, "Too many Tx sges\n");
 		goto skb_error;
 	}
+	if (unlikely(skb->len > HINIC_GSO_MAX_SIZE && nr_sges == 1)) {
+		txq->txq_stats.frag_len_overflow++;
+		goto skb_error;
+	}
 
 	err = tx_map_skb(nic_dev, skb, txq->sges);
 	if (err)
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_tx.h b/drivers/net/ethernet/huawei/hinic/hinic_tx.h
index f158b7db7fb8..ac65b4301c09 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_tx.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_tx.h
@@ -22,7 +22,7 @@ struct hinic_txq_stats {
 	u64     tx_wake;
 	u64     tx_dropped;
 	u64	big_frags_pkts;
-
+	u64     frag_len_overflow;
 	struct u64_stats_sync   syncp;
 };
 
-- 
2.17.1

[PATCH net 2/6] hinic: fix a bug of waitting for IO stopped

[Luo bin]

it's unreliable for fw to check whether IO is stopped, so driver
wait for enough time to ensure IO process is done in hw before
freeing resources

Signed-off-by: Luo bin <luobin9@huawei.com>
---
 .../net/ethernet/huawei/hinic/hinic_hw_dev.c  | 51 +------------------
 1 file changed, 2 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
index 79b3d53f2fbf..c7c75b772a86 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
@@ -360,50 +360,6 @@ static int wait_for_db_state(struct hinic_hwdev *hwdev)
 	return -EFAULT;
 }
 
-static int wait_for_io_stopped(struct hinic_hwdev *hwdev)
-{
-	struct hinic_cmd_io_status cmd_io_status;
-	struct hinic_hwif *hwif = hwdev->hwif;
-	struct pci_dev *pdev = hwif->pdev;
-	struct hinic_pfhwdev *pfhwdev;
-	unsigned long end;
-	u16 out_size;
-	int err;
-
-	if (!HINIC_IS_PF(hwif) && !HINIC_IS_PPF(hwif)) {
-		dev_err(&pdev->dev, "Unsupported PCI Function type\n");
-		return -EINVAL;
-	}
-
-	pfhwdev = container_of(hwdev, struct hinic_pfhwdev, hwdev);
-
-	cmd_io_status.func_idx = HINIC_HWIF_FUNC_IDX(hwif);
-
-	end = jiffies + msecs_to_jiffies(IO_STATUS_TIMEOUT);
-	do {
-		err = hinic_msg_to_mgmt(&pfhwdev->pf_to_mgmt, HINIC_MOD_COMM,
-					HINIC_COMM_CMD_IO_STATUS_GET,
-					&cmd_io_status, sizeof(cmd_io_status),
-					&cmd_io_status, &out_size,
-					HINIC_MGMT_MSG_SYNC);
-		if ((err) || (out_size != sizeof(cmd_io_status))) {
-			dev_err(&pdev->dev, "Failed to get IO status, ret = %d\n",
-				err);
-			return err;
-		}
-
-		if (cmd_io_status.status == IO_STOPPED) {
-			dev_info(&pdev->dev, "IO stopped\n");
-			return 0;
-		}
-
-		msleep(20);
-	} while (time_before(jiffies, end));
-
-	dev_err(&pdev->dev, "Wait for IO stopped - Timeout\n");
-	return -ETIMEDOUT;
-}
-
 /**
  * clear_io_resource - set the IO resources as not active in the NIC
  * @hwdev: the NIC HW device
@@ -423,11 +379,8 @@ static int clear_io_resources(struct hinic_hwdev *hwdev)
 		return -EINVAL;
 	}
 
-	err = wait_for_io_stopped(hwdev);
-	if (err) {
-		dev_err(&pdev->dev, "IO has not stopped yet\n");
-		return err;
-	}
+	/* sleep 100ms to wait for firmware stopping I/O */
+	msleep(100);
 
 	cmd_clear_io_res.func_idx = HINIC_HWIF_FUNC_IDX(hwif);
 
-- 
2.17.1

[PATCH net 3/6] hinic: fix the bug of clearing event queue

[Luo bin]

should disable and synchronize eq irq before freeing it, must
clear event queue depth in hw before freeing relevant memory
to avoid illegal memory access and update consumer idx to avoid
invalid interrupt

Signed-off-by: Luo bin <luobin9@huawei.com>
---
 .../net/ethernet/huawei/hinic/hinic_hw_eqs.c  | 25 +++++++++++++------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_eqs.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_eqs.c
index 79243b626ddb..13f3abac54ea 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_eqs.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_eqs.c
@@ -188,7 +188,7 @@ static u8 eq_cons_idx_checksum_set(u32 val)
  * eq_update_ci - update the HW cons idx of event queue
  * @eq: the event queue to update the cons idx for
  **/
-static void eq_update_ci(struct hinic_eq *eq)
+static void eq_update_ci(struct hinic_eq *eq, u32 arm_state)
 {
 	u32 val, addr = EQ_CONS_IDX_REG_ADDR(eq);
 
@@ -202,7 +202,7 @@ static void eq_update_ci(struct hinic_eq *eq)
 
 	val |= HINIC_EQ_CI_SET(eq->cons_idx, IDX)    |
 	       HINIC_EQ_CI_SET(eq->wrapped, WRAPPED) |
-	       HINIC_EQ_CI_SET(EQ_ARMED, INT_ARMED);
+	       HINIC_EQ_CI_SET(arm_state, INT_ARMED);
 
 	val |= HINIC_EQ_CI_SET(eq_cons_idx_checksum_set(val), XOR_CHKSUM);
 
@@ -347,7 +347,7 @@ static void eq_irq_handler(void *data)
 	else if (eq->type == HINIC_CEQ)
 		ceq_irq_handler(eq);
 
-	eq_update_ci(eq);
+	eq_update_ci(eq, EQ_ARMED);
 }
 
 /**
@@ -702,7 +702,7 @@ static int init_eq(struct hinic_eq *eq, struct hinic_hwif *hwif,
 	}
 
 	set_eq_ctrls(eq);
-	eq_update_ci(eq);
+	eq_update_ci(eq, EQ_ARMED);
 
 	err = alloc_eq_pages(eq);
 	if (err) {
@@ -752,18 +752,29 @@ static int init_eq(struct hinic_eq *eq, struct hinic_hwif *hwif,
  **/
 static void remove_eq(struct hinic_eq *eq)
 {
-	struct msix_entry *entry = &eq->msix_entry;
-
-	free_irq(entry->vector, eq);
+	hinic_set_msix_state(eq->hwif, eq->msix_entry.entry,
+			     HINIC_MSIX_DISABLE);
+	synchronize_irq(eq->msix_entry.vector);
+	free_irq(eq->msix_entry.vector, eq);
 
 	if (eq->type == HINIC_AEQ) {
 		struct hinic_eq_work *aeq_work = &eq->aeq_work;
 
 		cancel_work_sync(&aeq_work->work);
+		/* clear aeq_len to avoid hw access host memory */
+		hinic_hwif_write_reg(eq->hwif,
+				     HINIC_CSR_AEQ_CTRL_1_ADDR(eq->q_id), 0);
 	} else if (eq->type == HINIC_CEQ) {
 		tasklet_kill(&eq->ceq_tasklet);
+		/* clear ceq_len to avoid hw access host memory */
+		hinic_hwif_write_reg(eq->hwif,
+				     HINIC_CSR_CEQ_CTRL_1_ADDR(eq->q_id), 0);
 	}
 
+	/* update cons_idx to avoid invalid interrupt */
+	eq->cons_idx = hinic_hwif_read_reg(eq->hwif, EQ_PROD_IDX_REG_ADDR(eq));
+	eq_update_ci(eq, EQ_NOT_ARMED);
+
 	free_eq_pages(eq);
 }
 
-- 
2.17.1

[PATCH net 4/6] hinic: fix out-of-order excution in arm cpu

[Luo bin]

add read barrier in driver code to keep from reading other fileds
in dma memory which is writable for hw until we have verified the
memory is valid for driver

Signed-off-by: Luo bin <luobin9@huawei.com>
---
 drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 2 ++
 drivers/net/ethernet/huawei/hinic/hinic_hw_eqs.c  | 2 ++
 drivers/net/ethernet/huawei/hinic/hinic_rx.c      | 3 +++
 drivers/net/ethernet/huawei/hinic/hinic_tx.c      | 2 ++
 4 files changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c
index eb53c15b13f3..33f93cc25193 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c
@@ -623,6 +623,8 @@ static int cmdq_cmd_ceq_handler(struct hinic_cmdq *cmdq, u16 ci,
 	if (!CMDQ_WQE_COMPLETED(be32_to_cpu(ctrl->ctrl_info)))
 		return -EBUSY;
 
+	dma_rmb();
+
 	errcode = CMDQ_WQE_ERRCODE_GET(be32_to_cpu(status->status_info), VAL);
 
 	cmdq_sync_cmd_handler(cmdq, ci, errcode);
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_eqs.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_eqs.c
index 13f3abac54ea..fb662ab9b85d 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_eqs.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_eqs.c
@@ -235,6 +235,8 @@ static void aeq_irq_handler(struct hinic_eq *eq)
 		if (HINIC_EQ_ELEM_DESC_GET(aeqe_desc, WRAPPED) == eq->wrapped)
 			break;
 
+		dma_rmb();
+
 		event = HINIC_EQ_ELEM_DESC_GET(aeqe_desc, TYPE);
 		if (event >= HINIC_MAX_AEQ_EVENTS) {
 			dev_err(&pdev->dev, "Unknown AEQ Event %d\n", event);
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_rx.c b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
index 2695ad69fca6..815649e37cb1 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_rx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
@@ -350,6 +350,9 @@ static int rxq_recv(struct hinic_rxq *rxq, int budget)
 		if (!rq_wqe)
 			break;
 
+		/* make sure we read rx_done before packet length */
+		dma_rmb();
+
 		cqe = rq->cqe[ci];
 		status =  be32_to_cpu(cqe->status);
 		hinic_rq_get_sge(rxq->rq, rq_wqe, ci, &sge);
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_tx.c b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
index 3c6762086fff..cabcc9019ee4 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_tx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
@@ -627,6 +627,8 @@ static int free_tx_poll(struct napi_struct *napi, int budget)
 	do {
 		hw_ci = HW_CONS_IDX(sq) & wq->mask;
 
+		dma_rmb();
+
 		/* Reading a WQEBB to get real WQE size and consumer index. */
 		sq_wqe = hinic_sq_read_wqebb(sq, &skb, &wqe_size, &sw_ci);
 		if ((!sq_wqe) ||
-- 
2.17.1

[PATCH net 5/6] hinic: fix wrong para of wait_for_completion_timeout

[Luo bin]

the second input parameter of wait_for_completion_timeout should
be jiffies instead of millisecond

Signed-off-by: Luo bin <luobin9@huawei.com>
---
 drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 3 ++-
 drivers/net/ethernet/huawei/hinic/hinic_hw_mgmt.c | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c
index 33f93cc25193..5f2d57d1b2d3 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c
@@ -389,7 +389,8 @@ static int cmdq_sync_cmd_direct_resp(struct hinic_cmdq *cmdq,
 
 	spin_unlock_bh(&cmdq->cmdq_lock);
 
-	if (!wait_for_completion_timeout(&done, CMDQ_TIMEOUT)) {
+	if (!wait_for_completion_timeout(&done,
+					 msecs_to_jiffies(CMDQ_TIMEOUT))) {
 		spin_lock_bh(&cmdq->cmdq_lock);
 
 		if (cmdq->errcode[curr_prod_idx] == &errcode)
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_mgmt.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_mgmt.c
index c1a6be6bf6a8..8995e32dd1c0 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_mgmt.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_mgmt.c
@@ -43,7 +43,7 @@
 
 #define MSG_NOT_RESP                    0xFFFF
 
-#define MGMT_MSG_TIMEOUT                1000
+#define MGMT_MSG_TIMEOUT                5000
 
 #define mgmt_to_pfhwdev(pf_mgmt)        \
 		container_of(pf_mgmt, struct hinic_pfhwdev, pf_to_mgmt)
@@ -267,7 +267,8 @@ static int msg_to_mgmt_sync(struct hinic_pf_to_mgmt *pf_to_mgmt,
 		goto unlock_sync_msg;
 	}
 
-	if (!wait_for_completion_timeout(recv_done, MGMT_MSG_TIMEOUT)) {
+	if (!wait_for_completion_timeout(recv_done,
+					 msecs_to_jiffies(MGMT_MSG_TIMEOUT))) {
 		dev_err(&pdev->dev, "MGMT timeout, MSG id = %d\n", msg_id);
 		err = -ETIMEDOUT;
 		goto unlock_sync_msg;
-- 
2.17.1

[PATCH net 6/6] hinic: fix wrong value of MIN_SKB_LEN

[Luo bin]

the minimum value of skb len that hw supports is 32 rather than 17

Signed-off-by: Luo bin <luobin9@huawei.com>
---
 drivers/net/ethernet/huawei/hinic/hinic_tx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_tx.c b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
index cabcc9019ee4..8993f5b07059 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_tx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
@@ -45,7 +45,7 @@
 
 #define HW_CONS_IDX(sq)                 be16_to_cpu(*(u16 *)((sq)->hw_ci_addr))
 
-#define MIN_SKB_LEN			17
+#define MIN_SKB_LEN			32
 #define HINIC_GSO_MAX_SIZE		65536
 
 #define	MAX_PAYLOAD_OFFSET		221
-- 
2.17.1

Re: [PATCH net 1/6] hinic: fix process of long length skb without frags

[Jakub Kicinski]

On Mon, 16 Mar 2020 00:56:25 +0000 Luo bin wrote:
> -#define MIN_SKB_LEN                     17
> +#define MIN_SKB_LEN			17
> +#define HINIC_GSO_MAX_SIZE		65536

> +	if (unlikely(skb->len > HINIC_GSO_MAX_SIZE && nr_sges == 1)) {
> +		txq->txq_stats.frag_len_overflow++;
> +		goto skb_error;
> +	}

I don't think drivers should have to check this condition.

We have netdev->gso_max_size which should be initialized to 

include/linux/netdevice.h:#define GSO_MAX_SIZE          65536

in

net/core/dev.c: dev->gso_max_size = GSO_MAX_SIZE;

Please send a patch to pktgen to uphold the normal stack guarantees.

Re: [PATCH net 3/6] hinic: fix the bug of clearing event queue

[Jakub Kicinski]

On Mon, 16 Mar 2020 00:56:27 +0000 Luo bin wrote:
> +	synchronize_irq(eq->msix_entry.vector);
> +	free_irq(eq->msix_entry.vector, eq);

Doesn't free_irq() imply synchronize_irq()?

Re: [PATCH net 1/6] hinic: fix process of long length skb without frags

[David Miller]

From: Jakub Kicinski <kuba@kernel.org>
Date: Mon, 16 Mar 2020 14:44:08 -0700

> On Mon, 16 Mar 2020 00:56:25 +0000 Luo bin wrote:
>> -#define MIN_SKB_LEN                     17
>> +#define MIN_SKB_LEN			17
>> +#define HINIC_GSO_MAX_SIZE		65536
> 
>> +	if (unlikely(skb->len > HINIC_GSO_MAX_SIZE && nr_sges == 1)) {
>> +		txq->txq_stats.frag_len_overflow++;
>> +		goto skb_error;
>> +	}
> 
> I don't think drivers should have to check this condition.
> 
> We have netdev->gso_max_size which should be initialized to 
> 
> include/linux/netdevice.h:#define GSO_MAX_SIZE          65536
> 
> in
> 
> net/core/dev.c: dev->gso_max_size = GSO_MAX_SIZE;
> 
> Please send a patch to pktgen to uphold the normal stack guarantees.

Agreed, the driver should not have to validate this.

Re: [PATCH net 1/6] hinic: fix process of long length skb without frags

[luobin (L)]

Ok，I will undo this patch.

On 2020/3/17 8:33, David Miller wrote:
> From: Jakub Kicinski <kuba@kernel.org>
> Date: Mon, 16 Mar 2020 14:44:08 -0700
>
>> On Mon, 16 Mar 2020 00:56:25 +0000 Luo bin wrote:
>>> -#define MIN_SKB_LEN                     17
>>> +#define MIN_SKB_LEN			17
>>> +#define HINIC_GSO_MAX_SIZE		65536
>>> +	if (unlikely(skb->len > HINIC_GSO_MAX_SIZE && nr_sges == 1)) {
>>> +		txq->txq_stats.frag_len_overflow++;
>>> +		goto skb_error;
>>> +	}
>> I don't think drivers should have to check this condition.
>>
>> We have netdev->gso_max_size which should be initialized to
>>
>> include/linux/netdevice.h:#define GSO_MAX_SIZE          65536
>>
>> in
>>
>> net/core/dev.c: dev->gso_max_size = GSO_MAX_SIZE;
>>
>> Please send a patch to pktgen to uphold the normal stack guarantees.
> Agreed, the driver should not have to validate this.
> .