[PATCH v3 2/5] staging: et131x: drop packet when error occurs in et131x_tx

ZHAO Gang gamerh2o at gmail.com
Fri Nov 22 11:47:12 UTC 2013


On Fri, Nov 22, 2013 at 5:17 PM, Denis Kirjanov <kirjanov at gmail.com> wrote:
> On 11/22/13, Mark Einon <mark.einon at gmail.com> wrote:
>> On Wed, Nov 20, 2013 at 03:55:27PM +0800, ZHAO Gang wrote:
>>> As TODO file suggested, drop packet instead of return NETDEV_TX_BUSY
>>> when tx failed.
>>>
>>> et131x_tx calls function et131x_send_packets, I put the work of
>>> et131x_send_packets directly into et131x_tx, and made some changes to
>>> let the code more readable.
>>>
>>> Signed-off-by: ZHAO Gang <gamerh2o at gmail.com>
>>
>> Acked-by: Mark Einon <mark.einon at gmail.com>
>>
>>> ---
>>>  drivers/staging/et131x/et131x.c | 84
>>> +++++++++++------------------------------
>>>  1 file changed, 22 insertions(+), 62 deletions(-)
>>>
>>> diff --git a/drivers/staging/et131x/et131x.c
>>> b/drivers/staging/et131x/et131x.c
>>> index 836a4db..cda037a 100644
>>> --- a/drivers/staging/et131x/et131x.c
>>> +++ b/drivers/staging/et131x/et131x.c
>>> @@ -3123,55 +3123,6 @@ static int send_packet(struct sk_buff *skb, struct
>>> et131x_adapter *adapter)
>>>      return 0;
>>>  }
>>>
>>> -/* et131x_send_packets - This function is called by the OS to send
>>> packets
>>> - * @skb: the packet(s) to send
>>> - * @netdev:device on which to TX the above packet(s)
>>> - *
>>> - * Return 0 in almost all cases; non-zero value in extreme hard failure
>>> only
>>> - */
>>> -static int et131x_send_packets(struct sk_buff *skb, struct net_device
>>> *netdev)
>>> -{
>>> -    int status = 0;
>>> -    struct et131x_adapter *adapter = netdev_priv(netdev);
>>> -
>>> -    /* Send these packets
>>> -     *
>>> -     * NOTE: The Linux Tx entry point is only given one packet at a time
>>> -     * to Tx, so the PacketCount and it's array used makes no sense here
>>> -     */
>>> -
>>> -    /* TCB is not available */
>>> -    if (adapter->tx_ring.used >= NUM_TCB) {
>>> -            /* NOTE: If there's an error on send, no need to queue the
>>> -             * packet under Linux; if we just send an error up to the
>>> -             * netif layer, it will resend the skb to us.
>>> -             */
>>> -            status = -ENOMEM;
>>> -    } else {
>>> -            /* We need to see if the link is up; if it's not, make the
>>> -             * netif layer think we're good and drop the packet
>>> -             */
>>> -            if ((adapter->flags & FMP_ADAPTER_FAIL_SEND_MASK) ||
>>> -                                    !netif_carrier_ok(netdev)) {
>>> -                    dev_kfree_skb_any(skb);
>>> -                    skb = NULL;
>>> -
>>> -                    adapter->net_stats.tx_dropped++;
>>> -            } else {
>>> -                    status = send_packet(skb, adapter);
>>> -                    if (status != 0 && status != -ENOMEM) {
>>> -                            /* On any other error, make netif think we're
>>> -                             * OK and drop the packet
>>> -                             */
>>> -                            dev_kfree_skb_any(skb);
>>> -                            skb = NULL;
>>> -                            adapter->net_stats.tx_dropped++;
>>> -                    }
>>> -            }
>>> -    }
>>> -    return status;
>>> -}
>>> -
>>>  /* free_send_packet - Recycle a struct tcb
>>>   * @adapter: pointer to our adapter
>>>   * @tcb: pointer to struct tcb
>>> @@ -4537,12 +4488,9 @@ static void et131x_multicast(struct net_device
>>> *netdev)
>>>  /* et131x_tx - The handler to tx a packet on the device
>>>   * @skb: data to be Tx'd
>>>   * @netdev: device on which data is to be Tx'd
>>> - *
>>> - * Returns 0 on success, errno on failure (as defined in errno.h)
>>>   */
>>> -static int et131x_tx(struct sk_buff *skb, struct net_device *netdev)
>>> +static netdev_tx_t et131x_tx(struct sk_buff *skb, struct net_device
>>> *netdev)
>>>  {
>>> -    int status = 0;
>>>      struct et131x_adapter *adapter = netdev_priv(netdev);
>>>
>>>      /* stop the queue if it's getting full */
>>> @@ -4553,17 +4501,29 @@ static int et131x_tx(struct sk_buff *skb, struct
>>> net_device *netdev)
>>>      /* Save the timestamp for the TX timeout watchdog */
>>>      netdev->trans_start = jiffies;
>>>
>>> -    /* Call the device-specific data Tx routine */
>>> -    status = et131x_send_packets(skb, netdev);
>>> +    /* TCB is not available */
>>> +    if (adapter->tx_ring.used >= NUM_TCB)
>>> +            goto drop;
>
> That's wrong. You have to properly manage tx queue and not just drop packets

Could you kindly tell me what should be done to tx queue?

>
>>> -    /* Check status and manage the netif queue if necessary */
>>> -    if (status != 0) {
>>> -            if (status == -ENOMEM)
>>> -                    status = NETDEV_TX_BUSY;
>>> -            else
>>> -                    status = NETDEV_TX_OK;
>>> +    /* We need to see if the link is up; if it's not, make the
>>> +     * netif layer think we're good and drop the packet
>>> +     */
>>> +    if ((adapter->flags & FMP_ADAPTER_FAIL_SEND_MASK) ||
>>> +        !netif_carrier_ok(netdev)) {
>>> +            goto drop;
>>> +    } else {
>>> +            if (!send_packet(skb, adapter))
>>> +                    goto out;
>>> +            /* On error (send_packet returns != 0), make netif
>>> +             * think we're OK and drop the packet
>>> +             */
>>>      }
>>> -    return status;
>>> +
>>> +drop:
>>> +    dev_kfree_skb_any(skb);
>>> +    adapter->net_stats.tx_dropped++;
>>> +out:
>>> +    return NETDEV_TX_OK;
>>>  }
>>>
>>>  /* et131x_tx_timeout - Timeout handler
>>> --
>>> 1.8.3.1
>>>
>> _______________________________________________
>> devel mailing list
>> devel at linuxdriverproject.org
>> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
>>
>
>
> --
> Regards,
> Denis


More information about the devel mailing list