[PATCH 1/2] be2net: pass if_id for v1 and V2 versions of TX_CREATE cmd

[PATCH 1/2] be2net: pass if_id for v1 and V2 versions of TX_CREATE cmd

[Sathya Perla]

From: Vasundhara Volam <vasundhara.volam@emulex.com>

It is a required field for all TX_CREATE cmd versions > 0.
This fixes a driver initialization failure, caused by recent SH-R Firmwares
(versions > 10.0.639.0) failing the TX_CREATE cmd when if_id field is
not passed.

Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index bd0e0c0..c08fd32 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -1198,7 +1198,6 @@ int be_cmd_txq_create(struct be_adapter *adapter, struct be_tx_obj *txo)
 
 	if (lancer_chip(adapter)) {
 		req->hdr.version = 1;
-		req->if_id = cpu_to_le16(adapter->if_handle);
 	} else if (BEx_chip(adapter)) {
 		if (adapter->function_caps & BE_FUNCTION_CAPS_SUPER_NIC)
 			req->hdr.version = 2;
@@ -1206,6 +1205,8 @@ int be_cmd_txq_create(struct be_adapter *adapter, struct be_tx_obj *txo)
 		req->hdr.version = 2;
 	}
 
+	if (req->hdr.version > 0)
+		req->if_id = cpu_to_le16(adapter->if_handle);
 	req->num_pages = PAGES_4K_SPANNED(q_mem->va, q_mem->size);
 	req->ulp_num = BE_ULP1_NUM;
 	req->type = BE_ETH_TX_RING_TYPE_STANDARD;
-- 
1.7.1

[PATCH 2/2] be2net: drop non-tso frames longer than mtu

[Sathya Perla]

From: Vasundhara Volam <vasundhara.volam@emulex.com>

Pktgen can generate non-TSO frames of arbitrary length disregarding
the MTU value of the physical interface. Drop such frames in the driver
instead of sending them to HW as it cannot handle such frames.

Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_main.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 2c38cc4..76057b8 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -855,6 +855,13 @@ static struct sk_buff *be_xmit_workarounds(struct be_adapter *adapter,
 	unsigned int eth_hdr_len;
 	struct iphdr *ip;
 
+	/* Don't allow non-TSO packets longer than MTU */
+	eth_hdr_len = (ntohs(skb->protocol) == ETH_P_8021Q) ?
+			VLAN_ETH_HLEN : ETH_HLEN;
+	if (!skb_is_gso(skb) &&
+	    (skb->len - eth_hdr_len) > adapter->netdev->mtu)
+			goto tx_drop;
+
 	/* Lancer, SH-R ASICs have a bug wherein Packets that are 32 bytes or less
 	 * may cause a transmit stall on that port. So the work-around is to
 	 * pad short packets (<= 32 bytes) to a 36-byte length.
@@ -869,8 +876,6 @@ static struct sk_buff *be_xmit_workarounds(struct be_adapter *adapter,
 	 * incorrecly when VLAN tag is inserted by HW.
 	 * For padded packets, Lancer computes incorrect checksum.
 	 */
-	eth_hdr_len = ntohs(skb->protocol) == ETH_P_8021Q ?
-						VLAN_ETH_HLEN : ETH_HLEN;
 	if (skb->len <= 60 &&
 	    (lancer_chip(adapter) || vlan_tx_tag_present(skb)) &&
 	    is_ipv4_pkt(skb)) {
-- 
1.7.1

Re: [PATCH 2/2] be2net: drop non-tso frames longer than mtu

[Sergei Shtylyov]

Hello.

On 15-10-2013 15:56, Sathya Perla wrote:

> From: Vasundhara Volam <vasundhara.volam@emulex.com>

> Pktgen can generate non-TSO frames of arbitrary length disregarding
> the MTU value of the physical interface. Drop such frames in the driver
> instead of sending them to HW as it cannot handle such frames.

> Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
> ---
>   drivers/net/ethernet/emulex/benet/be_main.c |    9 +++++++--
>   1 files changed, 7 insertions(+), 2 deletions(-)

> diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
> index 2c38cc4..76057b8 100644
> --- a/drivers/net/ethernet/emulex/benet/be_main.c
> +++ b/drivers/net/ethernet/emulex/benet/be_main.c
> @@ -855,6 +855,13 @@ static struct sk_buff *be_xmit_workarounds(struct be_adapter *adapter,
>   	unsigned int eth_hdr_len;
>   	struct iphdr *ip;
>
> +	/* Don't allow non-TSO packets longer than MTU */
> +	eth_hdr_len = (ntohs(skb->protocol) == ETH_P_8021Q) ?
> +			VLAN_ETH_HLEN : ETH_HLEN;
> +	if (!skb_is_gso(skb) &&
> +	    (skb->len - eth_hdr_len) > adapter->netdev->mtu)
> +			goto tx_drop;

    This *goto* is indented one tab too much.

WBR, Sergei

Re: [PATCH 2/2] be2net: drop non-tso frames longer than mtu

[Eric Dumazet]

On Tue, 2013-10-15 at 17:26 +0530, Sathya Perla wrote:
> From: Vasundhara Volam <vasundhara.volam@emulex.com>
> 
> Pktgen can generate non-TSO frames of arbitrary length disregarding
> the MTU value of the physical interface. Drop such frames in the driver
> instead of sending them to HW as it cannot handle such frames.
> 
> Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
> ---
>  drivers/net/ethernet/emulex/benet/be_main.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
> index 2c38cc4..76057b8 100644
> --- a/drivers/net/ethernet/emulex/benet/be_main.c
> +++ b/drivers/net/ethernet/emulex/benet/be_main.c
> @@ -855,6 +855,13 @@ static struct sk_buff *be_xmit_workarounds(struct be_adapter *adapter,
>  	unsigned int eth_hdr_len;
>  	struct iphdr *ip;
>  
> +	/* Don't allow non-TSO packets longer than MTU */
> +	eth_hdr_len = (ntohs(skb->protocol) == ETH_P_8021Q) ?
> +			VLAN_ETH_HLEN : ETH_HLEN;
> +	if (!skb_is_gso(skb) &&
> +	    (skb->len - eth_hdr_len) > adapter->netdev->mtu)
> +			goto tx_drop;
> +

When you say 'cannot handle them', is it some kind of nasty thing like
hang / crash ?

One could imagine gso_size + sizeof(headers) > mtu, and give the same
problem ?


AFAIK, we have no check in net/core/dev.c.

Maybe we should instead add them there (and in pktgen)

Re: [PATCH 2/2] be2net: drop non-tso frames longer than mtu

[Ivan Vecera]

On 10/15/2013 03:47 PM, Eric Dumazet wrote:
> On Tue, 2013-10-15 at 17:26 +0530, Sathya Perla wrote:
>> From: Vasundhara Volam <vasundhara.volam@emulex.com>
>>
>> Pktgen can generate non-TSO frames of arbitrary length disregarding
>> the MTU value of the physical interface. Drop such frames in the driver
>> instead of sending them to HW as it cannot handle such frames.
>>
>> Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
>> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
>> ---
>>   drivers/net/ethernet/emulex/benet/be_main.c |    9 +++++++--
>>   1 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
>> index 2c38cc4..76057b8 100644
>> --- a/drivers/net/ethernet/emulex/benet/be_main.c
>> +++ b/drivers/net/ethernet/emulex/benet/be_main.c
>> @@ -855,6 +855,13 @@ static struct sk_buff *be_xmit_workarounds(struct be_adapter *adapter,
>>   	unsigned int eth_hdr_len;
>>   	struct iphdr *ip;
>>
>> +	/* Don't allow non-TSO packets longer than MTU */
>> +	eth_hdr_len = (ntohs(skb->protocol) == ETH_P_8021Q) ?
>> +			VLAN_ETH_HLEN : ETH_HLEN;
>> +	if (!skb_is_gso(skb) &&
>> +	    (skb->len - eth_hdr_len) > adapter->netdev->mtu)
>> +			goto tx_drop;
>> +
>
> When you say 'cannot handle them', is it some kind of nasty thing like
> hang / crash ?
AFAIK, the firmware in the card becomes unresponsive and reboot is 
needed to make the NIC working.

Ivan

RE: [PATCH 2/2] be2net: drop non-tso frames longer than mtu

[Sathya Perla]

> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> >
> > Pktgen can generate non-TSO frames of arbitrary length disregarding
> > the MTU value of the physical interface. Drop such frames in the driver
> > instead of sending them to HW as it cannot handle such frames.
> >
> > Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
> > Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
> > ---
> >  drivers/net/ethernet/emulex/benet/be_main.c |    9 +++++++--
> >  1 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/emulex/benet/be_main.c
> b/drivers/net/ethernet/emulex/benet/be_main.c
> > index 2c38cc4..76057b8 100644
> > --- a/drivers/net/ethernet/emulex/benet/be_main.c
> > +++ b/drivers/net/ethernet/emulex/benet/be_main.c
> > @@ -855,6 +855,13 @@ static struct sk_buff *be_xmit_workarounds(struct be_adapter
> *adapter,
> >  	unsigned int eth_hdr_len;
> >  	struct iphdr *ip;
> >
> > +	/* Don't allow non-TSO packets longer than MTU */
> > +	eth_hdr_len = (ntohs(skb->protocol) == ETH_P_8021Q) ?
> > +			VLAN_ETH_HLEN : ETH_HLEN;
> > +	if (!skb_is_gso(skb) &&
> > +	    (skb->len - eth_hdr_len) > adapter->netdev->mtu)
> > +			goto tx_drop;
> > +
> 
> When you say 'cannot handle them', is it some kind of nasty thing like
> hang / crash ?
This chip goes into an unrecoverable error state (needing a server reboot to rectify.)

> 
> One could imagine gso_size + sizeof(headers) > mtu, and give the same
> problem ?
Won't the mss (gso_size) value for TCP pkts be derived from the mtu of the out-going interface?
In that case when will gso_size + headers > mtu?
In any case, for TSO pkts, the HW just drops pkts when frames after segmentation
are longer than 9018bytes (or in recent FW versions 9200b)

> 
> 
> AFAIK, we have no check in net/core/dev.c.
> 
> Maybe we should instead add them there (and in pktgen)
I agree; doing this will solve issues with any other drivers/devices if they have a similar problem.

thanks,
-Sathya

RE: [PATCH 2/2] be2net: drop non-tso frames longer than mtu

[Eric Dumazet]

On Wed, 2013-10-16 at 12:08 +0000, Sathya Perla wrote:

> Won't the mss (gso_size) value for TCP pkts be derived from the mtu of the out-going interface?
> In that case when will gso_size + headers > mtu?


Not on forwarding :(

Let see we have two ethernet devices, with different mtu : 1500 and 1400

GRO could aggregate GRO packets with MSS=1460 (gso_size=1460), then
we hapilly send this GSO packet to the other device, and apparently we
have no check against the lower mtu.

In the best case GRO packet is lost, but in some other cases, the device
hangs...

> In any case, for TSO pkts, the HW just drops pkts when frames after segmentation
> are longer than 9018bytes (or in recent FW versions 9200b)