[ovs-dev,RFC] stt: fix return code during xmit
diff mbox series

Message ID f7td0owmvx2.fsf@dhcp-25.97.bos.redhat.com
State Accepted
Headers show
Series
  • [ovs-dev,RFC] stt: fix return code during xmit
Related show

Commit Message

Aaron Conole Jan. 16, 2019, 4:03 p.m. UTC
Following code looks like it might be wrong.  I don't know much about
the way the stt infrastructure is being used, so feel free to ignore if
it is expected to return NETDEV_TX_OK even in error cases (just seems
strange).

Caught by compiler warning:

  /home/travis/build/ovsrobot/ovs/datapath/linux/stt.c: In function ‘ovs_stt_xmit’:
  /home/travis/build/ovsrobot/ovs/datapath/linux/stt.c:1005:6: warning: variable ‘err’ set but not used [-Wunused-but-set-variable]
    int err;
        ^

If not used, then consider alternatively just dropping the variable.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---

---

Comments

Gregory Rose Jan. 21, 2019, 6:23 p.m. UTC | #1
On 1/16/2019 8:03 AM, Aaron Conole wrote:
> Following code looks like it might be wrong.  I don't know much about
> the way the stt infrastructure is being used, so feel free to ignore if
> it is expected to return NETDEV_TX_OK even in error cases (just seems
> strange).
>
> Caught by compiler warning:
>
>    /home/travis/build/ovsrobot/ovs/datapath/linux/stt.c: In function ‘ovs_stt_xmit’:
>    /home/travis/build/ovsrobot/ovs/datapath/linux/stt.c:1005:6: warning: variable ‘err’ set but not used [-Wunused-but-set-variable]
>      int err;
>          ^
>
> If not used, then consider alternatively just dropping the variable.
>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>
> diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c
> index 506f1d90a..5f045120e 100644
> --- a/datapath/linux/compat/stt.c
> +++ b/datapath/linux/compat/stt.c
> @@ -1029,7 +1029,7 @@ netdev_tx_t ovs_stt_xmit(struct sk_buff *skb)
>   error:
>          kfree_skb(skb);
>          dev->stats.tx_errors++;
> -       return NETDEV_TX_OK;
> +       return err;
>   }
>   EXPORT_SYMBOL(ovs_stt_xmit);

Looks good to me.  I don't know if you need to resend without the RFC 
tag or not but in any case...

Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Justin Pettit Jan. 24, 2019, 6:41 p.m. UTC | #2
> On Jan 21, 2019, at 10:23 AM, Gregory Rose <gvrose8192@gmail.com> wrote:
> 
> On 1/16/2019 8:03 AM, Aaron Conole wrote:
>> Following code looks like it might be wrong.  I don't know much about
>> the way the stt infrastructure is being used, so feel free to ignore if
>> it is expected to return NETDEV_TX_OK even in error cases (just seems
>> strange).
>> 
>> Caught by compiler warning:
>> 
>>   /home/travis/build/ovsrobot/ovs/datapath/linux/stt.c: In function ‘ovs_stt_xmit’:
>>   /home/travis/build/ovsrobot/ovs/datapath/linux/stt.c:1005:6: warning: variable ‘err’ set but not used [-Wunused-but-set-variable]
>>     int err;
>>         ^
>> 
>> If not used, then consider alternatively just dropping the variable.
>> 
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>> 
>> diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c
>> index 506f1d90a..5f045120e 100644
>> --- a/datapath/linux/compat/stt.c
>> +++ b/datapath/linux/compat/stt.c
>> @@ -1029,7 +1029,7 @@ netdev_tx_t ovs_stt_xmit(struct sk_buff *skb)
>>  error:
>>         kfree_skb(skb);
>>         dev->stats.tx_errors++;
>> -       return NETDEV_TX_OK;
>> +       return err;
>>  }
>>  EXPORT_SYMBOL(ovs_stt_xmit);
> 
> Looks good to me.  I don't know if you need to resend without the RFC tag or not but in any case...
> 
> Reviewed-by: Greg Rose <gvrose8192@gmail.com>

Thanks, Aaron and Greg.  I updated the commit message to remove the doubts from it and pushed it all the way back to branch-2.5.

--Justin

Patch
diff mbox series

diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c
index 506f1d90a..5f045120e 100644
--- a/datapath/linux/compat/stt.c
+++ b/datapath/linux/compat/stt.c
@@ -1029,7 +1029,7 @@  netdev_tx_t ovs_stt_xmit(struct sk_buff *skb)
 error:
        kfree_skb(skb);
        dev->stats.tx_errors++;
-       return NETDEV_TX_OK;
+       return err;
 }
 EXPORT_SYMBOL(ovs_stt_xmit);