| Message ID | f7td0owmvx2.fsf@dhcp-25.97.bos.redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,RFC] stt: fix return code during xmit | expand |
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>
> 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
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);
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> --- ---