[net,1/2] openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info()
diff mbox series

Message ID a5a946ce525d00f927c010fca7da675ddc212c97.1574769406.git.pabeni@redhat.com
State Changes Requested
Delegated to: David Miller
Headers show
Series
  • openvswitch: remove a couple of BUG_ON()
Related show

Commit Message

Paolo Abeni Nov. 26, 2019, 12:10 p.m. UTC
All callers already deal with errors correctly, dump a warn instead.

Fixes: ccb1352e76cf ("net: Add Open vSwitch kernel components.")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/openvswitch/datapath.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Sergei Shtylyov Nov. 27, 2019, 10:07 a.m. UTC | #1
Hello!

On 26.11.2019 15:10, Paolo Abeni wrote:

> All callers already deal with errors correctly, dump a warn instead.

    Warning, maybe?

> Fixes: ccb1352e76cf ("net: Add Open vSwitch kernel components.")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>   net/openvswitch/datapath.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index d8c364d637b1..e94f675794f1 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -882,7 +882,7 @@ static struct sk_buff *ovs_flow_cmd_build_info(const struct sw_flow *flow,
>   	retval = ovs_flow_cmd_fill_info(flow, dp_ifindex, skb,
>   					info->snd_portid, info->snd_seq, 0,
>   					cmd, ufid_flags);
> -	BUG_ON(retval < 0);
> +	WARN_ON_ONCE(retval < 0);
>   	return skb;
>   }
>   

MBR, Sergei
David Miller Nov. 28, 2019, 9:17 p.m. UTC | #2
From: Paolo Abeni <pabeni@redhat.com>
Date: Tue, 26 Nov 2019 13:10:29 +0100

> All callers already deal with errors correctly, dump a warn instead.
> 
> Fixes: ccb1352e76cf ("net: Add Open vSwitch kernel components.")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  net/openvswitch/datapath.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index d8c364d637b1..e94f675794f1 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -882,7 +882,7 @@ static struct sk_buff *ovs_flow_cmd_build_info(const struct sw_flow *flow,
>  	retval = ovs_flow_cmd_fill_info(flow, dp_ifindex, skb,
>  					info->snd_portid, info->snd_seq, 0,
>  					cmd, ufid_flags);
> -	BUG_ON(retval < 0);
> +	WARN_ON_ONCE(retval < 0);
>  	return skb;
>  }

I don't think this is right.  We should propagate the error by freeing the skb
and returning a proper error pointer based upon retval.
Paolo Abeni Nov. 29, 2019, 10:16 a.m. UTC | #3
On Thu, 2019-11-28 at 13:17 -0800, David Miller wrote:
> From: Paolo Abeni <pabeni@redhat.com>
> Date: Tue, 26 Nov 2019 13:10:29 +0100
> 
> > All callers already deal with errors correctly, dump a warn instead.
> > 
> > Fixes: ccb1352e76cf ("net: Add Open vSwitch kernel components.")
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> >  net/openvswitch/datapath.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> > index d8c364d637b1..e94f675794f1 100644
> > --- a/net/openvswitch/datapath.c
> > +++ b/net/openvswitch/datapath.c
> > @@ -882,7 +882,7 @@ static struct sk_buff *ovs_flow_cmd_build_info(const struct sw_flow *flow,
> >       retval = ovs_flow_cmd_fill_info(flow, dp_ifindex, skb,
> >                                       info->snd_portid, info->snd_seq, 0,
> >                                       cmd, ufid_flags);
> > -     BUG_ON(retval < 0);
> > +     WARN_ON_ONCE(retval < 0);
> >       return skb;
> >  }
> 
> I don't think this is right.  We should propagate the error by freeing the skb
> and returning a proper error pointer based upon retval.

Indeed you are right. Thank you for catching this.

Never cook patches when coffee is too low :/

Will send a v2

Thank you!

Paolo

Patch
diff mbox series

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index d8c364d637b1..e94f675794f1 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -882,7 +882,7 @@  static struct sk_buff *ovs_flow_cmd_build_info(const struct sw_flow *flow,
 	retval = ovs_flow_cmd_fill_info(flow, dp_ifindex, skb,
 					info->snd_portid, info->snd_seq, 0,
 					cmd, ufid_flags);
-	BUG_ON(retval < 0);
+	WARN_ON_ONCE(retval < 0);
 	return skb;
 }