* [PATCH net 1/1] driver: veth: Return the actual value instead return NETDEV_TX_OK always
@ 2016-11-02 9:59 fgao
2016-11-02 20:22 ` Cong Wang
0 siblings, 1 reply; 5+ messages in thread
From: fgao @ 2016-11-02 9:59 UTC (permalink / raw)
To: davem, cwang, vijayp, ej, pabeni, netdev; +Cc: gfree.wind, Gao Feng
From: Gao Feng <fgao@ikuai8.com>
Current veth_xmit always returns NETDEV_TX_OK whatever if it is really
sent successfully. Now return the actual value instead of NETDEV_TX_OK
always.
Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
drivers/net/veth.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index fbc853e..769a3bd 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -111,15 +111,18 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
struct veth_priv *priv = netdev_priv(dev);
struct net_device *rcv;
int length = skb->len;
+ int ret = NETDEV_TX_OK;
rcu_read_lock();
rcv = rcu_dereference(priv->peer);
if (unlikely(!rcv)) {
kfree_skb(skb);
+ ret = NET_RX_DROP;
goto drop;
}
- if (likely(dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
+ ret = dev_forward_skb(rcv, skb);
+ if (likely(ret == NET_RX_SUCCESS)) {
struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
u64_stats_update_begin(&stats->syncp);
@@ -131,7 +134,7 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
atomic64_inc(&priv->dropped);
}
rcu_read_unlock();
- return NETDEV_TX_OK;
+ return ret;
}
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net 1/1] driver: veth: Return the actual value instead return NETDEV_TX_OK always
2016-11-02 9:59 [PATCH net 1/1] driver: veth: Return the actual value instead return NETDEV_TX_OK always fgao
@ 2016-11-02 20:22 ` Cong Wang
2016-11-03 0:52 ` Gao Feng
0 siblings, 1 reply; 5+ messages in thread
From: Cong Wang @ 2016-11-02 20:22 UTC (permalink / raw)
To: fgao
Cc: David Miller, Cong Wang, Vijay Pandurangan, Evan Jones, pabeni,
Linux Kernel Network Developers, gfree.wind
On Wed, Nov 2, 2016 at 2:59 AM, <fgao@ikuai8.com> wrote:
> From: Gao Feng <fgao@ikuai8.com>
>
> Current veth_xmit always returns NETDEV_TX_OK whatever if it is really
> sent successfully. Now return the actual value instead of NETDEV_TX_OK
> always.
>
> Signed-off-by: Gao Feng <fgao@ikuai8.com>
> ---
> drivers/net/veth.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index fbc853e..769a3bd 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -111,15 +111,18 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
> struct veth_priv *priv = netdev_priv(dev);
> struct net_device *rcv;
> int length = skb->len;
> + int ret = NETDEV_TX_OK;
>
> rcu_read_lock();
> rcv = rcu_dereference(priv->peer);
> if (unlikely(!rcv)) {
> kfree_skb(skb);
> + ret = NET_RX_DROP;
Returning NET_RX_DROP doesn't look correct in a xmit function.
> goto drop;
> }
>
> - if (likely(dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
> + ret = dev_forward_skb(rcv, skb);
> + if (likely(ret == NET_RX_SUCCESS)) {
> struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
>
> u64_stats_update_begin(&stats->syncp);
> @@ -131,7 +134,7 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
> atomic64_inc(&priv->dropped);
> }
> rcu_read_unlock();
> - return NETDEV_TX_OK;
> + return ret;
> }
>
> /*
> --
> 1.9.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net 1/1] driver: veth: Return the actual value instead return NETDEV_TX_OK always
2016-11-02 20:22 ` Cong Wang
@ 2016-11-03 0:52 ` Gao Feng
2016-11-03 0:58 ` Florian Fainelli
0 siblings, 1 reply; 5+ messages in thread
From: Gao Feng @ 2016-11-03 0:52 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Cong Wang, Vijay Pandurangan, Evan Jones, pabeni,
Linux Kernel Network Developers
Hi Cong,
On Thu, Nov 3, 2016 at 4:22 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Wed, Nov 2, 2016 at 2:59 AM, <fgao@ikuai8.com> wrote:
>> From: Gao Feng <fgao@ikuai8.com>
>>
>> Current veth_xmit always returns NETDEV_TX_OK whatever if it is really
>> sent successfully. Now return the actual value instead of NETDEV_TX_OK
>> always.
>>
>> Signed-off-by: Gao Feng <fgao@ikuai8.com>
>> ---
>> drivers/net/veth.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
>> index fbc853e..769a3bd 100644
>> --- a/drivers/net/veth.c
>> +++ b/drivers/net/veth.c
>> @@ -111,15 +111,18 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
>> struct veth_priv *priv = netdev_priv(dev);
>> struct net_device *rcv;
>> int length = skb->len;
>> + int ret = NETDEV_TX_OK;
>>
>> rcu_read_lock();
>> rcv = rcu_dereference(priv->peer);
>> if (unlikely(!rcv)) {
>> kfree_skb(skb);
>> + ret = NET_RX_DROP;
>
>
> Returning NET_RX_DROP doesn't look correct in a xmit function.
Yes. But I don't find good macro.
NETDEV_TX_BUSY or NET_RX_DROP, which is better ?
Thanks
Feng
>
>
>> goto drop;
>> }
>>
>> - if (likely(dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
>> + ret = dev_forward_skb(rcv, skb);
>> + if (likely(ret == NET_RX_SUCCESS)) {
>> struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
>>
>> u64_stats_update_begin(&stats->syncp);
>> @@ -131,7 +134,7 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
>> atomic64_inc(&priv->dropped);
>> }
>> rcu_read_unlock();
>> - return NETDEV_TX_OK;
>> + return ret;
>> }
>>
>> /*
>> --
>> 1.9.1
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net 1/1] driver: veth: Return the actual value instead return NETDEV_TX_OK always
2016-11-03 0:52 ` Gao Feng
@ 2016-11-03 0:58 ` Florian Fainelli
2016-11-03 1:04 ` Gao Feng
0 siblings, 1 reply; 5+ messages in thread
From: Florian Fainelli @ 2016-11-03 0:58 UTC (permalink / raw)
To: Gao Feng, Cong Wang
Cc: David Miller, Cong Wang, Vijay Pandurangan, Evan Jones, pabeni,
Linux Kernel Network Developers
On 11/02/2016 05:52 PM, Gao Feng wrote:
> Hi Cong,
>
> On Thu, Nov 3, 2016 at 4:22 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Wed, Nov 2, 2016 at 2:59 AM, <fgao@ikuai8.com> wrote:
>>> From: Gao Feng <fgao@ikuai8.com>
>>>
>>> Current veth_xmit always returns NETDEV_TX_OK whatever if it is really
>>> sent successfully. Now return the actual value instead of NETDEV_TX_OK
>>> always.
>>>
>>> Signed-off-by: Gao Feng <fgao@ikuai8.com>
>>> ---
>>> drivers/net/veth.c | 7 +++++--
>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
>>> index fbc853e..769a3bd 100644
>>> --- a/drivers/net/veth.c
>>> +++ b/drivers/net/veth.c
>>> @@ -111,15 +111,18 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
>>> struct veth_priv *priv = netdev_priv(dev);
>>> struct net_device *rcv;
>>> int length = skb->len;
>>> + int ret = NETDEV_TX_OK;
>>>
>>> rcu_read_lock();
>>> rcv = rcu_dereference(priv->peer);
>>> if (unlikely(!rcv)) {
>>> kfree_skb(skb);
>>> + ret = NET_RX_DROP;
>>
>>
>> Returning NET_RX_DROP doesn't look correct in a xmit function.
>
> Yes. But I don't find good macro.
> NETDEV_TX_BUSY or NET_RX_DROP, which is better ?
There is no much choice you need to return a correct value from the
netdev_tx_t enum, which NET_RX_DROP is not part of, so that probably
means using NETDEV_TX_OK here, the packet has been freed, and there is
no flow control problem mandating the return of NETDEV_TX_BUSY it seems...
--
Florian
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net 1/1] driver: veth: Return the actual value instead return NETDEV_TX_OK always
2016-11-03 0:58 ` Florian Fainelli
@ 2016-11-03 1:04 ` Gao Feng
0 siblings, 0 replies; 5+ messages in thread
From: Gao Feng @ 2016-11-03 1:04 UTC (permalink / raw)
To: Florian Fainelli
Cc: Cong Wang, David Miller, Cong Wang, Vijay Pandurangan, Evan Jones,
pabeni, Linux Kernel Network Developers
Hi Florian,
On Thu, Nov 3, 2016 at 8:58 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 11/02/2016 05:52 PM, Gao Feng wrote:
>> Hi Cong,
>>
>> On Thu, Nov 3, 2016 at 4:22 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> On Wed, Nov 2, 2016 at 2:59 AM, <fgao@ikuai8.com> wrote:
>>>> From: Gao Feng <fgao@ikuai8.com>
>>>>
>>>> Current veth_xmit always returns NETDEV_TX_OK whatever if it is really
>>>> sent successfully. Now return the actual value instead of NETDEV_TX_OK
>>>> always.
>>>>
>>>> Signed-off-by: Gao Feng <fgao@ikuai8.com>
>>>> ---
>>>> drivers/net/veth.c | 7 +++++--
>>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
>>>> index fbc853e..769a3bd 100644
>>>> --- a/drivers/net/veth.c
>>>> +++ b/drivers/net/veth.c
>>>> @@ -111,15 +111,18 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
>>>> struct veth_priv *priv = netdev_priv(dev);
>>>> struct net_device *rcv;
>>>> int length = skb->len;
>>>> + int ret = NETDEV_TX_OK;
>>>>
>>>> rcu_read_lock();
>>>> rcv = rcu_dereference(priv->peer);
>>>> if (unlikely(!rcv)) {
>>>> kfree_skb(skb);
>>>> + ret = NET_RX_DROP;
>>>
>>>
>>> Returning NET_RX_DROP doesn't look correct in a xmit function.
>>
>> Yes. But I don't find good macro.
>> NETDEV_TX_BUSY or NET_RX_DROP, which is better ?
>
> There is no much choice you need to return a correct value from the
> netdev_tx_t enum, which NET_RX_DROP is not part of, so that probably
> means using NETDEV_TX_OK here, the packet has been freed, and there is
> no flow control problem mandating the return of NETDEV_TX_BUSY it seems...
> --
> Florian
Thanks your explanation.
It means the veth_xmit must return NETDEV_TX_OK.
Regards
Feng
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-11-03 1:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-02 9:59 [PATCH net 1/1] driver: veth: Return the actual value instead return NETDEV_TX_OK always fgao
2016-11-02 20:22 ` Cong Wang
2016-11-03 0:52 ` Gao Feng
2016-11-03 0:58 ` Florian Fainelli
2016-11-03 1:04 ` Gao Feng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox