* Re: [PATCH v4 20/42] virtio_net: pass vi around
From: Cornelia Huck @ 2014-11-26 12:35 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: rusty, netdev, linux-kernel, virtualization, pbonzini,
David Miller
In-Reply-To: <1416933600-21398-21-git-send-email-mst@redhat.com>
On Tue, 25 Nov 2014 18:42:49 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> Too many places poke at [rs]q->vq->vdev->priv just to get
> the the vi structure. Let's just pass the pointer around: seems
s/the the/the/
> cleaner, and might even be faster.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 38 ++++++++++++++++++++------------------
> 1 file changed, 20 insertions(+), 18 deletions(-)
>
Looks reasonable.
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply
* [PATCH 0/5 net] bridge: Fix missing Netlink message validations
From: Thomas Graf @ 2014-11-26 12:42 UTC (permalink / raw)
To: davem; +Cc: stephen, netdev
Adds various missing length checks in the bridging code for Netlink
messages and corresponding attributes provided by user space.
Thomas Graf (5):
bridge: Validate IFLA_BRIDGE_FLAGS attribute length
net: Validate IFLA_BRIDGE_MODE attribute length
net: Check for presence of IFLA_AF_SPEC
bridge: Add missing policy entry for IFLA_BRPORT_FAST_LEAVE
bridge: Sanitize IFLA_EXT_MASK for AF_BRIDGE:RTM_GETLINK
drivers/net/ethernet/emulex/benet/be_main.c | 5 +++++
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +++++
net/bridge/br_netlink.c | 1 +
net/core/rtnetlink.c | 23 ++++++++++++++++++-----
4 files changed, 29 insertions(+), 5 deletions(-)
--
1.9.3
^ permalink raw reply
* [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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox