* [PATCH net 0/2] openvswitch: remove a couple of BUG_ON() @ 2019-11-26 12:10 Paolo Abeni 2019-11-26 12:10 ` [PATCH net 1/2] openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info() Paolo Abeni 2019-11-26 12:10 ` [PATCH net 2/2] openvswitch: remove another BUG_ON() Paolo Abeni 0 siblings, 2 replies; 6+ messages in thread From: Paolo Abeni @ 2019-11-26 12:10 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, Pravin B Shelar The openvswitch kernel datapath includes some BUG_ON() statements to check for exceptional/unexpected failures. These patches drop a couple of them, where we can do that without introducing other side effects. Paolo Abeni (2): openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info() openvswitch: remove another BUG_ON() net/openvswitch/datapath.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) -- 2.21.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 1/2] openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info() 2019-11-26 12:10 [PATCH net 0/2] openvswitch: remove a couple of BUG_ON() Paolo Abeni @ 2019-11-26 12:10 ` Paolo Abeni 2019-11-27 10:07 ` Sergei Shtylyov 2019-11-28 21:17 ` David Miller 2019-11-26 12:10 ` [PATCH net 2/2] openvswitch: remove another BUG_ON() Paolo Abeni 1 sibling, 2 replies; 6+ messages in thread From: Paolo Abeni @ 2019-11-26 12:10 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, Pravin B Shelar 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; } -- 2.21.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info() 2019-11-26 12:10 ` [PATCH net 1/2] openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info() Paolo Abeni @ 2019-11-27 10:07 ` Sergei Shtylyov 2019-11-28 21:17 ` David Miller 1 sibling, 0 replies; 6+ messages in thread From: Sergei Shtylyov @ 2019-11-27 10:07 UTC (permalink / raw) To: Paolo Abeni, netdev; +Cc: David S. Miller, Pravin B Shelar 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info() 2019-11-26 12:10 ` [PATCH net 1/2] openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info() Paolo Abeni 2019-11-27 10:07 ` Sergei Shtylyov @ 2019-11-28 21:17 ` David Miller 2019-11-29 10:16 ` Paolo Abeni 1 sibling, 1 reply; 6+ messages in thread From: David Miller @ 2019-11-28 21:17 UTC (permalink / raw) To: pabeni; +Cc: netdev, pshelar 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info() 2019-11-28 21:17 ` David Miller @ 2019-11-29 10:16 ` Paolo Abeni 0 siblings, 0 replies; 6+ messages in thread From: Paolo Abeni @ 2019-11-29 10:16 UTC (permalink / raw) To: David Miller; +Cc: netdev, pshelar 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 2/2] openvswitch: remove another BUG_ON() 2019-11-26 12:10 [PATCH net 0/2] openvswitch: remove a couple of BUG_ON() Paolo Abeni 2019-11-26 12:10 ` [PATCH net 1/2] openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info() Paolo Abeni @ 2019-11-26 12:10 ` Paolo Abeni 1 sibling, 0 replies; 6+ messages in thread From: Paolo Abeni @ 2019-11-26 12:10 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, Pravin B Shelar If we can't build the flow del notification, we can simply delete the flow, no need to crash the kernel. Still keep a WARN_ON to preserve debuggability. Note: the BUG_ON() predates the Fixes tag, but this change can be applied only after the mentioned commit. Fixes: aed067783e50 ("openvswitch: Minimize ovs_flow_cmd_del critical section.") Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- net/openvswitch/datapath.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index e94f675794f1..50656e807c8c 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -1346,7 +1346,8 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct genl_info *info) OVS_FLOW_CMD_DEL, ufid_flags); rcu_read_unlock(); - BUG_ON(err < 0); + if (WARN_ON_ONCE(err < 0)) + goto out_free; ovs_notify(&dp_flow_genl_family, reply, info); } else { @@ -1354,6 +1355,7 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct genl_info *info) } } +out_free: ovs_flow_free(flow, true); return 0; unlock: -- 2.21.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-29 10:16 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-11-26 12:10 [PATCH net 0/2] openvswitch: remove a couple of BUG_ON() Paolo Abeni 2019-11-26 12:10 ` [PATCH net 1/2] openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info() Paolo Abeni 2019-11-27 10:07 ` Sergei Shtylyov 2019-11-28 21:17 ` David Miller 2019-11-29 10:16 ` Paolo Abeni 2019-11-26 12:10 ` [PATCH net 2/2] openvswitch: remove another BUG_ON() Paolo Abeni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).