* Re: [PATCH 0/5 net] bridge: Fix missing Netlink message validations
From: John Fastabend @ 2014-11-26 16:58 UTC (permalink / raw)
To: Thomas Graf, Jiri Pirko; +Cc: davem, stephen, netdev
In-Reply-To: <cover.1417005245.git.tgraf@suug.ch>
On 11/26/2014 04:42 AM, Thomas Graf wrote:
> 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(-)
>
+Jiri
Looks like a miss in bond_netlink also? Seems like writing
a smatch or cocci check for this would be worthwhile.
>
> diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
> index 3e6eebd..7b11243 100644
> --- a/drivers/net/bonding/bond_netlink.c
> +++ b/drivers/net/bonding/bond_netlink.c
> @@ -225,7 +225,12 @@ static int bond_changelink(struct net_device *bond_dev,
>
> bond_option_arp_ip_targets_clear(bond);
> nla_for_each_nested(attr, data[IFLA_BOND_ARP_IP_TARGET], rem) {
> - __be32 target = nla_get_be32(attr);
> + __be32 target;
> +
> + if (nla_len(attr) < sizeof(target))
> + return -EINVAL;
> +
> + target = nla_get_be32(attr);
>
> bond_opt_initval(&newval, (__force u64)target);
> err = __bond_opt_set(bond, BOND_OPT_ARP_TARGETS,
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [PATCH] x86: bpf_jit_comp: simplify trivial boolean return
From: Joe Perches @ 2014-11-26 16:58 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Quentin Lambert, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, x86, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CAADnVQK8mn4ZHwbhBArosEw3Bmsc5vJfOzAeRE3R_1S7dbNfkg@mail.gmail.com>
On Wed, 2014-11-26 at 08:42 -0800, Alexei Starovoitov wrote:
> On Wed, Nov 26, 2014 at 1:18 AM, Quentin Lambert
> <lambert.quentin@gmail.com> wrote:
> > Remove if then else statements preceding
> > boolean return.
[]
> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
[]
> > @@ -135,11 +135,9 @@ static const int reg2hex[] = {
> > */
> > static inline bool is_ereg(u32 reg)
> > {
> > - if (reg == BPF_REG_5 || reg == AUX_REG ||
> > - (reg >= BPF_REG_7 && reg <= BPF_REG_9))
> > - return true;
> > - else
> > - return false;
> > + return (reg == BPF_REG_5 ||
> > + reg == AUX_REG ||
> > + (reg >= BPF_REG_7 && reg <= BPF_REG_9));
>
> please remove extra () around the whole expression, and
> align in properly, and
> don't move reg==AUX_REG check to a different line.
> Subject is not warranted. I don't think it's a simplification.
It's not really a simplification,
gcc should emit the same object code.
> imo existing code is fine and I don't think the time spent
> reviewing such changes is worth it when there is no
> improvement in readability.
Is there any value in reordering these tests for frequency
or maybe using | instead of || to avoid multiple jumps?
^ permalink raw reply
* Re: [PATCH net] r8152: drop the tx packet with invalid length
From: Eric Dumazet @ 2014-11-26 16:52 UTC (permalink / raw)
To: Hayes Wang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-104-Taiwan-albertk@realtek.com>
On Wed, 2014-11-26 at 17:56 +0800, Hayes Wang wrote:
> Drop the tx packet which is more than the size of agg_buf_sz. When
> creating a bridge with the device, we may get the tx packet with
> TSO and the length is more than the gso_max_size which is set by
> the driver through netif_set_gso_max_size(). Such packets couldn't
> be transmitted and should be dropped directly.
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
> drivers/net/usb/r8152.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index c6554c7..ebdaff7 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -1897,6 +1897,15 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
> {
> struct r8152 *tp = netdev_priv(netdev);
>
> + if ((skb->len + sizeof(struct tx_desc)) > agg_buf_sz) {
> + struct net_device_stats *stats = &netdev->stats;
> +
> + dev_kfree_skb_any(skb);
> + stats->tx_dropped++;
> + WARN_ON_ONCE(1);
> + return NETDEV_TX_OK;
> + }
> +
> skb_tx_timestamp(skb);
>
> skb_queue_tail(&tp->tx_queue, skb);
Looks like a candidate for ndo_gso_check(), so that we do not drop, but
instead segment from netif_needs_gso()/validate_xmit_skb()
^ permalink raw reply
* Re: [PATCH 2/5] net: Validate IFLA_BRIDGE_MODE attribute length
From: John Fastabend @ 2014-11-26 16:42 UTC (permalink / raw)
To: Thomas Graf; +Cc: davem, stephen, netdev, Ajit Khaparde, John Fastabend
In-Reply-To: <4a88a0350064b5c2ec4e2adcef5afdfcab3e45dd.1417005245.git.tgraf@suug.ch>
On 11/26/2014 04:42 AM, Thomas Graf 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(+)
>
Thanks Thomas.
Acked-by: John Fastabend <john.r.fastabend@intel.com>
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [PATCH] x86: bpf_jit_comp: simplify trivial boolean return
From: Alexei Starovoitov @ 2014-11-26 16:42 UTC (permalink / raw)
To: Quentin Lambert
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, x86, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
On Wed, Nov 26, 2014 at 1:18 AM, Quentin Lambert
<lambert.quentin@gmail.com> wrote:
> Remove if then else statements preceding
> boolean return. Occurences were found using
> Coccinelle.
>
> The semantic patch used was:
>
> @@
> expression expr;
> @@
>
>
> - if ( expr )
> - return true;
> - else
> - return false;
> + return expr;
>
> Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
>
> ---
> arch/x86/net/bpf_jit_comp.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 3f62734..1542f39 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -135,11 +135,9 @@ static const int reg2hex[] = {
> */
> static inline bool is_ereg(u32 reg)
> {
> - if (reg == BPF_REG_5 || reg == AUX_REG ||
> - (reg >= BPF_REG_7 && reg <= BPF_REG_9))
> - return true;
> - else
> - return false;
> + return (reg == BPF_REG_5 ||
> + reg == AUX_REG ||
> + (reg >= BPF_REG_7 && reg <= BPF_REG_9));
please remove extra () around the whole expression, and
align in properly, and
don't move reg==AUX_REG check to a different line.
Subject is not warranted. I don't think it's a simplification.
imo existing code is fine and I don't think the time spent
reviewing such changes is worth it when there is no
improvement in readability.
^ permalink raw reply
* Re: [patch net-next v3 04/17] net: introduce generic switch devices support
From: Thomas Graf @ 2014-11-26 16:08 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Scott Feldman, Jiri Pirko, Netdev, David S. Miller,
nhorman@tuxdriver.com, Andy Gospodarek, dborkman@redhat.com,
ogerlitz@mellanox.com, jesse@nicira.com, pshelar@nicira.com,
azhou@nicira.com, ben@decadent.org.uk, stephen@networkplumber.org,
Kirsher, Jeffrey T, vyasevic@redhat.com, Cong Wang,
Fastabend, John R, Eric Dumazet, Florian Fainelli, Roopa Prabhu,
John Linville
In-Reply-To: <5475BB53.3070200@mojatatu.com>
On 11/26/14 at 06:36am, Jamal Hadi Salim wrote:
> On 11/25/14 23:18, Scott Feldman wrote:
> >On Tue, Nov 25, 2014 at 5:33 PM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> >
> >You have a pointer to the kernel driver for that HW?
>
> I wasnt sure if that was a passive aggressive move there to
> question what i am claiming?(Only Canadians are allowed to be
> passive aggressive Scott). To answer your question, no
> code currently littered with vendor SDK unfortunately (as you
> would know!).
> But hopefully if we get these changes in correctly it would
> not be hard to show the driver working fully in the kernel.
> There are definetely a few other pieces of hardware that are
> making me come back here and invest time and effort in these
> long discussions.
>
> >Can you show how
> >you're using Linux tc netlink msg in kernel to program HW? I'd like
> >to see the in-kernel API.
> >
>
> Lets do the L2/port thing first. But yes, I am using Linux tc in
> kernel.
Jamal,
What is irriating in this context is that you are pushing back on
Jiri and others while referring to properitary and closed code which
you are unwilling or unable to share. I don't see this as being
passive aggressive, everybody is treated the same way in this regard.
It is exactly the point of this API and related discussions to
decouple the control plane (tc) from any vendor specifics while
allowing them to innovate, compete, and solve different use cases.
I think it's absolutely the right thing to write the API against
code that is public, which in this case is rocker and the existing
in-kernel NIC drivers.
^ permalink raw reply
* Re: [PATCH v5 2/4] arch: Add lightweight memory barriers dma_rmb() and dma_wmb()
From: Will Deacon @ 2014-11-26 16:04 UTC (permalink / raw)
To: Alexander Duyck
Cc: linux-arch@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, mathieu.desnoyers@polymtl.ca,
peterz@infradead.org, benh@kernel.crashing.org,
heiko.carstens@de.ibm.com, mingo@kernel.org, mikey@neuling.org,
linux@arm.linux.org.uk, donald.c.skidmore@intel.com,
matthew.vick@intel.com, geert@linux-m68k.org,
jeffrey.t.kirsher@intel.com, romieu@fr.zoreil.com,
paulmck@linux.vnet.ibm.com,
"nic_swsd@realtek.com" <nic_
In-Reply-To: <5474ADB4.5070200@redhat.com>
On Tue, Nov 25, 2014 at 04:26:28PM +0000, Alexander Duyck wrote:
> On 11/25/2014 06:01 AM, Will Deacon wrote:
> > If we ever see platforms using Linux/dma_alloc_coherent with devices
> > mastering from a different outer-shareable domain that the one containing
> > the CPUs, then we'll need to revisit this.
>
> Would we just need a system wide memory barrier in that case instead of
> an outer shareable memory barrier, or would we need to look as something
> like a sync barrier?
I think dmb(sy) would do the trick, but let's cross that bridge if/when we
have to.
Will
^ permalink raw reply
* Re: [PATCH RFC net-next] net: Add GRO support for GRE tunneling of TEB packets
From: Tom Herbert @ 2014-11-26 15:44 UTC (permalink / raw)
To: Or Gerlitz
Cc: David S. Miller, Linux Netdev List, Eric Dumazet, H.K. Jerry Chu
In-Reply-To: <1417014504-5929-1-git-send-email-ogerlitz@mellanox.com>
On Wed, Nov 26, 2014 at 7:08 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
> Add the missing parts in the gre gro handlers when the inner protocol
> is ETH_P_TEB which is the case for OVS based GRE tunneling.
>
> Cc: H.K. Jerry Chu <hkchu@google.com>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> ---
> net/ipv4/gre_offload.c | 57 +++++++++++++++++++++++++++++++++++++----------
> 1 files changed, 45 insertions(+), 12 deletions(-)
>
> diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
> index bb5947b..06ae197 100644
> --- a/net/ipv4/gre_offload.c
> +++ b/net/ipv4/gre_offload.c
> @@ -14,6 +14,7 @@
> #include <linux/init.h>
> #include <net/protocol.h>
> #include <net/gre.h>
> +#include <linux/etherdevice.h>
>
> static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
> netdev_features_t features)
> @@ -121,8 +122,9 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head,
> struct sk_buff **pp = NULL;
> struct sk_buff *p;
> const struct gre_base_hdr *greh;
> + struct ethhdr *eh = NULL;
> unsigned int hlen, grehlen;
> - unsigned int off;
> + unsigned int off, off_eth = 0;
> int flush = 1;
> struct packet_offload *ptype;
> __be16 type;
> @@ -147,11 +149,6 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head,
>
> type = greh->protocol;
>
> - rcu_read_lock();
> - ptype = gro_find_receive_by_type(type);
> - if (ptype == NULL)
> - goto out_unlock;
> -
> grehlen = GRE_HEADER_SECTION;
>
> if (greh->flags & GRE_KEY)
> @@ -164,22 +161,45 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head,
> if (skb_gro_header_hard(skb, hlen)) {
> greh = skb_gro_header_slow(skb, hlen, off);
> if (unlikely(!greh))
> - goto out_unlock;
> + goto out;
> }
>
> /* Don't bother verifying checksum if we're going to flush anyway. */
> if ((greh->flags & GRE_CSUM) && !NAPI_GRO_CB(skb)->flush) {
> if (skb_gro_checksum_simple_validate(skb))
> - goto out_unlock;
> + goto out;
>
> skb_gro_checksum_try_convert(skb, IPPROTO_GRE, 0,
> null_compute_pseudo);
> }
>
> + skb_gro_pull(skb, grehlen);
> +
> + /* Adjusted NAPI_GRO_CB(skb)->csum after skb_gro_pull()*/
> + skb_gro_postpull_rcsum(skb, greh, grehlen);
> +
> + if (type == ntohs(ETH_P_TEB)) {
> + off_eth = skb_gro_offset(skb);
> + hlen = off_eth + sizeof(*eh);
> + eh = skb_gro_header_fast(skb, off_eth);
> + if (skb_gro_header_hard(skb, hlen)) {
> + eh = skb_gro_header_slow(skb, hlen, off_eth);
> + if (unlikely(!eh))
> + goto out;
> + }
> + type = eh->h_proto;
I don't think this is the right approach. It would probably be better
to a add a gro_receive handler for ETH_P_TEB and then you wouldn't
need to modify GRE path with special case code. That would also be
applicable in geneve.
> + }
> +
> + rcu_read_lock();
> + ptype = gro_find_receive_by_type(type);
> + if (ptype == NULL)
> + goto out_unlock;
> +
> flush = 0;
>
> for (p = *head; p; p = p->next) {
> const struct gre_base_hdr *greh2;
> + const struct ethhdr *eh2;
>
> if (!NAPI_GRO_CB(p)->same_flow)
> continue;
> @@ -205,13 +225,19 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head,
> NAPI_GRO_CB(p)->same_flow = 0;
> continue;
> }
> +
> + eh2 = (struct ethhdr *)(p->data + off_eth);
> + if (eh && compare_ether_header(eh, eh2)) {
> + NAPI_GRO_CB(p)->same_flow = 0;
> + continue;
> + }
> }
> }
>
> - skb_gro_pull(skb, grehlen);
> -
> - /* Adjusted NAPI_GRO_CB(skb)->csum after skb_gro_pull()*/
> - skb_gro_postpull_rcsum(skb, greh, grehlen);
> + if (eh) {
> + skb_gro_pull(skb, sizeof(*eh)); /* pull inner eth header */
> + skb_gro_postpull_rcsum(skb, eh, sizeof(*eh));
> + }
>
> pp = ptype->callbacks.gro_receive(head, skb);
>
> @@ -229,6 +255,7 @@ static int gre_gro_complete(struct sk_buff *skb, int nhoff)
> struct packet_offload *ptype;
> unsigned int grehlen = sizeof(*greh);
> int err = -ENOENT;
> + struct ethhdr *eh;
> __be16 type;
>
> skb->encapsulation = 1;
> @@ -241,6 +268,12 @@ static int gre_gro_complete(struct sk_buff *skb, int nhoff)
> if (greh->flags & GRE_CSUM)
> grehlen += GRE_HEADER_SECTION;
>
> + if (type == ntohs(ETH_P_TEB)) {
> + eh = (struct ethhdr *)(skb->data + nhoff + grehlen);
> + type = eh->h_proto;
> + grehlen += sizeof(*eh);
> + }
> +
> rcu_read_lock();
> ptype = gro_find_complete_by_type(type);
> if (ptype != NULL)
> --
> 1.7.1
>
^ permalink raw reply
* [PATCH RFC net-next] net: Add GRO support for GRE tunneling of TEB packets
From: Or Gerlitz @ 2014-11-26 15:08 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, therbert, edumazet, Or Gerlitz, H.K. Jerry Chu
Add the missing parts in the gre gro handlers when the inner protocol
is ETH_P_TEB which is the case for OVS based GRE tunneling.
Cc: H.K. Jerry Chu <hkchu@google.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
net/ipv4/gre_offload.c | 57 +++++++++++++++++++++++++++++++++++++----------
1 files changed, 45 insertions(+), 12 deletions(-)
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index bb5947b..06ae197 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -14,6 +14,7 @@
#include <linux/init.h>
#include <net/protocol.h>
#include <net/gre.h>
+#include <linux/etherdevice.h>
static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
netdev_features_t features)
@@ -121,8 +122,9 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head,
struct sk_buff **pp = NULL;
struct sk_buff *p;
const struct gre_base_hdr *greh;
+ struct ethhdr *eh = NULL;
unsigned int hlen, grehlen;
- unsigned int off;
+ unsigned int off, off_eth = 0;
int flush = 1;
struct packet_offload *ptype;
__be16 type;
@@ -147,11 +149,6 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head,
type = greh->protocol;
- rcu_read_lock();
- ptype = gro_find_receive_by_type(type);
- if (ptype == NULL)
- goto out_unlock;
-
grehlen = GRE_HEADER_SECTION;
if (greh->flags & GRE_KEY)
@@ -164,22 +161,45 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head,
if (skb_gro_header_hard(skb, hlen)) {
greh = skb_gro_header_slow(skb, hlen, off);
if (unlikely(!greh))
- goto out_unlock;
+ goto out;
}
/* Don't bother verifying checksum if we're going to flush anyway. */
if ((greh->flags & GRE_CSUM) && !NAPI_GRO_CB(skb)->flush) {
if (skb_gro_checksum_simple_validate(skb))
- goto out_unlock;
+ goto out;
skb_gro_checksum_try_convert(skb, IPPROTO_GRE, 0,
null_compute_pseudo);
}
+ skb_gro_pull(skb, grehlen);
+
+ /* Adjusted NAPI_GRO_CB(skb)->csum after skb_gro_pull()*/
+ skb_gro_postpull_rcsum(skb, greh, grehlen);
+
+ if (type == ntohs(ETH_P_TEB)) {
+ off_eth = skb_gro_offset(skb);
+ hlen = off_eth + sizeof(*eh);
+ eh = skb_gro_header_fast(skb, off_eth);
+ if (skb_gro_header_hard(skb, hlen)) {
+ eh = skb_gro_header_slow(skb, hlen, off_eth);
+ if (unlikely(!eh))
+ goto out;
+ }
+ type = eh->h_proto;
+ }
+
+ rcu_read_lock();
+ ptype = gro_find_receive_by_type(type);
+ if (ptype == NULL)
+ goto out_unlock;
+
flush = 0;
for (p = *head; p; p = p->next) {
const struct gre_base_hdr *greh2;
+ const struct ethhdr *eh2;
if (!NAPI_GRO_CB(p)->same_flow)
continue;
@@ -205,13 +225,19 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head,
NAPI_GRO_CB(p)->same_flow = 0;
continue;
}
+
+ eh2 = (struct ethhdr *)(p->data + off_eth);
+ if (eh && compare_ether_header(eh, eh2)) {
+ NAPI_GRO_CB(p)->same_flow = 0;
+ continue;
+ }
}
}
- skb_gro_pull(skb, grehlen);
-
- /* Adjusted NAPI_GRO_CB(skb)->csum after skb_gro_pull()*/
- skb_gro_postpull_rcsum(skb, greh, grehlen);
+ if (eh) {
+ skb_gro_pull(skb, sizeof(*eh)); /* pull inner eth header */
+ skb_gro_postpull_rcsum(skb, eh, sizeof(*eh));
+ }
pp = ptype->callbacks.gro_receive(head, skb);
@@ -229,6 +255,7 @@ static int gre_gro_complete(struct sk_buff *skb, int nhoff)
struct packet_offload *ptype;
unsigned int grehlen = sizeof(*greh);
int err = -ENOENT;
+ struct ethhdr *eh;
__be16 type;
skb->encapsulation = 1;
@@ -241,6 +268,12 @@ static int gre_gro_complete(struct sk_buff *skb, int nhoff)
if (greh->flags & GRE_CSUM)
grehlen += GRE_HEADER_SECTION;
+ if (type == ntohs(ETH_P_TEB)) {
+ eh = (struct ethhdr *)(skb->data + nhoff + grehlen);
+ type = eh->h_proto;
+ grehlen += sizeof(*eh);
+ }
+
rcu_read_lock();
ptype = gro_find_complete_by_type(type);
if (ptype != NULL)
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v4 26/42] vhost/net: force len for TX to host endian
From: Michael S. Tsirkin @ 2014-11-26 15:01 UTC (permalink / raw)
To: Cornelia Huck
Cc: kvm, rusty, netdev, linux-kernel, virtualization, pbonzini,
David Miller
In-Reply-To: <20141126155440.4f49b70f.cornelia.huck@de.ibm.com>
On Wed, Nov 26, 2014 at 03:54:40PM +0100, Cornelia Huck wrote:
> On Wed, 26 Nov 2014 16:44:00 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > 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.
>
> Ah, ok. It might make sense to add this explanation to the patch :)
Absolutely.
^ permalink raw reply
* Re: [PATCH v4 26/42] vhost/net: force len for TX to host endian
From: Cornelia Huck @ 2014-11-26 14:54 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm, rusty, netdev, linux-kernel, virtualization, pbonzini,
David Miller
In-Reply-To: <20141126144400.GA8086@redhat.com>
On Wed, 26 Nov 2014 16:44:00 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> 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.
Ah, ok. It might make sense to add this explanation to the patch :)
^ 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
* 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: 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 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 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
* [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: 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
* 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: 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
* [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 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
* 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 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 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
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