Netdev List
 help / color / mirror / Atom feed
* [PATCH 1/5] bridge: Validate IFLA_BRIDGE_FLAGS attribute length
From: Thomas Graf @ 2014-11-26 12:42 UTC (permalink / raw)
  To: davem; +Cc: stephen, netdev, Vlad Yasevich
In-Reply-To: <cover.1417005245.git.tgraf@suug.ch>

Payload is currently accessed blindly and may exceed valid message
boundaries.

Fixes: 407af3299 ("bridge: Add netlink interface to configure vlans on bridge ports")
Cc: Vlad Yasevich <vyasevic@redhat.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
 net/core/rtnetlink.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a688268..5a853f8 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2798,6 +2798,9 @@ static int rtnl_bridge_setlink(struct sk_buff *skb, struct nlmsghdr *nlh)
 	if (br_spec) {
 		nla_for_each_nested(attr, br_spec, rem) {
 			if (nla_type(attr) == IFLA_BRIDGE_FLAGS) {
+				if (nla_len(attr) < sizeof(flags))
+					return -EINVAL;
+
 				have_flags = true;
 				flags = nla_get_u16(attr);
 				break;
@@ -2868,6 +2871,9 @@ static int rtnl_bridge_dellink(struct sk_buff *skb, struct nlmsghdr *nlh)
 	if (br_spec) {
 		nla_for_each_nested(attr, br_spec, rem) {
 			if (nla_type(attr) == IFLA_BRIDGE_FLAGS) {
+				if (nla_len(attr) < sizeof(flags))
+					return -EINVAL;
+
 				have_flags = true;
 				flags = nla_get_u16(attr);
 				break;
-- 
1.9.3

^ permalink raw reply related

* [PATCH 2/5] net: Validate IFLA_BRIDGE_MODE attribute length
From: Thomas Graf @ 2014-11-26 12:42 UTC (permalink / raw)
  To: davem; +Cc: stephen, netdev, Ajit Khaparde, John Fastabend
In-Reply-To: <cover.1417005245.git.tgraf@suug.ch>

Payload is currently accessed blindly and may exceed valid message
boundaries.

Fixes: a77dcb8c8 ("be2net: set and query VEB/VEPA mode of the PF interface")
Fixes: 815cccbf1 ("ixgbe: add setlink, getlink support to ixgbe and ixgbevf")
Cc: Ajit Khaparde <ajit.khaparde@emulex.com>
Cc: John Fastabend <john.r.fastabend@intel.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
 drivers/net/ethernet/emulex/benet/be_main.c   | 3 +++
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 3e8475c..337e4cd 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -4314,6 +4314,9 @@ static int be_ndo_bridge_setlink(struct net_device *dev, struct nlmsghdr *nlh)
 		if (nla_type(attr) != IFLA_BRIDGE_MODE)
 			continue;
 
+		if (nla_len(attr) < sizeof(mode))
+			return -EINVAL;
+
 		mode = nla_get_u16(attr);
 		if (mode != BRIDGE_MODE_VEPA && mode != BRIDGE_MODE_VEB)
 			return -EINVAL;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 82ffe8b..dff9905 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7677,6 +7677,9 @@ static int ixgbe_ndo_bridge_setlink(struct net_device *dev,
 		if (nla_type(attr) != IFLA_BRIDGE_MODE)
 			continue;
 
+		if (nla_len(attr) < sizeof(mode))
+			return -EINVAL;
+
 		mode = nla_get_u16(attr);
 		if (mode == BRIDGE_MODE_VEPA) {
 			reg = 0;
-- 
1.9.3

^ permalink raw reply related

* [PATCH 3/5] net: Check for presence of IFLA_AF_SPEC
From: Thomas Graf @ 2014-11-26 12:42 UTC (permalink / raw)
  To: davem; +Cc: stephen, netdev, Ajit Khaparde, John Fastabend
In-Reply-To: <cover.1417005245.git.tgraf@suug.ch>

ndo_bridge_setlink() is currently only called on the slave if
IFLA_AF_SPEC is set but this is a very fragile assumption and may
change in the future.

Cc: Ajit Khaparde <ajit.khaparde@emulex.com>
Cc: John Fastabend <john.r.fastabend@intel.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
 drivers/net/ethernet/emulex/benet/be_main.c   | 2 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 337e4cd..597c463 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -4309,6 +4309,8 @@ static int be_ndo_bridge_setlink(struct net_device *dev, struct nlmsghdr *nlh)
 		return -EOPNOTSUPP;
 
 	br_spec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
+	if (!br_spec)
+		return -EINVAL;
 
 	nla_for_each_nested(attr, br_spec, rem) {
 		if (nla_type(attr) != IFLA_BRIDGE_MODE)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index dff9905..cc51554 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7669,6 +7669,8 @@ static int ixgbe_ndo_bridge_setlink(struct net_device *dev,
 		return -EOPNOTSUPP;
 
 	br_spec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
+	if (!br_spec)
+		return -EINVAL;
 
 	nla_for_each_nested(attr, br_spec, rem) {
 		__u16 mode;
-- 
1.9.3

^ permalink raw reply related

* [PATCH 4/5] bridge: Add missing policy entry for IFLA_BRPORT_FAST_LEAVE
From: Thomas Graf @ 2014-11-26 12:42 UTC (permalink / raw)
  To: davem; +Cc: stephen, netdev
In-Reply-To: <cover.1417005245.git.tgraf@suug.ch>

Fixes: c2d3babf ("bridge: implement multicast fast leave")
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
 net/bridge/br_netlink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 2ff9706..e5ec470 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -280,6 +280,7 @@ static const struct nla_policy br_port_policy[IFLA_BRPORT_MAX + 1] = {
 	[IFLA_BRPORT_MODE]	= { .type = NLA_U8 },
 	[IFLA_BRPORT_GUARD]	= { .type = NLA_U8 },
 	[IFLA_BRPORT_PROTECT]	= { .type = NLA_U8 },
+	[IFLA_BRPORT_FAST_LEAVE]= { .type = NLA_U8 },
 	[IFLA_BRPORT_LEARNING]	= { .type = NLA_U8 },
 	[IFLA_BRPORT_UNICAST_FLOOD] = { .type = NLA_U8 },
 };
-- 
1.9.3

^ permalink raw reply related

* [PATCH 5/5] bridge: Sanitize IFLA_EXT_MASK for AF_BRIDGE:RTM_GETLINK
From: Thomas Graf @ 2014-11-26 12:42 UTC (permalink / raw)
  To: davem; +Cc: stephen, netdev, Vlad Yasevich
In-Reply-To: <cover.1417005245.git.tgraf@suug.ch>

Only search for IFLA_EXT_MASK if the message actually carries a
ifinfomsg header and validate minimal length requirements for
IFLA_EXT_MASK.

Fixes: 6cbdceeb ("bridge: Dump vlan information from a bridge port")
Cc: Vlad Yasevich <vyasevic@redhat.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
 net/core/rtnetlink.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 5a853f8..b9b7dfa 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2685,13 +2685,20 @@ static int rtnl_bridge_getlink(struct sk_buff *skb, struct netlink_callback *cb)
 	int idx = 0;
 	u32 portid = NETLINK_CB(cb->skb).portid;
 	u32 seq = cb->nlh->nlmsg_seq;
-	struct nlattr *extfilt;
 	u32 filter_mask = 0;
 
-	extfilt = nlmsg_find_attr(cb->nlh, sizeof(struct ifinfomsg),
-				  IFLA_EXT_MASK);
-	if (extfilt)
-		filter_mask = nla_get_u32(extfilt);
+	if (nlmsg_len(cb->nlh) > sizeof(struct ifinfomsg)) {
+		struct nlattr *extfilt;
+
+		extfilt = nlmsg_find_attr(cb->nlh, sizeof(struct ifinfomsg),
+					  IFLA_EXT_MASK);
+		if (extfilt) {
+			if (nla_len(extfilt) < sizeof(filter_mask))
+				return -EINVAL;
+
+			filter_mask = nla_get_u32(extfilt);
+		}
+	}
 
 	rcu_read_lock();
 	for_each_netdev_rcu(net, dev) {
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH v4 21/42] virtio_net: get rid of virtio_net_hdr/skb_vnet_hdr
From: Cornelia Huck @ 2014-11-26 12:50 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: rusty, netdev, linux-kernel, virtualization, pbonzini,
	David Miller
In-Reply-To: <1416933600-21398-22-git-send-email-mst@redhat.com>

On Tue, 25 Nov 2014 18:42:53 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> virtio 1.0 doesn't use virtio_net_hdr anymore, and in fact, it's not
> really useful since virtio_net_hdr_mrg_rxbuf includes that as the first
> field anyway.
> 
> Let's drop it, precalculate header len and store within vi instead.
> 
> This way we can also remove struct skb_vnet_hdr.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/net/virtio_net.c | 90 ++++++++++++++++++++++--------------------------
>  1 file changed, 41 insertions(+), 49 deletions(-)
> 

> @@ -260,13 +256,11 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> 
>  	hdr = skb_vnet_hdr(skb);
> 
> -	if (vi->mergeable_rx_bufs) {
> -		hdr_len = sizeof hdr->mhdr;
> -		hdr_padded_len = sizeof hdr->mhdr;
> -	} else {
> -		hdr_len = sizeof hdr->hdr;
> +	hdr_len = vi->hdr_len;
> +	if (vi->mergeable_rx_bufs)
> +		hdr_padded_len = sizeof *hdr;

sizeof without () looks weird to me.

> +	else
>  		hdr_padded_len = sizeof(struct padded_vnet_hdr);
> -	}
> 
>  	memcpy(hdr, p, hdr_len);
> 

Otherwise:

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply

* Re: [PATCH v4 22/42] virtio_net: stricter short buffer length checks
From: Cornelia Huck @ 2014-11-26 13:00 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: rusty, netdev, linux-kernel, virtualization, pbonzini,
	David Miller
In-Reply-To: <1416933600-21398-23-git-send-email-mst@redhat.com>

On Tue, 25 Nov 2014 18:42:58 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> Our buffer length check is not strict enough for mergeable
> buffers: buffer can still be shorter that header + address
> by 2 bytes.
> 
> Fix that up.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/net/virtio_net.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply

* Re: [PATCH v4 23/42] virtio_net: bigger header when VERSION_1 is set
From: Cornelia Huck @ 2014-11-26 13:04 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: rusty, netdev, linux-kernel, virtualization, pbonzini,
	David Miller
In-Reply-To: <1416933600-21398-24-git-send-email-mst@redhat.com>

On Tue, 25 Nov 2014 18:43:02 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> With VERSION_1 virtio_net uses same header size
> whether mergeable buffers are enabled or not.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/net/virtio_net.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply

* Re: [PATCH v4 24/42] virtio_net: enable v1.0 support
From: Cornelia Huck @ 2014-11-26 13:08 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: rusty, netdev, linux-kernel, virtualization, pbonzini,
	David Miller
In-Reply-To: <1416933600-21398-25-git-send-email-mst@redhat.com>

On Tue, 25 Nov 2014 18:43:06 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> Now that we have completed 1.0 support, enable it in our driver.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/net/virtio_net.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Hm. The spec states that mac is driver-writable in the legacy case.
Don't we need to fence writing it in virtnet_set_mac_address() in the
virtio 1.0 case?

^ permalink raw reply

* Re: [PATCH v4 21/42] virtio_net: get rid of virtio_net_hdr/skb_vnet_hdr
From: Michael S. Tsirkin @ 2014-11-26 13:11 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: rusty, netdev, linux-kernel, virtualization, pbonzini,
	David Miller
In-Reply-To: <20141126135053.1bf4f97c.cornelia.huck@de.ibm.com>

On Wed, Nov 26, 2014 at 01:50:53PM +0100, Cornelia Huck wrote:
> On Tue, 25 Nov 2014 18:42:53 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > virtio 1.0 doesn't use virtio_net_hdr anymore, and in fact, it's not
> > really useful since virtio_net_hdr_mrg_rxbuf includes that as the first
> > field anyway.
> > 
> > Let's drop it, precalculate header len and store within vi instead.
> > 
> > This way we can also remove struct skb_vnet_hdr.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  drivers/net/virtio_net.c | 90 ++++++++++++++++++++++--------------------------
> >  1 file changed, 41 insertions(+), 49 deletions(-)
> > 
> 
> > @@ -260,13 +256,11 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > 
> >  	hdr = skb_vnet_hdr(skb);
> > 
> > -	if (vi->mergeable_rx_bufs) {
> > -		hdr_len = sizeof hdr->mhdr;
> > -		hdr_padded_len = sizeof hdr->mhdr;
> > -	} else {
> > -		hdr_len = sizeof hdr->hdr;
> > +	hdr_len = vi->hdr_len;
> > +	if (vi->mergeable_rx_bufs)
> > +		hdr_padded_len = sizeof *hdr;
> 
> sizeof without () looks weird to me.

It's how it was originally, so I think I'd rather keep it.
I kind of like it too: it stresses the fact that sizeof is not a function
and so does not actually dereference the pointer
even though it looks like it does.

We can change it with a patch on top, but should then be changed
everywhere in this file.

> > +	else
> >  		hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > -	}
> > 
> >  	memcpy(hdr, p, hdr_len);
> > 
> 
> Otherwise:
> 
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply

* Re: [PATCH iproute2] iplink: allow to show ip addresses
From: Jiri Benc @ 2014-11-26 13:12 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: Nicolas Dichtel, shemminger, netdev
In-Reply-To: <20141125061023.GA8819@unicorn.suse.cz>

On Tue, 25 Nov 2014 07:10:23 +0100, Michal Kubecek wrote:
> On Mon, Nov 24, 2014 at 05:42:17PM +0100, Nicolas Dichtel wrote:
> > This patch adds a new option (-addresses) to the 'ip link' command so that the
> > user can display link details and IP addresses with the same command.
> > 
> > Example:
> > $ ip -d -a l ls gre1
> > 9: gre1@NONE: <POINTOPOINT,NOARP,UP,LOWER_UP> mtu 1468 qdisc noqueue state UNKNOWN mode DEFAULT group default
> >     link/gre 10.16.0.249 peer 10.16.0.121 promiscuity 0
> >     gre remote 10.16.0.121 local 10.16.0.249 ttl inherit ikey 0.0.0.10 okey 0.0.0.10 icsum ocsum
> >     inet 192.168.0.249 peer 192.168.0.121/32 scope global gre1
> >        valid_lft forever preferred_lft forever
> >     inet6 fe80::5efe:a10:f9/64 scope link
> >        valid_lft forever preferred_lft forever
> 
> Perhaps it would be more consistent to add -d option to "ip addr show" 
> instead as we already have -s for statistics there (commit 5d5cf1b43).

Agreed.

 Jiri

-- 
Jiri Benc

^ permalink raw reply

* Re: [PATCH v4 24/42] virtio_net: enable v1.0 support
From: Michael S. Tsirkin @ 2014-11-26 13:28 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: rusty, netdev, linux-kernel, virtualization, pbonzini,
	David Miller
In-Reply-To: <20141126140857.6ff85e4b.cornelia.huck@de.ibm.com>

On Wed, Nov 26, 2014 at 02:08:57PM +0100, Cornelia Huck wrote:
> On Tue, 25 Nov 2014 18:43:06 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > Now that we have completed 1.0 support, enable it in our driver.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  drivers/net/virtio_net.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> 
> Hm. The spec states that mac is driver-writable in the legacy case.
> Don't we need to fence writing it in virtnet_set_mac_address() in the
> virtio 1.0 case?

You are right. I'll add a patch to fix that one: we
should return -EOPNOTSUPP unless
	VIRTIO_NET_F_CTRL_MAC_ADDR or
	VERSION_1 is clear and VIRTIO_NET_F_MAC is set.

^ permalink raw reply

* Re: [PATCH 2/5] net: Validate IFLA_BRIDGE_MODE attribute length
From: Jeff Kirsher @ 2014-11-26 13:29 UTC (permalink / raw)
  To: Thomas Graf
  Cc: David Miller, Stephen Hemminger, netdev, Ajit Khaparde,
	John Fastabend
In-Reply-To: <4a88a0350064b5c2ec4e2adcef5afdfcab3e45dd.1417005245.git.tgraf@suug.ch>

On Wed, Nov 26, 2014 at 4:42 AM, Thomas Graf <tgraf@suug.ch> wrote:
> Payload is currently accessed blindly and may exceed valid message
> boundaries.
>
> Fixes: a77dcb8c8 ("be2net: set and query VEB/VEPA mode of the PF interface")
> Fixes: 815cccbf1 ("ixgbe: add setlink, getlink support to ixgbe and ixgbevf")
> Cc: Ajit Khaparde <ajit.khaparde@emulex.com>
> Cc: John Fastabend <john.r.fastabend@intel.com>
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
> ---
>  drivers/net/ethernet/emulex/benet/be_main.c   | 3 +++
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +++
>  2 files changed, 6 insertions(+)
>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

^ permalink raw reply

* Re: [PATCH 3/5] net: Check for presence of IFLA_AF_SPEC
From: Jeff Kirsher @ 2014-11-26 13:30 UTC (permalink / raw)
  To: Thomas Graf
  Cc: David Miller, Stephen Hemminger, netdev, Ajit Khaparde,
	John Fastabend
In-Reply-To: <495eced061d109e02b035ad56e56a3de2505ee22.1417005245.git.tgraf@suug.ch>

On Wed, Nov 26, 2014 at 4:42 AM, Thomas Graf <tgraf@suug.ch> wrote:
> ndo_bridge_setlink() is currently only called on the slave if
> IFLA_AF_SPEC is set but this is a very fragile assumption and may
> change in the future.
>
> Cc: Ajit Khaparde <ajit.khaparde@emulex.com>
> Cc: John Fastabend <john.r.fastabend@intel.com>
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
> ---
>  drivers/net/ethernet/emulex/benet/be_main.c   | 2 ++
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 ++
>  2 files changed, 4 insertions(+)
>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

^ permalink raw reply

* Re: [PATCH net-next V2] tun/macvtap: use consume_skb() instead of kfree_skb() when needed
From: Michael S. Tsirkin @ 2014-11-26 13:47 UTC (permalink / raw)
  To: Jason Wang; +Cc: davem, netdev, linux-kernel, Eric Dumazet
In-Reply-To: <1416987810-23263-1-git-send-email-jasowang@redhat.com>

On Wed, Nov 26, 2014 at 03:43:30PM +0800, Jason Wang wrote:
> To be more friendly with drop monitor, we should only call kfree_skb() when
> the packets were dropped and use consume_skb() in other cases.
> 
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> Changes from V1:
> - check the return value of tun/macvtap_put_user()
> ---
>  drivers/net/macvtap.c | 5 ++++-
>  drivers/net/tun.c     | 5 ++++-
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 42a80d3..c171ab6 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -862,7 +862,10 @@ static ssize_t macvtap_do_read(struct macvtap_queue *q,
>  		}
>  		iov_iter_init(&iter, READ, iv, segs, len);
>  		ret = macvtap_put_user(q, skb, &iter);
> -		kfree_skb(skb);
> +		if (ret < 0)

Maybe unlikely() here?

> +			kfree_skb(skb);
> +		else
> +			consume_skb(skb);
>  		break;
>  	}
>  
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index ac53a73..a21c130 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1363,7 +1363,10 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
>  
>  	iov_iter_init(&iter, READ, iv, segs, len);
>  	ret = tun_put_user(tun, tfile, skb, &iter);
> -	kfree_skb(skb);
> +	if (ret < 0)
> +		kfree_skb(skb);
> +	else
> +		consume_skb(skb);
>  
>  	return ret;
>  }
> -- 
> 1.9.1

^ permalink raw reply

* [PATCH net-next] bridge: add vlan id to mdb notifications
From: roopa @ 2014-11-26 13:53 UTC (permalink / raw)
  To: vyasevich, stephen, roopa; +Cc: netdev, wkok, gospo, jtoppins, sashok

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch adds vlan id to bridge mdb notifications.

I have tested it with older iproute2 and does not seem to break
compatibility.

Signed-off-by: Wilson kok <wkok@cumulusnetworks.com>
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 include/uapi/linux/if_bridge.h |    1 +
 net/bridge/br_mdb.c            |    4 ++++
 2 files changed, 5 insertions(+)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index da17e45..db061fd 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -185,6 +185,7 @@ struct br_mdb_entry {
 			struct in6_addr ip6;
 		} u;
 		__be16		proto;
+		__be16		vid;
 	} addr;
 };
 
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 5df0526..fa28540 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -92,6 +92,7 @@ static int br_mdb_fill_info(struct sk_buff *skb, struct netlink_callback *cb,
 						e.addr.u.ip6 = p->addr.u.ip6;
 #endif
 					e.addr.proto = p->addr.proto;
+					e.addr.vid = p->addr.vid;
 					if (nla_put(skb, MDBA_MDB_ENTRY_INFO, sizeof(e), &e)) {
 						nla_nest_cancel(skb, nest2);
 						err = -EMSGSIZE;
@@ -240,6 +241,7 @@ void br_mdb_notify(struct net_device *dev, struct net_bridge_port *port,
 #if IS_ENABLED(CONFIG_IPV6)
 	entry.addr.u.ip6 = group->u.ip6;
 #endif
+	entry.addr.vid = group->vid;
 	__br_mdb_notify(dev, &entry, type);
 }
 
@@ -377,6 +379,7 @@ static int __br_mdb_add(struct net *net, struct net_bridge *br,
 	else
 		ip.u.ip6 = entry->addr.u.ip6;
 #endif
+	ip.vid = entry->addr.vid;
 
 	spin_lock_bh(&br->multicast_lock);
 	ret = br_mdb_add_group(br, p, &ip, entry->state);
@@ -430,6 +433,7 @@ static int __br_mdb_del(struct net_bridge *br, struct br_mdb_entry *entry)
 		ip.u.ip6 = entry->addr.u.ip6;
 #endif
 	}
+	ip.vid = entry->addr.vid;
 
 	spin_lock_bh(&br->multicast_lock);
 	mdb = mlock_dereference(br->mdb, br);
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH v4 25/42] vhost: add memory access wrappers
From: Cornelia Huck @ 2014-11-26 13:54 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kvm, rusty, netdev, linux-kernel, virtualization, pbonzini,
	David Miller
In-Reply-To: <1416933600-21398-26-git-send-email-mst@redhat.com>

On Tue, 25 Nov 2014 18:43:10 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

"These wrappers are needed to handle virtio endianness conversions."

?

> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/vhost/vhost.h | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 3eda654..b9032e8 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -174,6 +174,37 @@ enum {
> 
>  static inline int vhost_has_feature(struct vhost_virtqueue *vq, int bit)
>  {
> -	return vq->acked_features & (1 << bit);
> +	return vq->acked_features & (1ULL << bit);

Should this hunk go into patch 28?

> +}

^ permalink raw reply

* Re: [PATCH iproute2] iplink: allow to show ip addresses
From: Nicolas Dichtel @ 2014-11-26 14:00 UTC (permalink / raw)
  To: Jiri Benc, Michal Kubecek; +Cc: shemminger, netdev
In-Reply-To: <20141126141204.42c76a37@griffin>

Le 26/11/2014 14:12, Jiri Benc a écrit :
> On Tue, 25 Nov 2014 07:10:23 +0100, Michal Kubecek wrote:
>> On Mon, Nov 24, 2014 at 05:42:17PM +0100, Nicolas Dichtel wrote:
>>> This patch adds a new option (-addresses) to the 'ip link' command so that the
>>> user can display link details and IP addresses with the same command.
>>>
>>> Example:
>>> $ ip -d -a l ls gre1
>>> 9: gre1@NONE: <POINTOPOINT,NOARP,UP,LOWER_UP> mtu 1468 qdisc noqueue state UNKNOWN mode DEFAULT group default
>>>      link/gre 10.16.0.249 peer 10.16.0.121 promiscuity 0
>>>      gre remote 10.16.0.121 local 10.16.0.249 ttl inherit ikey 0.0.0.10 okey 0.0.0.10 icsum ocsum
>>>      inet 192.168.0.249 peer 192.168.0.121/32 scope global gre1
>>>         valid_lft forever preferred_lft forever
>>>      inet6 fe80::5efe:a10:f9/64 scope link
>>>         valid_lft forever preferred_lft forever
>>
>> Perhaps it would be more consistent to add -d option to "ip addr show"
>> instead as we already have -s for statistics there (commit 5d5cf1b43).
>
> Agreed.
I also agree ;-)
Will send a v2.


Thank you,
Nicolas

^ permalink raw reply

* Re: [PATCH v4 25/42] vhost: add memory access wrappers
From: Michael S. Tsirkin @ 2014-11-26 14:05 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: kvm, rusty, netdev, linux-kernel, virtualization, pbonzini,
	David Miller
In-Reply-To: <20141126145438.4421c00d.cornelia.huck@de.ibm.com>

On Wed, Nov 26, 2014 at 02:54:38PM +0100, Cornelia Huck wrote:
> On Tue, 25 Nov 2014 18:43:10 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> "These wrappers are needed to handle virtio endianness conversions."
> 
> ?

yes, it's same as virtio ones. I'll add this text, thanks.

> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  drivers/vhost/vhost.h | 33 ++++++++++++++++++++++++++++++++-
> >  1 file changed, 32 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > index 3eda654..b9032e8 100644
> > --- a/drivers/vhost/vhost.h
> > +++ b/drivers/vhost/vhost.h
> > @@ -174,6 +174,37 @@ enum {
> > 
> >  static inline int vhost_has_feature(struct vhost_virtqueue *vq, int bit)
> >  {
> > -	return vq->acked_features & (1 << bit);
> > +	return vq->acked_features & (1ULL << bit);
> 
> Should this hunk go into patch 28?

Well, this is needed here since 1 << 32 is not legal C.

I can move it - this means patch 28 will have to move earlier
in series though.



> > +}

^ permalink raw reply

* [PATCH net-next] sky2: Fix crash inside sky2_rx_clean
From: Mirko Lindner @ 2014-11-26 14:13 UTC (permalink / raw)
  To: davem, netdev@vger.kernel.org

If sky2->tx_le = pci_alloc_consistent() or sky2->tx_ring = kcalloc() in
sky2_alloc_buffers() fails, sky2->rx_ring = kcalloc() will never be called.
In this error case handling, sky2_rx_clean() is called from within
sky2_free_buffers().

In sky2_rx_clean() we find the following:

...
   memset(sky2->rx_le, 0, RX_LE_BYTES);
...

This results in a memset using a NULL pointer and will crash the system.

Signed-off-by: Mirko Lindner <mlindner@marvell.com>
---
 drivers/net/ethernet/marvell/sky2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/sky2.c b/drivers/net/ethernet/marvell/sky2.c
index 53a1cc5..f8ab220 100644
--- a/drivers/net/ethernet/marvell/sky2.c
+++ b/drivers/net/ethernet/marvell/sky2.c
@@ -1361,7 +1361,9 @@ static void sky2_rx_clean(struct sky2_port *sky2)
 {
 	unsigned i;
 
-	memset(sky2->rx_le, 0, RX_LE_BYTES);
+	if (sky2->rx_le)
+		memset(sky2->rx_le, 0, RX_LE_BYTES);
+
 	for (i = 0; i < sky2->rx_pending; i++) {
 		struct rx_ring_info *re = sky2->rx_ring + i;
 
-- 
2.1.3

^ permalink raw reply related

* Re: [PATCH v4 25/42] vhost: add memory access wrappers
From: Cornelia Huck @ 2014-11-26 14:17 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kvm, rusty, netdev, linux-kernel, virtualization, pbonzini,
	David Miller
In-Reply-To: <20141126140539.GA5528@redhat.com>

On Wed, 26 Nov 2014 16:05:39 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Nov 26, 2014 at 02:54:38PM +0100, Cornelia Huck wrote:
> > On Tue, 25 Nov 2014 18:43:10 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:

> > > @@ -174,6 +174,37 @@ enum {
> > > 
> > >  static inline int vhost_has_feature(struct vhost_virtqueue *vq, int bit)
> > >  {
> > > -	return vq->acked_features & (1 << bit);
> > > +	return vq->acked_features & (1ULL << bit);
> > 
> > Should this hunk go into patch 28?
> 
> Well, this is needed here since 1 << 32 is not legal C.
> 
> I can move it - this means patch 28 will have to move earlier
> in series though.

Yes, I think it makes sense to move patch 28 earlier.

^ permalink raw reply

* Re: [PATCH v4 25/42] vhost: add memory access wrappers
From: Michael S. Tsirkin @ 2014-11-26 14:24 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: kvm, rusty, netdev, linux-kernel, virtualization, pbonzini,
	David Miller
In-Reply-To: <20141126151750.61a40fe4.cornelia.huck@de.ibm.com>

On Wed, Nov 26, 2014 at 03:17:50PM +0100, Cornelia Huck wrote:
> On Wed, 26 Nov 2014 16:05:39 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Nov 26, 2014 at 02:54:38PM +0100, Cornelia Huck wrote:
> > > On Tue, 25 Nov 2014 18:43:10 +0200
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > > > @@ -174,6 +174,37 @@ enum {
> > > > 
> > > >  static inline int vhost_has_feature(struct vhost_virtqueue *vq, int bit)
> > > >  {
> > > > -	return vq->acked_features & (1 << bit);
> > > > +	return vq->acked_features & (1ULL << bit);
> > > 
> > > Should this hunk go into patch 28?
> > 
> > Well, this is needed here since 1 << 32 is not legal C.
> > 
> > I can move it - this means patch 28 will have to move earlier
> > in series though.
> 
> Yes, I think it makes sense to move patch 28 earlier.

Will do, thanks.

-- 
MST

^ permalink raw reply

* Re: [PATCH v4 26/42] vhost/net: force len for TX to host endian
From: Cornelia Huck @ 2014-11-26 14:31 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kvm, rusty, netdev, linux-kernel, virtualization, pbonzini,
	David Miller
In-Reply-To: <1416933600-21398-27-git-send-email-mst@redhat.com>

On Tue, 25 Nov 2014 18:43:14 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> We use native endian-ness internally but never
> expose it to guest.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/vhost/net.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 8dae2f7..dce5c58 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -48,15 +48,15 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
>   * status internally; used for zerocopy tx only.
>   */
>  /* Lower device DMA failed */
> -#define VHOST_DMA_FAILED_LEN	3
> +#define VHOST_DMA_FAILED_LEN	((__force __virtio32)3)
>  /* Lower device DMA done */
> -#define VHOST_DMA_DONE_LEN	2
> +#define VHOST_DMA_DONE_LEN	((__force __virtio32)2)
>  /* Lower device DMA in progress */
> -#define VHOST_DMA_IN_PROGRESS	1
> +#define VHOST_DMA_IN_PROGRESS	((__force __virtio32)1)
>  /* Buffer unused */
> -#define VHOST_DMA_CLEAR_LEN	0
> +#define VHOST_DMA_CLEAR_LEN	((__force __virtio32)0)

I find these constants a bit confusing: What does __virtio32 mean
without the context of a vq or device?

> 
> -#define VHOST_DMA_IS_DONE(len) ((len) >= VHOST_DMA_DONE_LEN)
> +#define VHOST_DMA_IS_DONE(len) ((__force u32)(len) >= (__force u32)VHOST_DMA_DONE_LEN)

And here you cast it to a plain u32 again.

I looked at the final code, and you seem either to use the above
constants for .len or do a cpu_to_vhost32(). Wouldn't you need to
convert the constants as well?

> 
>  enum {
>  	VHOST_NET_FEATURES = VHOST_FEATURES |

^ permalink raw reply

* Re: [PATCH net] net/mlx4_core: Limit count field to 24 bits in qp_alloc_res
From: Or Gerlitz @ 2014-11-26 14:20 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, matanb, amirv, jackm
In-Reply-To: <20141125.141620.1331667846639491375.davem@davemloft.net>

On 11/25/2014 9:16 PM, David Miller wrote:
> From: Or Gerlitz <ogerlitz@mellanox.com>
> Date: Tue, 25 Nov 2014 11:54:31 +0200
>
>>   	case RES_OP_RESERVE:
>> -		count = get_param_l(&in_param);
>> +		count = get_param_l(&in_param) & 0xffffff;
> I think if these high bits are set, you should be using the maximum
> value rather then truncating.

Dave,

To make it clear (maybe the change-log wasn't explaining it well 
enough), the 32 bits in_param are
divided to 24 bits of count and 8 bits for optimized allocation scheme 
which isn't yet supported
by the Linux PF driver.

Currently, the upstream PF driver is wrongly NOT masking out these eight 
bits and as such, some
VF drivers which are already setting them for optimized allocation are 
failing to allocate QPs as
the count seen by the PF becomes way too large.  The optimization is 
best effort anyway, and hence
we can safely ignore their request.

If we follow your suggestion and allocate to these VFs per the maximum 
number, we will
hit their quota after they open one QP and their over-all bring up will 
fail, so this will not help.

Or.

^ permalink raw reply

* Re: [PATCH v4 26/42] vhost/net: force len for TX to host endian
From: Michael S. Tsirkin @ 2014-11-26 14:44 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: kvm, rusty, netdev, linux-kernel, virtualization, pbonzini,
	David Miller
In-Reply-To: <20141126153102.3f5b8fc9.cornelia.huck@de.ibm.com>

On Wed, Nov 26, 2014 at 03:31:02PM +0100, Cornelia Huck wrote:
> On Tue, 25 Nov 2014 18:43:14 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > We use native endian-ness internally but never
> > expose it to guest.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  drivers/vhost/net.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > index 8dae2f7..dce5c58 100644
> > --- a/drivers/vhost/net.c
> > +++ b/drivers/vhost/net.c
> > @@ -48,15 +48,15 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
> >   * status internally; used for zerocopy tx only.
> >   */
> >  /* Lower device DMA failed */
> > -#define VHOST_DMA_FAILED_LEN	3
> > +#define VHOST_DMA_FAILED_LEN	((__force __virtio32)3)
> >  /* Lower device DMA done */
> > -#define VHOST_DMA_DONE_LEN	2
> > +#define VHOST_DMA_DONE_LEN	((__force __virtio32)2)
> >  /* Lower device DMA in progress */
> > -#define VHOST_DMA_IN_PROGRESS	1
> > +#define VHOST_DMA_IN_PROGRESS	((__force __virtio32)1)
> >  /* Buffer unused */
> > -#define VHOST_DMA_CLEAR_LEN	0
> > +#define VHOST_DMA_CLEAR_LEN	((__force __virtio32)0)
> 
> I find these constants a bit confusing: What does __virtio32 mean
> without the context of a vq or device?
> 
> > 
> > -#define VHOST_DMA_IS_DONE(len) ((len) >= VHOST_DMA_DONE_LEN)
> > +#define VHOST_DMA_IS_DONE(len) ((__force u32)(len) >= (__force u32)VHOST_DMA_DONE_LEN)
> 
> And here you cast it to a plain u32 again.
> 
> I looked at the final code, and you seem either to use the above
> constants for .len or do a cpu_to_vhost32(). Wouldn't you need to
> convert the constants as well?

I tried to explain it in the commit message.
It's a hack in vhost: it keeps virtio used structure in host
memory, but abuses length field for internal housekeeping.
This works because length in used ring for tx is always 0.


> > 
> >  enum {
> >  	VHOST_NET_FEATURES = VHOST_FEATURES |

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox