[GIT net] Open vSwitch

[GIT net] Open vSwitch

[Jesse Gross]

Two small bug fixes for net/3.9 including the issue previously
discussed where allocation of netlink notifications can fail after
changes have been committed.

The following changes since commit 330305cc4a6b0cb75c22fc01b8826f0ad755550f:

  ipv4: Fix ip-header identification for gso packets. (2013-03-26 13:50:05 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jesse/openvswitch.git fixes

for you to fetch changes up to d3e1101c9b75574e68380b5cb10c9395fd8855de:

  openvswitch: correct an invalid BUG_ON (2013-03-27 09:07:41 -0700)

----------------------------------------------------------------
Hong Zhiguo (1):
      openvswitch: correct an invalid BUG_ON

Jesse Gross (1):
      openvswitch: Preallocate reply skb in ovs_vport_cmd_set().

 net/openvswitch/datapath.c |   30 ++++++++++++++++++------------
 net/openvswitch/flow.c     |    2 +-
 2 files changed, 19 insertions(+), 13 deletions(-)

[PATCH net 1/2] openvswitch: Preallocate reply skb in ovs_vport_cmd_set().

[Jesse Gross]

Allocation of the Netlink notification skb can potentially fail
after changing vport configuration.  In general, we try to avoid
this by undoing any change we made but that is difficult for existing
objects.  This avoids the problem by preallocating the buffer (which
is fixed size).

Signed-off-by: Jesse Gross <jesse@nicira.com>
---
 net/openvswitch/datapath.c |   30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index a4b7247..6980c3e 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1593,10 +1593,8 @@ struct sk_buff *ovs_vport_cmd_build_info(struct vport *vport, u32 portid,
 		return ERR_PTR(-ENOMEM);
 
 	retval = ovs_vport_cmd_fill_info(vport, skb, portid, seq, 0, cmd);
-	if (retval < 0) {
-		kfree_skb(skb);
-		return ERR_PTR(retval);
-	}
+	BUG_ON(retval < 0);
+
 	return skb;
 }
 
@@ -1726,24 +1724,32 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct genl_info *info)
 	    nla_get_u32(a[OVS_VPORT_ATTR_TYPE]) != vport->ops->type)
 		err = -EINVAL;
 
+	reply = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!reply) {
+		err = -ENOMEM;
+		goto exit_unlock;
+	}
+
 	if (!err && a[OVS_VPORT_ATTR_OPTIONS])
 		err = ovs_vport_set_options(vport, a[OVS_VPORT_ATTR_OPTIONS]);
 	if (err)
-		goto exit_unlock;
+		goto exit_free;
+
 	if (a[OVS_VPORT_ATTR_UPCALL_PID])
 		vport->upcall_portid = nla_get_u32(a[OVS_VPORT_ATTR_UPCALL_PID]);
 
-	reply = ovs_vport_cmd_build_info(vport, info->snd_portid, info->snd_seq,
-					 OVS_VPORT_CMD_NEW);
-	if (IS_ERR(reply)) {
-		netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
-				ovs_dp_vport_multicast_group.id, PTR_ERR(reply));
-		goto exit_unlock;
-	}
+	err = ovs_vport_cmd_fill_info(vport, reply, info->snd_portid,
+				      info->snd_seq, 0, OVS_VPORT_CMD_NEW);
+	BUG_ON(err < 0);
 
 	genl_notify(reply, genl_info_net(info), info->snd_portid,
 		    ovs_dp_vport_multicast_group.id, info->nlhdr, GFP_KERNEL);
 
+	rtnl_unlock();
+	return 0;
+
+exit_free:
+	kfree_skb(reply);
 exit_unlock:
 	rtnl_unlock();
 	return err;
-- 
1.7.10.4

[PATCH net 2/2] openvswitch: correct an invalid BUG_ON

[Jesse Gross]

From: Hong Zhiguo <honkiko@gmail.com>

table->count is uint32_t

Signed-off-by: Hong Zhiguo <honkiko@gmail.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
 net/openvswitch/flow.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index fe0e421..67a2b78 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -795,9 +795,9 @@ void ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow)
 
 void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow)
 {
+	BUG_ON(table->count == 0);
 	hlist_del_rcu(&flow->hash_node[table->node_ver]);
 	table->count--;
-	BUG_ON(table->count < 0);
 }
 
 /* The size of the argument for each %OVS_KEY_ATTR_* Netlink attribute.  */
-- 
1.7.10.4

Re: [GIT net] Open vSwitch

[David Miller]

From: Jesse Gross <jesse@nicira.com>
Date: Tue, 16 Apr 2013 13:17:27 -0700

> Two small bug fixes for net/3.9 including the issue previously
> discussed where allocation of netlink notifications can fail after
> changes have been committed.

Looks good, pulled, thanks!

Re: [GIT net] Open vSwitch

[Jesse Gross]

On Tue, Apr 16, 2013 at 1:53 PM, David Miller <davem@davemloft.net> wrote:
> From: Jesse Gross <jesse@nicira.com>
> Date: Tue, 16 Apr 2013 13:17:27 -0700
>
>> Two small bug fixes for net/3.9 including the issue previously
>> discussed where allocation of netlink notifications can fail after
>> changes have been committed.
>
> Looks good, pulled, thanks!

Sorry, but can you check whether this was pulled correctly? I'm not
seeing it in your net tree.

Re: [GIT net] Open vSwitch

[David Miller]

From: Jesse Gross <jesse@nicira.com>
Date: Wed, 17 Apr 2013 19:31:41 -0700

> On Tue, Apr 16, 2013 at 1:53 PM, David Miller <davem@davemloft.net> wrote:
>> From: Jesse Gross <jesse@nicira.com>
>> Date: Tue, 16 Apr 2013 13:17:27 -0700
>>
>>> Two small bug fixes for net/3.9 including the issue previously
>>> discussed where allocation of netlink notifications can fail after
>>> changes have been committed.
>>
>> Looks good, pulled, thanks!
> 
> Sorry, but can you check whether this was pulled correctly? I'm not
> seeing it in your net tree.

I don't see it either.

It might be sitting locally on a computer I'm not physically in front
of right now, I'll check when I next go there (either tonight or
tomorrow).

Sorry!

Re: [GIT net] Open vSwitch

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Wed, 17 Apr 2013 22:37:41 -0400 (EDT)

> From: Jesse Gross <jesse@nicira.com>
> Date: Wed, 17 Apr 2013 19:31:41 -0700
> 
>> On Tue, Apr 16, 2013 at 1:53 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Jesse Gross <jesse@nicira.com>
>>> Date: Tue, 16 Apr 2013 13:17:27 -0700
>>>
>>>> Two small bug fixes for net/3.9 including the issue previously
>>>> discussed where allocation of netlink notifications can fail after
>>>> changes have been committed.
>>>
>>> Looks good, pulled, thanks!
>> 
>> Sorry, but can you check whether this was pulled correctly? I'm not
>> seeing it in your net tree.
> 
> I don't see it either.
> 
> It might be sitting locally on a computer I'm not physically in front
> of right now, I'll check when I next go there (either tonight or
> tomorrow).
> 
> Sorry!

Yes, that's exactly what happened, I forgot to push out right before
leaving this machine yesterday.

I've fixed this up and pushed it all back out to 'net'.