Re: [PATCH v6 4/4] drivers: net: Add APM X-Gene SoC ethernet driver support.
On Tue, Jun 17, 2014 at 1:20 PM, David Miller wrote:
> From: Iyappan Subramanian
> Date: Tue, 17 Jun 2014 12:21:08 -0700
>
>> On Mon, Jun 16, 2014 at 9:42 PM, David Miller wrote:
>>> From: Iyappan Subramanian
>>> Date: Mon, 16 Jun 2014 17:18:46 -0700
>>>
+static netdev_tx_t xgene_enet_start_xmit(struct sk_buff *skb,
+ struct net_device *ndev)
+{
+ struct xgene_enet_pdata *pdata = netdev_priv(ndev);
+ struct xgene_enet_desc_ring *tx_ring = pdata->tx_ring;
+ struct xgene_enet_desc_ring *cp_ring = tx_ring->cp_ring;
+ u32 tx_level, cq_level;
+
+ tx_level = xgene_enet_ring_len(tx_ring);
+ cq_level = xgene_enet_ring_len(cp_ring);
+ if (unlikely(tx_level > pdata->tx_qcnt_hi ||
+ cq_level > pdata->cp_qcnt_hi)) {
+ netif_stop_queue(ndev);
+ return NETDEV_TX_BUSY;
+ }
+
+ if (xgene_enet_setup_tx_desc(tx_ring, skb))
+ return NETDEV_TX_OK;
>>>
>>> If you return NETDEV_TX_OK, it is your responsibility to deal with the
>>> SKB, because you own it.
>>>
>>> In particular, you have to free the packet.
>>
>> skb freeing happens in xgene_enet_tx_completion().
>
> You're not queueing up the packet when DMA mapping the packet fails,
> which is the only condition that causes xgene_enet_setup_tx_desc() to
> return an error, so xgene_enet_tx_completion() will not be called.
>
> Can you really not see this?
I am sorry I missed that. I will fix this.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/4] drivers: net: Add APM X-Gene SoC ethernet driver support.
On Tue, Jun 17, 2014 at 1:20 PM, David Miller da...@davemloft.net wrote:
From: Iyappan Subramanian isubraman...@apm.com
Date: Tue, 17 Jun 2014 12:21:08 -0700
On Mon, Jun 16, 2014 at 9:42 PM, David Miller da...@davemloft.net wrote:
From: Iyappan Subramanian isubraman...@apm.com
Date: Mon, 16 Jun 2014 17:18:46 -0700
+static netdev_tx_t xgene_enet_start_xmit(struct sk_buff *skb,
+ struct net_device *ndev)
+{
+ struct xgene_enet_pdata *pdata = netdev_priv(ndev);
+ struct xgene_enet_desc_ring *tx_ring = pdata-tx_ring;
+ struct xgene_enet_desc_ring *cp_ring = tx_ring-cp_ring;
+ u32 tx_level, cq_level;
+
+ tx_level = xgene_enet_ring_len(tx_ring);
+ cq_level = xgene_enet_ring_len(cp_ring);
+ if (unlikely(tx_level pdata-tx_qcnt_hi ||
+ cq_level pdata-cp_qcnt_hi)) {
+ netif_stop_queue(ndev);
+ return NETDEV_TX_BUSY;
+ }
+
+ if (xgene_enet_setup_tx_desc(tx_ring, skb))
+ return NETDEV_TX_OK;
If you return NETDEV_TX_OK, it is your responsibility to deal with the
SKB, because you own it.
In particular, you have to free the packet.
skb freeing happens in xgene_enet_tx_completion().
You're not queueing up the packet when DMA mapping the packet fails,
which is the only condition that causes xgene_enet_setup_tx_desc() to
return an error, so xgene_enet_tx_completion() will not be called.
Can you really not see this?
I am sorry I missed that. I will fix this.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/4] drivers: net: Add APM X-Gene SoC ethernet driver support.
From: Iyappan Subramanian
Date: Tue, 17 Jun 2014 12:21:08 -0700
> On Mon, Jun 16, 2014 at 9:42 PM, David Miller wrote:
>> From: Iyappan Subramanian
>> Date: Mon, 16 Jun 2014 17:18:46 -0700
>>
>>> +static netdev_tx_t xgene_enet_start_xmit(struct sk_buff *skb,
>>> + struct net_device *ndev)
>>> +{
>>> + struct xgene_enet_pdata *pdata = netdev_priv(ndev);
>>> + struct xgene_enet_desc_ring *tx_ring = pdata->tx_ring;
>>> + struct xgene_enet_desc_ring *cp_ring = tx_ring->cp_ring;
>>> + u32 tx_level, cq_level;
>>> +
>>> + tx_level = xgene_enet_ring_len(tx_ring);
>>> + cq_level = xgene_enet_ring_len(cp_ring);
>>> + if (unlikely(tx_level > pdata->tx_qcnt_hi ||
>>> + cq_level > pdata->cp_qcnt_hi)) {
>>> + netif_stop_queue(ndev);
>>> + return NETDEV_TX_BUSY;
>>> + }
>>> +
>>> + if (xgene_enet_setup_tx_desc(tx_ring, skb))
>>> + return NETDEV_TX_OK;
>>
>> If you return NETDEV_TX_OK, it is your responsibility to deal with the
>> SKB, because you own it.
>>
>> In particular, you have to free the packet.
>
> skb freeing happens in xgene_enet_tx_completion().
You're not queueing up the packet when DMA mapping the packet fails,
which is the only condition that causes xgene_enet_setup_tx_desc() to
return an error, so xgene_enet_tx_completion() will not be called.
Can you really not see this?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/4] drivers: net: Add APM X-Gene SoC ethernet driver support.
On Tue, Jun 17, 2014 at 12:24 PM, Florian Fainelli wrote:
> 2014-06-17 12:21 GMT-07:00 Iyappan Subramanian :
>> On Mon, Jun 16, 2014 at 9:42 PM, David Miller wrote:
>>> From: Iyappan Subramanian
>>> Date: Mon, 16 Jun 2014 17:18:46 -0700
>>>
+static netdev_tx_t xgene_enet_start_xmit(struct sk_buff *skb,
+ struct net_device *ndev)
+{
+ struct xgene_enet_pdata *pdata = netdev_priv(ndev);
+ struct xgene_enet_desc_ring *tx_ring = pdata->tx_ring;
+ struct xgene_enet_desc_ring *cp_ring = tx_ring->cp_ring;
+ u32 tx_level, cq_level;
+
+ tx_level = xgene_enet_ring_len(tx_ring);
+ cq_level = xgene_enet_ring_len(cp_ring);
+ if (unlikely(tx_level > pdata->tx_qcnt_hi ||
+ cq_level > pdata->cp_qcnt_hi)) {
+ netif_stop_queue(ndev);
+ return NETDEV_TX_BUSY;
+ }
+
+ if (xgene_enet_setup_tx_desc(tx_ring, skb))
+ return NETDEV_TX_OK;
>>>
>>> If you return NETDEV_TX_OK, it is your responsibility to deal with the
>>> SKB, because you own it.
>>>
>>> In particular, you have to free the packet.
>>
>> skb freeing happens in xgene_enet_tx_completion().
>
> I have not looked at what xgenet_enet_setup_tx_desc() does, but
> David's point is that, if this function fails and has not yet been
> able to queue the SKB for transmission at the HW, you should make sure
> that you free the SKB. Is xgene_enet_tx_completion() called if the
> driver failed to prepare a descriptor for a SKB?
Thanks for the explanation. I will free up skb when
xgene_enet_setup_tx_desc() fails
and submit the next patch shortly.
> --
> Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/4] drivers: net: Add APM X-Gene SoC ethernet driver support.
2014-06-17 12:21 GMT-07:00 Iyappan Subramanian :
> On Mon, Jun 16, 2014 at 9:42 PM, David Miller wrote:
>> From: Iyappan Subramanian
>> Date: Mon, 16 Jun 2014 17:18:46 -0700
>>
>>> +static netdev_tx_t xgene_enet_start_xmit(struct sk_buff *skb,
>>> + struct net_device *ndev)
>>> +{
>>> + struct xgene_enet_pdata *pdata = netdev_priv(ndev);
>>> + struct xgene_enet_desc_ring *tx_ring = pdata->tx_ring;
>>> + struct xgene_enet_desc_ring *cp_ring = tx_ring->cp_ring;
>>> + u32 tx_level, cq_level;
>>> +
>>> + tx_level = xgene_enet_ring_len(tx_ring);
>>> + cq_level = xgene_enet_ring_len(cp_ring);
>>> + if (unlikely(tx_level > pdata->tx_qcnt_hi ||
>>> + cq_level > pdata->cp_qcnt_hi)) {
>>> + netif_stop_queue(ndev);
>>> + return NETDEV_TX_BUSY;
>>> + }
>>> +
>>> + if (xgene_enet_setup_tx_desc(tx_ring, skb))
>>> + return NETDEV_TX_OK;
>>
>> If you return NETDEV_TX_OK, it is your responsibility to deal with the
>> SKB, because you own it.
>>
>> In particular, you have to free the packet.
>
> skb freeing happens in xgene_enet_tx_completion().
I have not looked at what xgenet_enet_setup_tx_desc() does, but
David's point is that, if this function fails and has not yet been
able to queue the SKB for transmission at the HW, you should make sure
that you free the SKB. Is xgene_enet_tx_completion() called if the
driver failed to prepare a descriptor for a SKB?
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/4] drivers: net: Add APM X-Gene SoC ethernet driver support.
On Mon, Jun 16, 2014 at 9:42 PM, David Miller wrote:
> From: Iyappan Subramanian
> Date: Mon, 16 Jun 2014 17:18:46 -0700
>
>> +static netdev_tx_t xgene_enet_start_xmit(struct sk_buff *skb,
>> + struct net_device *ndev)
>> +{
>> + struct xgene_enet_pdata *pdata = netdev_priv(ndev);
>> + struct xgene_enet_desc_ring *tx_ring = pdata->tx_ring;
>> + struct xgene_enet_desc_ring *cp_ring = tx_ring->cp_ring;
>> + u32 tx_level, cq_level;
>> +
>> + tx_level = xgene_enet_ring_len(tx_ring);
>> + cq_level = xgene_enet_ring_len(cp_ring);
>> + if (unlikely(tx_level > pdata->tx_qcnt_hi ||
>> + cq_level > pdata->cp_qcnt_hi)) {
>> + netif_stop_queue(ndev);
>> + return NETDEV_TX_BUSY;
>> + }
>> +
>> + if (xgene_enet_setup_tx_desc(tx_ring, skb))
>> + return NETDEV_TX_OK;
>
> If you return NETDEV_TX_OK, it is your responsibility to deal with the
> SKB, because you own it.
>
> In particular, you have to free the packet.
skb freeing happens in xgene_enet_tx_completion().
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/4] drivers: net: Add APM X-Gene SoC ethernet driver support.
On Mon, Jun 16, 2014 at 9:42 PM, David Miller da...@davemloft.net wrote:
From: Iyappan Subramanian isubraman...@apm.com
Date: Mon, 16 Jun 2014 17:18:46 -0700
+static netdev_tx_t xgene_enet_start_xmit(struct sk_buff *skb,
+ struct net_device *ndev)
+{
+ struct xgene_enet_pdata *pdata = netdev_priv(ndev);
+ struct xgene_enet_desc_ring *tx_ring = pdata-tx_ring;
+ struct xgene_enet_desc_ring *cp_ring = tx_ring-cp_ring;
+ u32 tx_level, cq_level;
+
+ tx_level = xgene_enet_ring_len(tx_ring);
+ cq_level = xgene_enet_ring_len(cp_ring);
+ if (unlikely(tx_level pdata-tx_qcnt_hi ||
+ cq_level pdata-cp_qcnt_hi)) {
+ netif_stop_queue(ndev);
+ return NETDEV_TX_BUSY;
+ }
+
+ if (xgene_enet_setup_tx_desc(tx_ring, skb))
+ return NETDEV_TX_OK;
If you return NETDEV_TX_OK, it is your responsibility to deal with the
SKB, because you own it.
In particular, you have to free the packet.
skb freeing happens in xgene_enet_tx_completion().
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/4] drivers: net: Add APM X-Gene SoC ethernet driver support.
2014-06-17 12:21 GMT-07:00 Iyappan Subramanian isubraman...@apm.com:
On Mon, Jun 16, 2014 at 9:42 PM, David Miller da...@davemloft.net wrote:
From: Iyappan Subramanian isubraman...@apm.com
Date: Mon, 16 Jun 2014 17:18:46 -0700
+static netdev_tx_t xgene_enet_start_xmit(struct sk_buff *skb,
+ struct net_device *ndev)
+{
+ struct xgene_enet_pdata *pdata = netdev_priv(ndev);
+ struct xgene_enet_desc_ring *tx_ring = pdata-tx_ring;
+ struct xgene_enet_desc_ring *cp_ring = tx_ring-cp_ring;
+ u32 tx_level, cq_level;
+
+ tx_level = xgene_enet_ring_len(tx_ring);
+ cq_level = xgene_enet_ring_len(cp_ring);
+ if (unlikely(tx_level pdata-tx_qcnt_hi ||
+ cq_level pdata-cp_qcnt_hi)) {
+ netif_stop_queue(ndev);
+ return NETDEV_TX_BUSY;
+ }
+
+ if (xgene_enet_setup_tx_desc(tx_ring, skb))
+ return NETDEV_TX_OK;
If you return NETDEV_TX_OK, it is your responsibility to deal with the
SKB, because you own it.
In particular, you have to free the packet.
skb freeing happens in xgene_enet_tx_completion().
I have not looked at what xgenet_enet_setup_tx_desc() does, but
David's point is that, if this function fails and has not yet been
able to queue the SKB for transmission at the HW, you should make sure
that you free the SKB. Is xgene_enet_tx_completion() called if the
driver failed to prepare a descriptor for a SKB?
--
Florian
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/4] drivers: net: Add APM X-Gene SoC ethernet driver support.
On Tue, Jun 17, 2014 at 12:24 PM, Florian Fainelli f.faine...@gmail.com wrote:
2014-06-17 12:21 GMT-07:00 Iyappan Subramanian isubraman...@apm.com:
On Mon, Jun 16, 2014 at 9:42 PM, David Miller da...@davemloft.net wrote:
From: Iyappan Subramanian isubraman...@apm.com
Date: Mon, 16 Jun 2014 17:18:46 -0700
+static netdev_tx_t xgene_enet_start_xmit(struct sk_buff *skb,
+ struct net_device *ndev)
+{
+ struct xgene_enet_pdata *pdata = netdev_priv(ndev);
+ struct xgene_enet_desc_ring *tx_ring = pdata-tx_ring;
+ struct xgene_enet_desc_ring *cp_ring = tx_ring-cp_ring;
+ u32 tx_level, cq_level;
+
+ tx_level = xgene_enet_ring_len(tx_ring);
+ cq_level = xgene_enet_ring_len(cp_ring);
+ if (unlikely(tx_level pdata-tx_qcnt_hi ||
+ cq_level pdata-cp_qcnt_hi)) {
+ netif_stop_queue(ndev);
+ return NETDEV_TX_BUSY;
+ }
+
+ if (xgene_enet_setup_tx_desc(tx_ring, skb))
+ return NETDEV_TX_OK;
If you return NETDEV_TX_OK, it is your responsibility to deal with the
SKB, because you own it.
In particular, you have to free the packet.
skb freeing happens in xgene_enet_tx_completion().
I have not looked at what xgenet_enet_setup_tx_desc() does, but
David's point is that, if this function fails and has not yet been
able to queue the SKB for transmission at the HW, you should make sure
that you free the SKB. Is xgene_enet_tx_completion() called if the
driver failed to prepare a descriptor for a SKB?
Thanks for the explanation. I will free up skb when
xgene_enet_setup_tx_desc() fails
and submit the next patch shortly.
--
Florian
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/4] drivers: net: Add APM X-Gene SoC ethernet driver support.
From: Iyappan Subramanian isubraman...@apm.com
Date: Tue, 17 Jun 2014 12:21:08 -0700
On Mon, Jun 16, 2014 at 9:42 PM, David Miller da...@davemloft.net wrote:
From: Iyappan Subramanian isubraman...@apm.com
Date: Mon, 16 Jun 2014 17:18:46 -0700
+static netdev_tx_t xgene_enet_start_xmit(struct sk_buff *skb,
+ struct net_device *ndev)
+{
+ struct xgene_enet_pdata *pdata = netdev_priv(ndev);
+ struct xgene_enet_desc_ring *tx_ring = pdata-tx_ring;
+ struct xgene_enet_desc_ring *cp_ring = tx_ring-cp_ring;
+ u32 tx_level, cq_level;
+
+ tx_level = xgene_enet_ring_len(tx_ring);
+ cq_level = xgene_enet_ring_len(cp_ring);
+ if (unlikely(tx_level pdata-tx_qcnt_hi ||
+ cq_level pdata-cp_qcnt_hi)) {
+ netif_stop_queue(ndev);
+ return NETDEV_TX_BUSY;
+ }
+
+ if (xgene_enet_setup_tx_desc(tx_ring, skb))
+ return NETDEV_TX_OK;
If you return NETDEV_TX_OK, it is your responsibility to deal with the
SKB, because you own it.
In particular, you have to free the packet.
skb freeing happens in xgene_enet_tx_completion().
You're not queueing up the packet when DMA mapping the packet fails,
which is the only condition that causes xgene_enet_setup_tx_desc() to
return an error, so xgene_enet_tx_completion() will not be called.
Can you really not see this?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/4] drivers: net: Add APM X-Gene SoC ethernet driver support.
From: Iyappan Subramanian
Date: Mon, 16 Jun 2014 17:18:46 -0700
> +static netdev_tx_t xgene_enet_start_xmit(struct sk_buff *skb,
> + struct net_device *ndev)
> +{
> + struct xgene_enet_pdata *pdata = netdev_priv(ndev);
> + struct xgene_enet_desc_ring *tx_ring = pdata->tx_ring;
> + struct xgene_enet_desc_ring *cp_ring = tx_ring->cp_ring;
> + u32 tx_level, cq_level;
> +
> + tx_level = xgene_enet_ring_len(tx_ring);
> + cq_level = xgene_enet_ring_len(cp_ring);
> + if (unlikely(tx_level > pdata->tx_qcnt_hi ||
> + cq_level > pdata->cp_qcnt_hi)) {
> + netif_stop_queue(ndev);
> + return NETDEV_TX_BUSY;
> + }
> +
> + if (xgene_enet_setup_tx_desc(tx_ring, skb))
> + return NETDEV_TX_OK;
If you return NETDEV_TX_OK, it is your responsibility to deal with the
SKB, because you own it.
In particular, you have to free the packet.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/4] drivers: net: Add APM X-Gene SoC ethernet driver support.
From: Iyappan Subramanian isubraman...@apm.com
Date: Mon, 16 Jun 2014 17:18:46 -0700
+static netdev_tx_t xgene_enet_start_xmit(struct sk_buff *skb,
+ struct net_device *ndev)
+{
+ struct xgene_enet_pdata *pdata = netdev_priv(ndev);
+ struct xgene_enet_desc_ring *tx_ring = pdata-tx_ring;
+ struct xgene_enet_desc_ring *cp_ring = tx_ring-cp_ring;
+ u32 tx_level, cq_level;
+
+ tx_level = xgene_enet_ring_len(tx_ring);
+ cq_level = xgene_enet_ring_len(cp_ring);
+ if (unlikely(tx_level pdata-tx_qcnt_hi ||
+ cq_level pdata-cp_qcnt_hi)) {
+ netif_stop_queue(ndev);
+ return NETDEV_TX_BUSY;
+ }
+
+ if (xgene_enet_setup_tx_desc(tx_ring, skb))
+ return NETDEV_TX_OK;
If you return NETDEV_TX_OK, it is your responsibility to deal with the
SKB, because you own it.
In particular, you have to free the packet.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
