* Re: [RFC PATCH 00/29] net: VRF support
From: David Ahern @ 2015-02-09 0:36 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Stephen Hemminger, netdev, Nicolas Dichtel, roopa, hannes,
Dinesh Dutt, Vipin Kumar, Shmulik Ladkani
In-Reply-To: <87k2zubw7l.fsf@x220.int.ebiederm.org>
On 2/6/15 1:50 PM, Eric W. Biederman wrote:
>
> David looking at your patches and reading through your code I think I
> understand what you are proposing, let me see if I can sum it up.
>
> Semantics:
> - The same as a network namespace.
> - Addition of a VRF-ANY context
> - No per VRF settings
> - No creation or removal operation.
Yes. Plus what I see is an important feature: the ability to layer VRFs
and network namespaces.
Network namespaces can be used to create smaller, logical switches
within a single physical switch. (i.e., A network namespace is on par
with what Cisco calls a VDC, virtual device context, for its Nexus 7000
switches -- logical separation at the device layer front panel port).
Layering VRFs (L3 separation) within a network namespace (L1 separation)
provides some nice end-user features.
>
> Implementation:
> - Add a VRF-id to every affected data structure.
>
> This implies that you see the current implementation of network
> namespaces to be space inefficient, and that you think you can remove
> this inefficiency by simply removing the settings and the associated
> proc files.
Not exactly. I see the current namespace implementation as an excellent
L1 separation construct, but not an L3 construct.
>
> Given that you have chosen to keep the same semantics as network
> namespaces for everything except for settings this raises the questions:
> - Are the settings and their associated proc files what actually cause
> the size cost you see in network namespaces?
> - Can we instead of reimplementing network namespaces instead optimize
> the current implementation?
The namespace memory consumption is side problem to the bigger problem
of how the isolation of namespaces affect processes (the need to have a
presence in each namespace).
What I was targeting is a trade-off. To make an L3 separation efficient
from a large scale* perspective one needs to give up something -- here
it is per-VRF procfs settings. Replicating procfs tree for each
namespace does have a high cost.
* scale here meaning VRFs from 1 to N, N for current products goes up to
4000, though I know of that has mentioned 16k VRFs.
>
> We need measurements to answer either of those questions and I think
> before proceeding we need to answer those questions.
agreed.
> Beyond that I want to point out that in general a data structure that
> has a tag on every member is going to have a larger memory foot print
> per entry, contain more entries, and by virtue of both of those be less
> memory efficient and less time efficient to use. So it is not clear
> that a implementation that tags everything with a vrf-id will actually
> be lighter weight.
The memory hit for a network namespace is >100k (yes, CONFIG option
dependent and that 100k is based on a 3.10 kernel which is higher than
what was measured for a 3.4 kernel).
This proposal puts a 4-byte tag to netdevices, sockets and tasks (skb's
are out per a prior email). So yes there will be a point that the number
of netdevices (logical + physical), plus tasks, and sockets will make
the memory hit of a VRF tag on par with namespace overhead. But the VRF
tagging alleviates the need to replicate processes/multiple
sockets/threads so in the big picture I can;t see how the overall hit to
memory is higher with a VRF id tag.
>
> Also there is a concern that placing tags in every data structure may be
> significantly more error prone to implement (as it is more more thing to
> keep trace of), and that can impact the maintainability and the
> correctness of the code for everyone.
I don't agree with this. You have already done the groundwork here by
plumbing through the namespace checks. Adding a vrf id has not proven to
be a huge problem. The patch changes are highly repetitive because again
it can leverage the namespace changes you have done.
> The standard that was applied to the network namespace was that
> it did not have any measurable performance impact when enabled. The
> measurments taken at the time did not show a slow down when a 1Gig
> interface was place in a network namespace. Compared to running an
> unpatched kernel.
Sure. I will build kernels at the commit id my patches are based on and
one with my changes and do a comparison. Virtual machines on KVM
emphasize the performance effects so I will compare a few netperf runs
with and without my changes. On a newer 3.x kernel I typically see
network throughput rates in 15 to 16 Gbps range (though H/W dependent),
so this far exceeds the 1G rate. Does that sound reasonable?
>
> I suspect your extra layer of indirection to get to struct net in
> addition to touching struct skb will give you a noticable performance
> impact.
I don't understand the 'extra layer of indirection' comment. I don't see
the indirection, I see an extra comparison. ie., from net_eq to net_eq +
(vrf_eq || vrf_eq_any).
From a struct comparison it has gone from:
struct net_device {
...
struct net *nd_net;
...
};
to
struct net_device {
...
struct net_ctx net_ctx {
struct net *net;
__u32 vrf;
};
...
};
>
>
> I have another concern. I don't think it is wise to have a data
> structure modified two different ways to deal with network namespaces
> and vrfs. For maintainability and our own sanity we should pick which
> version that we judge to be the most efficient implementation and go
> with it.
you lost me. What data structure is modified 2 different ways? VRFs are
sub context to a namespace.
>
>
>
> The architecture I imagine for using network namespaces as vrfs for
> devices that perform layer 2 bridging and layer 3 routing.
>
> port1 port2 port3 port4 port5 port6 port7 port8 port9 port10
> | | | | | | | | | |
> +-----+-----+-----+-----+-----+-----+-----+-----+-----+
> / Link Aggregation \
> + +
> | Bridging |
> +----------------------------+----------------------------+
> |
> cpu port
> |
> +---------------------+---------------------+
> / +---------------/ \---------------+ \
> / / +---------/ \---------+ \ \
> / / / +---/ \---+ \ \ \
> / / / / | | \ \ \ \
> | | | | | | | | | |
> vlan1 vlan2 vlan3 vlan4 vlan5 vlan6 vlan7 vlan8 vlan9 vlan10
> | | | | | | | | | |
> +-+-----+-----+-----+-----+-+ +-+-----+-----+-----+-----+-+
> | network namespace 1 | | network namespace2 |
> +---------------------------+ +---------------------------+
>
> Traffic to and from the rest of the world comes through the
> external ports.
>
> The traffic is then processed at layer two including link
> aggregation, bridging and classifying which vlan the traffic
> belongs in.
>
> If the traffic needs to be routed it then comes up to the cpu port.
> The cpu port looks at the tags on the traffic and places it into
> the appropriate vlan device.
>
> From the various vlans the traffic is then routed according
> to the routing table of whichever network namespace the vlan
> device is in.
>
> There are stateless offloads to this in modern hardware but this is a
> reasonable model how all of this works semantically.
>
> As such the vlan devices can be moved between network namespaces without
> affecting any layer two monitoring or work that happens on the lower
> level devices. The practical restriction is that L2 and L3 need to be
> handled on different network devices.
>
> This split of network devices ensures that L2 code that works today
> should not need any changes or in any way be concerned about network
> namespaces or that the parent devices are in.
9+ months ago I had considered something similar. I'll try to amend your
picture to show my concept:
port1 port2 port3 port4 port5 port6 port7 port8 port9 port10
| | | | | | | | | |
+-----+-----+-----+-----+-----+-----+-----+-----+-----+
/ Link Aggregation \
+ +
| Bridging |
+-----------------------------+---------------------------+
|
+-----------------------------+----------------------------+
|default namespace | |
| (init_net) NIC driver |
| | |
| +----+----+-----+-----+-------+----+----+----+----+ |
| eth1 eth2 eth3 eth4 eth5 eth6 eth7 eth8 eth9 eth10 |
| | | | | | | | | | | |
+-----------------------------+----------------------------+
| | | | | | | | | |
+--+----+----+-----+-----+---+ +--+----+----+----+----+----+
| | | | | | | | | | | | | |
| seth1 | seth3 | seth5 | | seth6 | seth8 | seth10|
| seth2 seth4 | | seth7 seth9 |
| | | |
| network namespace 1 | | network namespace 2 |
+----------------------------+ +---------------------------+
Essentially netdevices for front panel ports exist in the default
namespace (init_net). L2 processes, monitoring processes (collectd, snmp
agents for devices, etc) and such would run here. From there "shadow
devices" (the 's' on the eth pairs) are created for namespaces where the
path between real and shadow is similar to how veth pairs work. In the
end this approach seemed to be a rather complex solution playing a lot
of games so I abandoned it in favor of the approach in this patch set --
adding a VRF id to a network context.
The patch diff might be large but almost all of it is converting the
existing struct net passing and net_eq checks to the broader struct
net_ctx and net_ctx_eq comparisons. Really the change rides on top of
what you have done for namespaces.
As for your proposal with VLAN based tagging, I do not understand the
packet path from driver for front panel ports to namespace based
netdevices. The VLAN sorting, and hence VRF sorting, is done in H/W? So
there are netdevices in init_net the driver uses and then VLAN devices
in the namespaces -- so those would correspond to what I called a shadow
device? If the packets are also VLAN tagged we have nested tagging --
one for the port and one for the VRF?
Also, doesn't the VLAN design limit number of VRFs to 4096? My current
patch set might limit it to 4096 but fix the genid piece (IPv6 seems to
have removed genid comparisons betweeen 3.17 and 3.19 -- need to look
into that) and it becomes a 32-bit tag which is a huge range for VRFs.
David
^ permalink raw reply
* Re: [PATCH net-next] net: silence sparse endianness warnings in checksums
From: David Miller @ 2015-02-09 0:30 UTC (permalink / raw)
To: sd; +Cc: netdev, eric.dumazet, therbert
In-Reply-To: <1423233832-8229-1-git-send-email-sd@queasysnail.net>
From: Sabrina Dubroca <sd@queasysnail.net>
Date: Fri, 6 Feb 2015 15:43:52 +0100
> In __skb_checksum_validate_complete and its callers, we only test that
> the return value is non-zero, so it's safe to __force the type.
>
> Callers of gso_make_checksum pass a __sum16, use that.
>
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
I don't like this, you're just moving the casts from one place to
another.
These functions are a mess, they are combining one thing (returning
the final csum value) with another thing (non-zero return has some
boolean meaning).
These issues should be explicitly accomodated rather than papered
around.
^ permalink raw reply
* Re: [PATCH net-next] net: fix a typo in skb_checksum_validate_zero_check
From: David Miller @ 2015-02-09 0:29 UTC (permalink / raw)
To: sd; +Cc: netdev
In-Reply-To: <1423245259-10369-1-git-send-email-sd@queasysnail.net>
From: Sabrina Dubroca <sd@queasysnail.net>
Date: Fri, 6 Feb 2015 18:54:19 +0100
> Remove trailing underscore.
>
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] gre/ipip: use be16 variants of netlink functions
From: David Miller @ 2015-02-09 0:28 UTC (permalink / raw)
To: sd; +Cc: netdev
In-Reply-To: <1423239742-30865-1-git-send-email-sd@queasysnail.net>
From: Sabrina Dubroca <sd@queasysnail.net>
Date: Fri, 6 Feb 2015 17:22:22 +0100
> encap.sport and encap.dport are __be16, use nla_{get,put}_be16 instead
> of nla_{get,put}_u16.
>
> Fixes the sparse warnings:
...
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Applied.
^ permalink raw reply
* Re: Kill I4L?
From: Karsten Keil @ 2015-02-08 23:59 UTC (permalink / raw)
To: Tilman Schmidt, Paul Bolle, Joe Perches
Cc: Sergei Shtylyov, Bas Peters, isdn, julia.lawall, davem, netdev,
linux-kernel
In-Reply-To: <54D7BD4A.4090101@imap.cc>
Am 08.02.2015 um 20:47 schrieb Tilman Schmidt:
> Am 07.02.2015 um 21:43 schrieb Paul Bolle:
>> On Sat, 2015-02-07 at 11:19 -0800, Joe Perches wrote:
>>> Does anyone still use these cards?
>
>> 0) Good question.
>
> I very much doubt it. It was an ISA card, not even PnP. I'd imagine
> any systems of that kind would be retired by now.
>
Agreed in general, but I know people still running such ISA based
machines - mostly PC in industrial environments with ISDN uplinks for
service and maintenance.
And I4L is also used in some vending machines for card authorization so
far I know.
They usually using old kernels (<3.0) so they are not directly affected
if this will be removed.
>> 2) Broader picture: if I remember correctly there are now four
>> different flavors of ISDN in the kernel: - really old: pre-i4l
>
> I don't think any traces of that are still present in current kernel
> releases. It should all have been left behind on the switch to 2.6.
>
Yes.
>> - very old: i4l - just old: CAPI - not so old: mISDN
>
> Those are the in-tree ones. To make matters worse, the Asterisk people
> invented three more (Zaptel, DAHDI and vISDN) which are maintained
> out-of-tree. Apparently they weren't satisfied with any the in-tree
> ones and didn't feel like helping to improve one of them to match
> their needs.
>
>> [snip] So the current ISDN situation is a bit messy.
>
> That's putting it mildly.
>
>> Tilman might be able to provide a clearer, and maybe less grumpy,
>> summary of the current situation.
>
> Don't know about either. ;-)
>
>> [M]aybe we should consider, say, removing i4l and pre i4l and see
>> who complains. That might be a rude thing to do. So perhaps the
>> various ISDN flavors should be left alone until ... what exactly?
>
> I'd support that step. I don't think it'll hurt anyone because the
> cards supported by i4l are mostly ISA cards anyway. The only exceptions
> are the HiSax family which is now supported by mISDN, and the Hypercope
> family which is supported by CAPI.
>
> In the past we had Documentation/feature-removal-schedule.txt for
> announcing removals like that but Linus shot that down on 2012-10-01
> (commit 9c0ece069b32e8e122aea71aa47181c10eb85ba7) so I guess
> somebody could just submit a patch series starting with
>
> --- a/drivers/isdn/Kconfig
> +++ b/drivers/isdn/Kconfig
> @@ -20,25 +20,6 @@ menuconfig ISDN
>
> if ISDN
>
> -menuconfig ISDN_I4L
> - tristate "Old ISDN4Linux (deprecated)"
> - depends on TTY
> - ---help---
> - This driver allows you to use an ISDN adapter for networking
> - connections and as dialin/out device. The isdn-tty's have a
> built
> - in AT-compatible modem emulator. Network devices support
> autodial,
> - channel-bundling, callback and caller-authentication without
> having
> - a daemon running. A reduced T.70 protocol is supported with
> tty's
> - suitable for German BTX. On D-Channel, the protocols EDSS1
> - (Euro-ISDN) and 1TR6 (German style) are supported. See
> - <file:Documentation/isdn/README> for more information.
> -
> - ISDN support in the linux kernel is moving towards a new API,
> - called CAPI (Common ISDN Application Programming Interface).
> - Therefore the old ISDN4Linux layer will eventually become
> obsolete.
> - It is still available, though, for use with adapters that
> are not
> - supported by the new CAPI subsystem yet.
> -
> source "drivers/isdn/i4l/Kconfig"
>
> menuconfig ISDN_CAPI
>
> and working its way from that to remove anything that's become
> unreachable.
>
> Shall I?
>
But I4L is still the default in some Distros, so we should allow a
warning period. But again, I'm fine with this to do it.
Karsten
^ permalink raw reply
* Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.
From: Rusty Russell @ 2015-02-08 23:50 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: lkml, netdev@vger.kernel.org
In-Reply-To: <20150208105908.GI3185@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Fri, Feb 06, 2015 at 03:36:54PM +1030, Rusty Russell wrote:
>> In particular, the virtio header always has the u16 num_buffers field.
>> We define a new 'struct virtio_net_modern_hdr' for this (rather than
>> simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
>> if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
>>
>> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>> ---
>> include/uapi/linux/virtio_net.h | 30 ++++++++++++++++++++++++++++--
>> 1 file changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
>> index b5f1677b291c..32754f3000e8 100644
>> --- a/include/uapi/linux/virtio_net.h
>> +++ b/include/uapi/linux/virtio_net.h
>> @@ -35,7 +35,6 @@
>> #define VIRTIO_NET_F_CSUM 0 /* Host handles pkts w/ partial csum */
>> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */
>> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */
>> -#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
>> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */
>> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */
>> #define VIRTIO_NET_F_GUEST_ECN 9 /* Guest can handle TSO[6] w/ ECN in. */
>> @@ -56,6 +55,10 @@
>> * Steering */
>> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
>>
>> +#ifndef VIRTIO_NET_NO_LEGACY
>> +#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
>> +#endif /* VIRTIO_NET_NO_LEGACY */
>> +
>> #define VIRTIO_NET_S_LINK_UP 1 /* Link is up */
>> #define VIRTIO_NET_S_ANNOUNCE 2 /* Announcement is needed */
>>
>> @@ -71,8 +74,9 @@ struct virtio_net_config {
>> __u16 max_virtqueue_pairs;
>> } __attribute__((packed));
>>
>> +#ifndef VIRTIO_NET_NO_LEGACY
>> /* This header comes first in the scatter-gather list.
>> - * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
>> + * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
>> * be the first element of the scatter-gather list. If you don't
>> * specify GSO or CSUM features, you can simply ignore the header. */
>> struct virtio_net_hdr {
>> @@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
>> struct virtio_net_hdr hdr;
>> __virtio16 num_buffers; /* Number of merged rx buffers */
>> };
>> +#else /* ... VIRTIO_NET_NO_LEGACY */
>> +/*
>> + * This header comes first in the scatter-gather list. If you don't
>> + * specify GSO or CSUM features, you can simply ignore the header.
>> + */
>> +struct virtio_net_modern_hdr {
>> +#define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */
>> +#define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */
>> + __u8 flags;
>> +#define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */
>> +#define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */
>> +#define VIRTIO_NET_HDR_GSO_UDP 3 /* GSO frame, IPv4 UDP (UFO) */
>> +#define VIRTIO_NET_HDR_GSO_TCPV6 4 /* GSO frame, IPv6 TCP */
>> +#define VIRTIO_NET_HDR_GSO_ECN 0x80 /* TCP has ECN set */
>> + __u8 gso_type;
>> + __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
>> + __virtio16 gso_size; /* Bytes to append to hdr_len per frame */
>> + __virtio16 csum_start; /* Position to start checksumming from */
>> + __virtio16 csum_offset; /* Offset after that to place checksum */
>> + __virtio16 num_buffers; /* Number of merged rx buffers */
>> +};
>> +#endif /* ...VIRTIO_NET_NO_LEGACY */
>
> This kind of masks the fact that it's the same as
> virtio_net_hdr_mrg_rxbuf. So it's forcing people to duplicate
> code for transitional devices.
It's a hard problem to solve; someone suffers. My preference is to make
non-legacy implementations as "clean" as possible, even if transitional
devices suffer (since their suffering should be shorter-lived).
Transitional devices can't define VIRTIO_NET_NO_LEGACY, so they'll
have to use virtio_net_hdr_mrg_rxbuf anyway.
> How about
> struct virtio_net_modern_hdr {
> struct virtio_net_hdr_mrg_rxbuf hdr;
> }
But modern code now has refer to a field like this:
hdr.hdr.hdr.flags
> This will also make it look nicer when we start
> adding stuff in the header, the main header
> is separated in a struct by its own, so it's
> easy to apply operations such as sizeof.
net is the only header we've extended, and I'm not aware of any plans to
extend again. If we did, we could wrap the header to make it a bit more
awkward to use, but easier for next time, but that won't help for the
time *after* that.
We might rename struct virtio_net_modern_hdr to struct virtio_net_hdr_v1
though (which makes future naming easier).
Cheers,
Rusty.
^ permalink raw reply
* Re: DSA: Attaching phy twice?
From: Florian Fainelli @ 2015-02-08 22:59 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20150208203211.GB23581@lunn.ch>
Hi Andrew,
Le 08/02/2015 12:32, Andrew Lunn a écrit :
> Hi Florian
>
> Have you seen messages like this before?
Yes :)
>
> [ 2.495126] Distributed Switch Architecture driver version 0.1
> [ 2.501358] mvneta f1070000.ethernet eth0: [0]: detected a Marvell 88E6172 switch
> [ 2.556441] libphy: dsa slave smi: probed
> [ 2.638292] net lan4: PHY already attached
> [ 2.733285] net lan3: PHY already attached
> [ 2.820749] net lan2: PHY already attached
> [ 2.910749] net lan1: PHY already attached
> [ 3.000772] net internet: PHY already attached
>
> What appears to be happening is that dsa_slave_phy_setup() is finding
> the phy for the port via device tree and using of_phy_connect(), or it
> uses the fall back of taking a phy from the switch internal mdio bus
> and calling phy_connect_direct(). So if a phy is found,
> phy_attach_direct() gets called to attach the phy to the device.
>
> Then in dsa_slave_create(), a second call to phy_attach() is
> made. This is when we get the warning "PHY already attached", and it
> returns EBUSY, which is ignored.
>
> Is this code at the end of dsa_slave_create() doing anything useful?
>
> if (p->phy != NULL) {
> if (ds->drv->get_phy_flags)
> p->phy->dev_flags |= ds->drv->get_phy_flags(ds, port);
>
> phy_attach(slave_dev, dev_name(&p->phy->dev),
> PHY_INTERFACE_MODE_GMII);
>
> p->phy->autoneg = AUTONEG_ENABLE;
> p->phy->speed = 0;
> p->phy->duplex = 0;
> p->phy->advertising = p->phy->supported | ADVERTISED_Autoneg;
> }
>
> My quick testing suggests its not useful, so if you agree, i will
> submit a patch removing it. Or am i missing something?
Right, now that we have a call to phy_connect_direct() for the built-in
MDIO bus of the switch, we can remove that code, as it is indeed
redundant (note that it was not before, especially for !OF setups). I
would be happy to ack such a patch and give it a try here as well (with
bcm_sf2).
Thanks!
--
Florian
^ permalink raw reply
* Re: nft hash set expansion problem
From: Daniel Borkmann @ 2015-02-08 22:43 UTC (permalink / raw)
To: Josh Hunt; +Cc: Thomas Graf, Pablo Neira Ayuso, kaber, netdev, netfilter-devel
In-Reply-To: <54D7BB53.2050203@akamai.com>
On 02/08/2015 08:38 PM, Josh Hunt wrote:
> Nft hash sets are unable to expand past the initial # of buckets. This is b/c nft hash sets don't define the max_shift parameter and so rht_grow_above_75():
...
> diff --git a/lib/rhashtable.c b/lib/rhashtable.c
> index e96fc00..2c51617 100644
> --- a/lib/rhashtable.c
> +++ b/lib/rhashtable.c
> @@ -250,7 +250,7 @@ bool rht_grow_above_75(const struct rhashtable *ht, size_t new_size)
> {
> /* Expand table when exceeding 75% load */
> return atomic_read(&ht->nelems) > (new_size / 4 * 3) &&
> - (ht->p.max_shift && atomic_read(&ht->shift) < ht->p.max_shift);
> + (ht->p.max_shift ? atomic_read(&ht->shift) < ht->p.max_shift : 1);
> }
> EXPORT_SYMBOL_GPL(rht_grow_above_75);
This seems not correct as we want to have an upper limit for
rhashtable expansions. It's better to define a max_shift for
nftables, instead.
^ permalink raw reply
* Re: Implementing Deficit Round Robin for different irtual interfaces
From: Ronald Pina @ 2015-02-08 21:14 UTC (permalink / raw)
To: Dave Taht; +Cc: netdev
In-Reply-To: <CAA93jw5W+WwnWUWddksYSiuoVFkDvMCo3Vw6aiLK2Z3L4QTkGw@mail.gmail.com>
Hi Dave
It is done a great work especially on bufferbload problem . On my work
i am trying to use this scheduling algorithm to schedule the virtual
network interfaces on Xen Hypervisor Dom0, maybe there are better
algorithm than DRR like DRR++ or the algorithm that you referenced,
but since it is a study work i would like to test the performance of
DRR and later can chose a better scheduling algorithm. The question is
that since tc tools in Linux are only for one device while on Xen the
traffic must be scheduled from different domains netif devices what is
the function or the module responsible for making scheduling
decisions?
A similar idea is like using a qdisc to represent an netif network
virtual interface of guest on dom0.
On Sat, Feb 7, 2015 at 1:24 PM, Dave Taht <dave.taht@gmail.com> wrote:
> we have something better than drr already in the kernel.
>
> https://tools.ietf.org/html/draft-hoeiland-joergensen-aqm-fq-codel-01
>
> see also sch_fq
>
> as for fq over virtual devices....
>
> see recent posts on improving RPS steering.
>
> and regrettably FQ ignores cache locality and the cost of a context
> switch when taken too far. some batching (tso/gro/gso) is indeed
> required for a good compromise between latency and throughput.
>
> http://www.bufferbloat.net/projects/cerowrt/wiki/Bloat-videos#Dave-Taumlhts-Water-Videos-November-2012
> --
> Dave Täht
>
> http://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks
^ permalink raw reply
* Re: [PATCH net-next 1/1] tipc: fix bug in socket reception function
From: David Miller @ 2015-02-08 21:09 UTC (permalink / raw)
To: jon.maloy
Cc: netdev, paul.gortmaker, erik.hugne, ying.xue, maloy,
tipc-discussion
In-Reply-To: <1423411850-2594-1-git-send-email-jon.maloy@ericsson.com>
From: Jon Maloy <jon.maloy@ericsson.com>
Date: Sun, 8 Feb 2015 11:10:50 -0500
> In commit c637c1035534867b85b78b453c38c495b58e2c5a ("tipc: resolve race
> problem at unicast message reception") we introduced a time limit
> for how long the function tipc_sk_eneque() would be allowed to execute
> its loop. Unfortunately, the test for when this limit is passed was put
> in the wrong place, resulting in a lost message when the test is true.
>
> We fix this by moving the test to before we dequeue the next buffer
> from the input queue.
>
> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Applied, thanks Jon.
^ permalink raw reply
* Re: [PATCH] rt6_probe_deferred: Do not depend on struct ordering
From: David Miller @ 2015-02-08 21:03 UTC (permalink / raw)
To: m; +Cc: netdev, hannes
In-Reply-To: <20150208101407.517bcfa7@wiggum>
From: Michael Büsch <m@bues.ch>
Date: Sun, 8 Feb 2015 10:14:07 +0100
> rt6_probe allocates a struct __rt6_probe_work and schedules a work handler rt6_probe_deferred.
> But rt6_probe_deferred kfree's the struct work_struct instead of struct __rt6_probe_work.
> This works, because struct work_struct is the first element of struct __rt6_probe_work.
>
> Change it to kfree struct __rt6_probe_work to not implicitly depend on
> struct work_struct being the first element.
>
> This does not affect the generated code.
>
> Signed-off-by: Michael Buesch <m@bues.ch>
Applied.
^ permalink raw reply
* DSA: Attaching phy twice?
From: Andrew Lunn @ 2015-02-08 20:32 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev
Hi Florian
Have you seen messages like this before?
[ 2.495126] Distributed Switch Architecture driver version 0.1
[ 2.501358] mvneta f1070000.ethernet eth0: [0]: detected a Marvell 88E6172 switch
[ 2.556441] libphy: dsa slave smi: probed
[ 2.638292] net lan4: PHY already attached
[ 2.733285] net lan3: PHY already attached
[ 2.820749] net lan2: PHY already attached
[ 2.910749] net lan1: PHY already attached
[ 3.000772] net internet: PHY already attached
What appears to be happening is that dsa_slave_phy_setup() is finding
the phy for the port via device tree and using of_phy_connect(), or it
uses the fall back of taking a phy from the switch internal mdio bus
and calling phy_connect_direct(). So if a phy is found,
phy_attach_direct() gets called to attach the phy to the device.
Then in dsa_slave_create(), a second call to phy_attach() is
made. This is when we get the warning "PHY already attached", and it
returns EBUSY, which is ignored.
Is this code at the end of dsa_slave_create() doing anything useful?
if (p->phy != NULL) {
if (ds->drv->get_phy_flags)
p->phy->dev_flags |= ds->drv->get_phy_flags(ds, port);
phy_attach(slave_dev, dev_name(&p->phy->dev),
PHY_INTERFACE_MODE_GMII);
p->phy->autoneg = AUTONEG_ENABLE;
p->phy->speed = 0;
p->phy->duplex = 0;
p->phy->advertising = p->phy->supported | ADVERTISED_Autoneg;
}
My quick testing suggests its not useful, so if you agree, i will
submit a patch removing it. Or am i missing something?
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH v3] net: bluetooth: hci_sock: Use 'const u32 *' instead of 'void *' for 2nd parameter of hci_test_bit()
From: Marcel Holtmann @ 2015-02-08 20:17 UTC (permalink / raw)
To: Chen Gang S
Cc: Joe Perches, David Laight, Sergei Shtylyov, Gustavo F. Padovan,
Johan Hedberg, David S. Miller, linux-bluetooth@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <54D75698.7010808@sunrus.com.cn>
Hi Chen,
>>>> hci_test_bit() does not modify 2nd parameter, so it is better to let it
>>>> be constant, or may cause build warning. The related warning (with
>>>> allmodconfig under xtensa):
>>>>
>>>> net/bluetooth/hci_sock.c: In function 'hci_sock_sendmsg':
>>>> net/bluetooth/hci_sock.c:955:8: warning: passing argument 2 of 'hci_test_bit' discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
>>>> &hci_sec_filter.ocf_mask[ogf])) &&
>>>> ^
>>>> net/bluetooth/hci_sock.c:49:19: note: expected 'void *' but argument is of type 'const __u32 (*)[4] {aka const unsigned int (*)[4]}'
>>>> static inline int hci_test_bit(int nr, void *addr)
>>>> ^
>>>>
>>>> hci_test_bit() always treats 2nd parameter is u32, and all callers also
>>>> know about it, so 2nd parameter of hci_test_bit() need use 'const u32 *'
>>>> instead of 'void *'.
>>>>
>>>> C language treats the array function parameter as a pointer, so the
>>>> caller need not use '&' for the 2 demotion array, or it reports warning:
>>>> 'const unsigned int (*)[4]' is different with 'const unsigned int *'.
>>>
>>> I still think you are possibly papering over potential bugs
>>> on big-endian 64 bit systems.
>>>
>>> unsigned long vs u32.
>>>
>>> How are the bits actually set?
>>>
>>
>>> From current usage of event_mask, "(u32 *) f->event_mask" is only for
>> event_mask data storage, not for calculation (always as "u32 *" for
>> calculation).
>>
>> [root@localhost linux-next]# grep -rn "\<event_mask\>" include/net/bluetooth net/bluetooth
>> include/net/bluetooth/hci_sock.h:51: unsigned long event_mask[2];
>
> e.g. use "unsigned char event_mask[2 * sizeof(unsigned long)]" instead
> of "unsigned long event_mask[2]".
>
> There is still no any issue within "hci_sock.c" (although I am not sure
> whether this modification may cause issues in another modules outside
> kernel).
what about writing a test case for userspace that ensures that things are working correctly. As I said before, we left it this way since it is part of the API.
Regards
Marcel
^ permalink raw reply
* Re: Kill I4L? (was: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors)
From: Joe Perches @ 2015-02-08 20:04 UTC (permalink / raw)
To: Tilman Schmidt
Cc: Paul Bolle, Sergei Shtylyov, Bas Peters, isdn, julia.lawall,
davem, netdev, linux-kernel
In-Reply-To: <54D7BD4A.4090101@imap.cc>
On Sun, 2015-02-08 at 20:47 +0100, Tilman Schmidt wrote:
> Am 07.02.2015 um 21:43 schrieb Paul Bolle:
> > On Sat, 2015-02-07 at 11:19 -0800, Joe Perches wrote:
> >> Does anyone still use these cards?
> > 0) Good question.
> I very much doubt it. It was an ISA card, not even PnP. I'd imagine
> any systems of that kind would be retired by now.
[]
> I guess
> somebody could just submit a patch series starting with:
> +++ b/drivers/isdn/Kconfig
> @@ -20,25 +20,6 @@ menuconfig ISDN
>
> if ISDN
>
> -menuconfig ISDN_I4L
> - tristate "Old ISDN4Linux (deprecated)"
> - depends on TTY
[]
> Shall I?
I think removing code for completely unused,
no longer sold or supported devices could be
a good thing.
If someone pipes up saying "I use that" within
some shortish time-frame, it's easy to undo.
^ permalink raw reply
* Kill I4L? (was: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors)
From: Tilman Schmidt @ 2015-02-08 19:47 UTC (permalink / raw)
To: Paul Bolle, Joe Perches
Cc: Sergei Shtylyov, Bas Peters, isdn, julia.lawall, davem, netdev,
linux-kernel
In-Reply-To: <1423341800.2246.68.camel@x220>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Am 07.02.2015 um 21:43 schrieb Paul Bolle:
> On Sat, 2015-02-07 at 11:19 -0800, Joe Perches wrote:
>> Does anyone still use these cards?
>
> 0) Good question.
I very much doubt it. It was an ISA card, not even PnP. I'd imagine
any systems of that kind would be retired by now.
> 2) Broader picture: if I remember correctly there are now four
> different flavors of ISDN in the kernel: - really old: pre-i4l
I don't think any traces of that are still present in current kernel
releases. It should all have been left behind on the switch to 2.6.
> - very old: i4l - just old: CAPI - not so old: mISDN
Those are the in-tree ones. To make matters worse, the Asterisk people
invented three more (Zaptel, DAHDI and vISDN) which are maintained
out-of-tree. Apparently they weren't satisfied with any the in-tree
ones and didn't feel like helping to improve one of them to match
their needs.
> [snip] So the current ISDN situation is a bit messy.
That's putting it mildly.
> Tilman might be able to provide a clearer, and maybe less grumpy,
> summary of the current situation.
Don't know about either. ;-)
> [M]aybe we should consider, say, removing i4l and pre i4l and see
> who complains. That might be a rude thing to do. So perhaps the
> various ISDN flavors should be left alone until ... what exactly?
I'd support that step. I don't think it'll hurt anyone because the
cards supported by i4l are mostly ISA cards anyway. The only exceptions
are the HiSax family which is now supported by mISDN, and the Hypercope
family which is supported by CAPI.
In the past we had Documentation/feature-removal-schedule.txt for
announcing removals like that but Linus shot that down on 2012-10-01
(commit 9c0ece069b32e8e122aea71aa47181c10eb85ba7) so I guess
somebody could just submit a patch series starting with
- --- a/drivers/isdn/Kconfig
+++ b/drivers/isdn/Kconfig
@@ -20,25 +20,6 @@ menuconfig ISDN
if ISDN
- -menuconfig ISDN_I4L
- - tristate "Old ISDN4Linux (deprecated)"
- - depends on TTY
- - ---help---
- - This driver allows you to use an ISDN adapter for networking
- - connections and as dialin/out device. The isdn-tty's have a
built
- - in AT-compatible modem emulator. Network devices support
autodial,
- - channel-bundling, callback and caller-authentication without
having
- - a daemon running. A reduced T.70 protocol is supported with
tty's
- - suitable for German BTX. On D-Channel, the protocols EDSS1
- - (Euro-ISDN) and 1TR6 (German style) are supported. See
- - <file:Documentation/isdn/README> for more information.
- -
- - ISDN support in the linux kernel is moving towards a new API,
- - called CAPI (Common ISDN Application Programming Interface).
- - Therefore the old ISDN4Linux layer will eventually become
obsolete.
- - It is still available, though, for use with adapters that
are not
- - supported by the new CAPI subsystem yet.
- -
source "drivers/isdn/i4l/Kconfig"
menuconfig ISDN_CAPI
and working its way from that to remove anything that's become
unreachable.
Shall I?
- --
Tilman Schmidt E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQEcBAEBAgAGBQJU171KAAoJEFPuqx0v+F+qnqgH/3KCC6wnXWKOxfuWZ13lwqa2
GuFy+DMYZbXi5e5IHIYnTO9LvA7uzg4ryokP4iMWazhIpx5Cx0Riil59LJExE59p
jsLizpDeZ+z+wwrEZHimc8lYEFO70abvgKL5NdkF8luc+QPIJAmDyLA+8c6J0rKo
VcIb+vq3hW06L7FeKD3Y03NU1xZDtG6HxhnCQRV5b8Ve6sfeu4Oz/83yLyvvfNbK
mnRWbUisO2TmBWcTbZ0QB889iPBRrR1HD4TLA9WZ38fQs7YbuKNbz4jxMKDDSh2i
JHDK8tzn5P+73CPHkvRfELODn+V5uzUWh6QHJFD+4sMOKvNZfbxlEmt1GANmto0=
=PtE1
-----END PGP SIGNATURE-----
^ permalink raw reply
* nft hash set expansion problem
From: Josh Hunt @ 2015-02-08 19:38 UTC (permalink / raw)
To: Thomas Graf; +Cc: Pablo Neira Ayuso, kaber, netdev, netfilter-devel
Nft hash sets are unable to expand past the initial # of buckets. This
is b/c nft hash sets don't define the max_shift parameter and so
rht_grow_above_75():
return atomic_read(&ht->nelems) > (new_size / 4 * 3) &&
(ht->p.max_shift && atomic_read(&ht->shift) <
ht->p.max_shift);
can't return true.
It's not clear to me if this is intentional; requiring users of
rhashtables define a max_shift in order to support expansion, or a bug
in the grow decision function?
Here's a possible fix if it's the latter. Let me know and I can submit
something formal if that's the case.
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index e96fc00..2c51617 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -250,7 +250,7 @@ bool rht_grow_above_75(const struct rhashtable *ht,
size_t new_size)
{
/* Expand table when exceeding 75% load */
return atomic_read(&ht->nelems) > (new_size / 4 * 3) &&
- (ht->p.max_shift && atomic_read(&ht->shift) <
ht->p.max_shift);
+ (ht->p.max_shift ? atomic_read(&ht->shift) <
ht->p.max_shift : 1);
}
EXPORT_SYMBOL_GPL(rht_grow_above_75);
Thanks
Josh
^ permalink raw reply related
* [PATCH net-next 1/1] tipc: fix bug in socket reception function
From: Jon Maloy @ 2015-02-08 16:10 UTC (permalink / raw)
To: davem
Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
tipc-discussion, Jon Maloy
In commit c637c1035534867b85b78b453c38c495b58e2c5a ("tipc: resolve race
problem at unicast message reception") we introduced a time limit
for how long the function tipc_sk_eneque() would be allowed to execute
its loop. Unfortunately, the test for when this limit is passed was put
in the wrong place, resulting in a lost message when the test is true.
We fix this by moving the test to before we dequeue the next buffer
from the input queue.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/socket.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 6666680..4a98d15 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1802,12 +1802,11 @@ static int tipc_sk_enqueue(struct sk_buff_head *inputq, struct sock *sk,
unsigned long time_limit = jiffies + 2;
while (skb_queue_len(inputq)) {
+ if (unlikely(time_after_eq(jiffies, time_limit)))
+ return TIPC_OK;
skb = tipc_skb_dequeue(inputq, dport);
if (unlikely(!skb))
return TIPC_OK;
- /* Return if softirq window exhausted */
- if (unlikely(time_after_eq(jiffies, time_limit)))
- return TIPC_OK;
if (!sock_owned_by_user(sk)) {
err = filter_rcv(sk, &skb);
if (likely(!skb))
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v3] net: bluetooth: hci_sock: Use 'const u32 *' instead of 'void *' for 2nd parameter of hci_test_bit()
From: Chen Gang S @ 2015-02-08 12:29 UTC (permalink / raw)
To: Joe Perches
Cc: David Laight, Marcel Holtmann, Sergei Shtylyov,
Gustavo F. Padovan, Johan Hedberg, David S. Miller,
linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <54D6A729.1070905@sunrus.com.cn>
On 2/8/15 08:00, Chen Gang S wrote:
> On 2/8/15 03:52, Joe Perches wrote:
>> On Sat, 2015-02-07 at 21:24 +0800, Chen Gang S wrote:
>>> hci_test_bit() does not modify 2nd parameter, so it is better to let it
>>> be constant, or may cause build warning. The related warning (with
>>> allmodconfig under xtensa):
>>>
>>> net/bluetooth/hci_sock.c: In function 'hci_sock_sendmsg':
>>> net/bluetooth/hci_sock.c:955:8: warning: passing argument 2 of 'hci_test_bit' discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
>>> &hci_sec_filter.ocf_mask[ogf])) &&
>>> ^
>>> net/bluetooth/hci_sock.c:49:19: note: expected 'void *' but argument is of type 'const __u32 (*)[4] {aka const unsigned int (*)[4]}'
>>> static inline int hci_test_bit(int nr, void *addr)
>>> ^
>>>
>>> hci_test_bit() always treats 2nd parameter is u32, and all callers also
>>> know about it, so 2nd parameter of hci_test_bit() need use 'const u32 *'
>>> instead of 'void *'.
>>>
>>> C language treats the array function parameter as a pointer, so the
>>> caller need not use '&' for the 2 demotion array, or it reports warning:
>>> 'const unsigned int (*)[4]' is different with 'const unsigned int *'.
>>
>> I still think you are possibly papering over potential bugs
>> on big-endian 64 bit systems.
>>
>> unsigned long vs u32.
>>
>> How are the bits actually set?
>>
>
>>From current usage of event_mask, "(u32 *) f->event_mask" is only for
> event_mask data storage, not for calculation (always as "u32 *" for
> calculation).
>
> [root@localhost linux-next]# grep -rn "\<event_mask\>" include/net/bluetooth net/bluetooth
> include/net/bluetooth/hci_sock.h:51: unsigned long event_mask[2];
e.g. use "unsigned char event_mask[2 * sizeof(unsigned long)]" instead
of "unsigned long event_mask[2]".
There is still no any issue within "hci_sock.c" (although I am not sure
whether this modification may cause issues in another modules outside
kernel).
Thanks.
> include/net/bluetooth/hci_sock.h:57: __u32 event_mask[2];
> net/bluetooth/hci_sock.c:59: __u32 event_mask[2];
> net/bluetooth/hci_sock.c:110: if (!hci_test_bit(flt_event, (u32 *)&flt->event_mask))
> net/bluetooth/hci_sock.c:1041: uf.event_mask[0] = *((u32 *) f->event_mask + 0);
> net/bluetooth/hci_sock.c:1042: uf.event_mask[1] = *((u32 *) f->event_mask + 1);
> net/bluetooth/hci_sock.c:1053: uf.event_mask[0] &= *((u32 *) hci_sec_filter.event_mask + 0);
> net/bluetooth/hci_sock.c:1054: uf.event_mask[1] &= *((u32 *) hci_sec_filter.event_mask + 1);
> net/bluetooth/hci_sock.c:1062: *((u32 *) f->event_mask + 0) = uf.event_mask[0];
> net/bluetooth/hci_sock.c:1063: *((u32 *) f->event_mask + 1) = uf.event_mask[1];
> net/bluetooth/hci_sock.c:1124: uf.event_mask[0] = *((u32 *) f->event_mask + 0);
> net/bluetooth/hci_sock.c:1125: uf.event_mask[1] = *((u32 *) f->event_mask + 1);
>
> Calculation is machine endian dependency, but event_mask is always as
> "u32 *" for calculation, so there is no any type cast for calculation,
> it is OK.
>
> Storage is independent from machine endian, but it depends on machine
> bits. In our case, 'unsigned long' array has enough space to accept u32
> array, so there is no any data overwritten, it is OK.
>
>
> By the way, I intended to remain event_mask as 'unsigned long' type,
> because I am not quite sure whether it is also used by another modules
> in kernel (or any other systems). May we change it to u32?
>
> Thanks.
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply
* Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.
From: Michael S. Tsirkin @ 2015-02-08 10:59 UTC (permalink / raw)
To: Rusty Russell; +Cc: lkml, netdev@vger.kernel.org
In-Reply-To: <1423199216-2094-4-git-send-email-rusty@rustcorp.com.au>
On Fri, Feb 06, 2015 at 03:36:54PM +1030, Rusty Russell wrote:
> In particular, the virtio header always has the u16 num_buffers field.
> We define a new 'struct virtio_net_modern_hdr' for this (rather than
> simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
> if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> ---
> include/uapi/linux/virtio_net.h | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index b5f1677b291c..32754f3000e8 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -35,7 +35,6 @@
> #define VIRTIO_NET_F_CSUM 0 /* Host handles pkts w/ partial csum */
> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */
> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */
> -#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */
> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */
> #define VIRTIO_NET_F_GUEST_ECN 9 /* Guest can handle TSO[6] w/ ECN in. */
> @@ -56,6 +55,10 @@
> * Steering */
> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
>
> +#ifndef VIRTIO_NET_NO_LEGACY
> +#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
> +#endif /* VIRTIO_NET_NO_LEGACY */
> +
> #define VIRTIO_NET_S_LINK_UP 1 /* Link is up */
> #define VIRTIO_NET_S_ANNOUNCE 2 /* Announcement is needed */
>
> @@ -71,8 +74,9 @@ struct virtio_net_config {
> __u16 max_virtqueue_pairs;
> } __attribute__((packed));
>
> +#ifndef VIRTIO_NET_NO_LEGACY
> /* This header comes first in the scatter-gather list.
> - * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
> + * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
> * be the first element of the scatter-gather list. If you don't
> * specify GSO or CSUM features, you can simply ignore the header. */
> struct virtio_net_hdr {
> @@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
> struct virtio_net_hdr hdr;
> __virtio16 num_buffers; /* Number of merged rx buffers */
> };
> +#else /* ... VIRTIO_NET_NO_LEGACY */
> +/*
> + * This header comes first in the scatter-gather list. If you don't
> + * specify GSO or CSUM features, you can simply ignore the header.
> + */
> +struct virtio_net_modern_hdr {
> +#define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */
> +#define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */
> + __u8 flags;
> +#define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */
> +#define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */
> +#define VIRTIO_NET_HDR_GSO_UDP 3 /* GSO frame, IPv4 UDP (UFO) */
> +#define VIRTIO_NET_HDR_GSO_TCPV6 4 /* GSO frame, IPv6 TCP */
> +#define VIRTIO_NET_HDR_GSO_ECN 0x80 /* TCP has ECN set */
> + __u8 gso_type;
> + __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
> + __virtio16 gso_size; /* Bytes to append to hdr_len per frame */
> + __virtio16 csum_start; /* Position to start checksumming from */
> + __virtio16 csum_offset; /* Offset after that to place checksum */
> + __virtio16 num_buffers; /* Number of merged rx buffers */
> +};
> +#endif /* ...VIRTIO_NET_NO_LEGACY */
This kind of masks the fact that it's the same as
virtio_net_hdr_mrg_rxbuf. So it's forcing people to duplicate
code for transitional devices.
How about
struct virtio_net_modern_hdr {
struct virtio_net_hdr_mrg_rxbuf hdr;
}
This will also make it look nicer when we start
adding stuff in the header, the main header
is separated in a struct by its own, so it's
easy to apply operations such as sizeof.
> /*
> * Control virtqueue data structures
> --
> 2.1.0
^ permalink raw reply
* [PATCH] rt6_probe_deferred: Do not depend on struct ordering
From: Michael Büsch @ 2015-02-08 9:14 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Hannes Frederic Sowa
[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]
rt6_probe allocates a struct __rt6_probe_work and schedules a work handler rt6_probe_deferred.
But rt6_probe_deferred kfree's the struct work_struct instead of struct __rt6_probe_work.
This works, because struct work_struct is the first element of struct __rt6_probe_work.
Change it to kfree struct __rt6_probe_work to not implicitly depend on
struct work_struct being the first element.
This does not affect the generated code.
Signed-off-by: Michael Buesch <m@bues.ch>
---
The affected code was introduced in c2f17e827b419918c856131f592df9521e1a38e3
Index: linux/net/ipv6/route.c
===================================================================
--- linux.orig/net/ipv6/route.c
+++ linux/net/ipv6/route.c
@@ -499,7 +499,7 @@ static void rt6_probe_deferred(struct wo
addrconf_addr_solict_mult(&work->target, &mcaddr);
ndisc_send_ns(work->dev, NULL, &work->target, &mcaddr, NULL);
dev_put(work->dev);
- kfree(w);
+ kfree(work);
}
static void rt6_probe(struct rt6_info *rt)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 0/5] virtio 1.0 cleanups and one fix.
From: Michael S. Tsirkin @ 2015-02-08 9:52 UTC (permalink / raw)
To: Rusty Russell; +Cc: lkml, netdev@vger.kernel.org
In-Reply-To: <1423199216-2094-1-git-send-email-rusty@rustcorp.com.au>
On Fri, Feb 06, 2015 at 03:36:51PM +1030, Rusty Russell wrote:
> Hi all,
>
> Some minor fixes for my virtio-next tree. Michael, does
> QEMU implement the (compuslory!) VIRTIO_PCI_CAP_PCI_CFG field? I'm
> guessing not, since it wasn't defined in the Linux header :(
Not yet, thanks for the reminder.
BIOS is going to use that one so you can be sure
it'll be there when support is merged finally.
> Rusty Russell (5):
> virtio: define VIRTIO_PCI_CAP_PCI_CFG in header.
> virtio: Don't expose legacy block features when VIRTIO_BLK_NO_LEGACY
> defined.
> virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY
> defined.
> virtio: Don't expose legacy config features when
> VIRTIO_CONFIG_NO_LEGACY defined.
> virtio: don't require a config space on the console device.
>
> drivers/char/virtio_console.c | 12 ++++++++----
> include/uapi/linux/virtio_blk.h | 17 +++++++++++++----
> include/uapi/linux/virtio_config.h | 2 ++
> include/uapi/linux/virtio_net.h | 30 ++++++++++++++++++++++++++++--
> include/uapi/linux/virtio_pci.h | 4 +++-
> 5 files changed, 54 insertions(+), 11 deletions(-)
>
> --
> 2.1.0
^ permalink raw reply
* [PATCH V1 net-next 3/3] IB/mlx4: Reset flow support for IB kernel ULPs
From: Or Gerlitz @ 2015-02-08 9:49 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Roland Dreier, Amir Vadai, Tal Alon, Yishai Hadas,
Or Gerlitz
In-Reply-To: <1423388974-9635-1-git-send-email-ogerlitz@mellanox.com>
From: Yishai Hadas <yishaih@mellanox.com>
The driver exposes interfaces that directly relate to HW state. Upon fatal
error, consumers of these interfaces (ULPs) that rely on completion of
all their posted work-request could hang, thereby introducing dependencies
in shutdown order. To prevent this from happening, we manage the
relevant resources (CQs, QPs) that are used by the device. Upon a fatal error,
we now generate simulated completions for outstanding WQEs that were not
completed at the time the HW was reset.
It includes invoking the completion event handler for all involved CQs so that
the ULPs will poll those CQs. When polled we return simulated CQEs with
IB_WC_WR_FLUSH_ERR return code enabling ULPs to clean up their resources and
not wait forever for completions upon receiving remove_one.
The above change requires an extra check in the data path to make sure that when
device is in error state, the simulated CQEs will be returned and no further
WQEs will be posted.
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/infiniband/hw/mlx4/cq.c | 57 ++++++++++++++++++++++++++++++
drivers/infiniband/hw/mlx4/main.c | 64 ++++++++++++++++++++++++++++++++++
drivers/infiniband/hw/mlx4/mlx4_ib.h | 9 +++++
drivers/infiniband/hw/mlx4/qp.c | 59 ++++++++++++++++++++++++++++---
drivers/infiniband/hw/mlx4/srq.c | 8 ++++
include/linux/mlx4/device.h | 2 +
6 files changed, 193 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/cq.c b/drivers/infiniband/hw/mlx4/cq.c
index a3b70f6..543ecdd 100644
--- a/drivers/infiniband/hw/mlx4/cq.c
+++ b/drivers/infiniband/hw/mlx4/cq.c
@@ -188,6 +188,8 @@ struct ib_cq *mlx4_ib_create_cq(struct ib_device *ibdev, int entries, int vector
spin_lock_init(&cq->lock);
cq->resize_buf = NULL;
cq->resize_umem = NULL;
+ INIT_LIST_HEAD(&cq->send_qp_list);
+ INIT_LIST_HEAD(&cq->recv_qp_list);
if (context) {
struct mlx4_ib_create_cq ucmd;
@@ -594,6 +596,55 @@ static int use_tunnel_data(struct mlx4_ib_qp *qp, struct mlx4_ib_cq *cq, struct
return 0;
}
+static void mlx4_ib_qp_sw_comp(struct mlx4_ib_qp *qp, int num_entries,
+ struct ib_wc *wc, int *npolled, int is_send)
+{
+ struct mlx4_ib_wq *wq;
+ unsigned cur;
+ int i;
+
+ wq = is_send ? &qp->sq : &qp->rq;
+ cur = wq->head - wq->tail;
+
+ if (cur == 0)
+ return;
+
+ for (i = 0; i < cur && *npolled < num_entries; i++) {
+ wc->wr_id = wq->wrid[wq->tail & (wq->wqe_cnt - 1)];
+ wc->status = IB_WC_WR_FLUSH_ERR;
+ wc->vendor_err = MLX4_CQE_SYNDROME_WR_FLUSH_ERR;
+ wq->tail++;
+ (*npolled)++;
+ wc->qp = &qp->ibqp;
+ wc++;
+ }
+}
+
+static void mlx4_ib_poll_sw_comp(struct mlx4_ib_cq *cq, int num_entries,
+ struct ib_wc *wc, int *npolled)
+{
+ struct mlx4_ib_qp *qp;
+
+ *npolled = 0;
+ /* Find uncompleted WQEs belonging to that cq and retrun
+ * simulated FLUSH_ERR completions
+ */
+ list_for_each_entry(qp, &cq->send_qp_list, cq_send_list) {
+ mlx4_ib_qp_sw_comp(qp, num_entries, wc, npolled, 1);
+ if (*npolled >= num_entries)
+ goto out;
+ }
+
+ list_for_each_entry(qp, &cq->recv_qp_list, cq_recv_list) {
+ mlx4_ib_qp_sw_comp(qp, num_entries, wc + *npolled, npolled, 0);
+ if (*npolled >= num_entries)
+ goto out;
+ }
+
+out:
+ return;
+}
+
static int mlx4_ib_poll_one(struct mlx4_ib_cq *cq,
struct mlx4_ib_qp **cur_qp,
struct ib_wc *wc)
@@ -836,8 +887,13 @@ int mlx4_ib_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *wc)
unsigned long flags;
int npolled;
int err = 0;
+ struct mlx4_ib_dev *mdev = to_mdev(cq->ibcq.device);
spin_lock_irqsave(&cq->lock, flags);
+ if (mdev->dev->persist->state & MLX4_DEVICE_STATE_INTERNAL_ERROR) {
+ mlx4_ib_poll_sw_comp(cq, num_entries, wc, &npolled);
+ goto out;
+ }
for (npolled = 0; npolled < num_entries; ++npolled) {
err = mlx4_ib_poll_one(cq, &cur_qp, wc + npolled);
@@ -847,6 +903,7 @@ int mlx4_ib_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *wc)
mlx4_cq_set_ci(&cq->mcq);
+out:
spin_unlock_irqrestore(&cq->lock, flags);
if (err == 0 || err == -EAGAIN)
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 3140da5..eb8e215 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -2308,6 +2308,8 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
spin_lock_init(&ibdev->sm_lock);
mutex_init(&ibdev->cap_mask_mutex);
+ INIT_LIST_HEAD(&ibdev->qp_list);
+ spin_lock_init(&ibdev->reset_flow_resource_lock);
if (ibdev->steering_support == MLX4_STEERING_MODE_DEVICE_MANAGED &&
ib_num_ports) {
@@ -2622,6 +2624,67 @@ out:
return;
}
+static void mlx4_ib_handle_catas_error(struct mlx4_ib_dev *ibdev)
+{
+ struct mlx4_ib_qp *mqp;
+ unsigned long flags_qp;
+ unsigned long flags_cq;
+ struct mlx4_ib_cq *send_mcq, *recv_mcq;
+ struct list_head cq_notify_list;
+ struct mlx4_cq *mcq;
+ unsigned long flags;
+
+ pr_warn("mlx4_ib_handle_catas_error was started\n");
+ INIT_LIST_HEAD(&cq_notify_list);
+
+ /* Go over qp list reside on that ibdev, sync with create/destroy qp.*/
+ spin_lock_irqsave(&ibdev->reset_flow_resource_lock, flags);
+
+ list_for_each_entry(mqp, &ibdev->qp_list, qps_list) {
+ spin_lock_irqsave(&mqp->sq.lock, flags_qp);
+ if (mqp->sq.tail != mqp->sq.head) {
+ send_mcq = to_mcq(mqp->ibqp.send_cq);
+ spin_lock_irqsave(&send_mcq->lock, flags_cq);
+ if (send_mcq->mcq.comp &&
+ mqp->ibqp.send_cq->comp_handler) {
+ if (!send_mcq->mcq.reset_notify_added) {
+ send_mcq->mcq.reset_notify_added = 1;
+ list_add_tail(&send_mcq->mcq.reset_notify,
+ &cq_notify_list);
+ }
+ }
+ spin_unlock_irqrestore(&send_mcq->lock, flags_cq);
+ }
+ spin_unlock_irqrestore(&mqp->sq.lock, flags_qp);
+ /* Now, handle the QP's receive queue */
+ spin_lock_irqsave(&mqp->rq.lock, flags_qp);
+ /* no handling is needed for SRQ */
+ if (!mqp->ibqp.srq) {
+ if (mqp->rq.tail != mqp->rq.head) {
+ recv_mcq = to_mcq(mqp->ibqp.recv_cq);
+ spin_lock_irqsave(&recv_mcq->lock, flags_cq);
+ if (recv_mcq->mcq.comp &&
+ mqp->ibqp.recv_cq->comp_handler) {
+ if (!recv_mcq->mcq.reset_notify_added) {
+ recv_mcq->mcq.reset_notify_added = 1;
+ list_add_tail(&recv_mcq->mcq.reset_notify,
+ &cq_notify_list);
+ }
+ }
+ spin_unlock_irqrestore(&recv_mcq->lock,
+ flags_cq);
+ }
+ }
+ spin_unlock_irqrestore(&mqp->rq.lock, flags_qp);
+ }
+
+ list_for_each_entry(mcq, &cq_notify_list, reset_notify) {
+ mcq->comp(mcq);
+ }
+ spin_unlock_irqrestore(&ibdev->reset_flow_resource_lock, flags);
+ pr_warn("mlx4_ib_handle_catas_error ended\n");
+}
+
static void handle_bonded_port_state_event(struct work_struct *work)
{
struct ib_event_work *ew =
@@ -2701,6 +2764,7 @@ static void mlx4_ib_event(struct mlx4_dev *dev, void *ibdev_ptr,
case MLX4_DEV_EVENT_CATASTROPHIC_ERROR:
ibdev->ib_active = false;
ibev.event = IB_EVENT_DEVICE_FATAL;
+ mlx4_ib_handle_catas_error(ibdev);
break;
case MLX4_DEV_EVENT_PORT_MGMT_CHANGE:
diff --git a/drivers/infiniband/hw/mlx4/mlx4_ib.h b/drivers/infiniband/hw/mlx4/mlx4_ib.h
index 721540c..f829fd9 100644
--- a/drivers/infiniband/hw/mlx4/mlx4_ib.h
+++ b/drivers/infiniband/hw/mlx4/mlx4_ib.h
@@ -110,6 +110,9 @@ struct mlx4_ib_cq {
struct mutex resize_mutex;
struct ib_umem *umem;
struct ib_umem *resize_umem;
+ /* List of qps that it serves.*/
+ struct list_head send_qp_list;
+ struct list_head recv_qp_list;
};
struct mlx4_ib_mr {
@@ -300,6 +303,9 @@ struct mlx4_ib_qp {
struct mlx4_roce_smac_vlan_info pri;
struct mlx4_roce_smac_vlan_info alt;
u64 reg_id;
+ struct list_head qps_list;
+ struct list_head cq_recv_list;
+ struct list_head cq_send_list;
};
struct mlx4_ib_srq {
@@ -535,6 +541,9 @@ struct mlx4_ib_dev {
/* lock when destroying qp1_proxy and getting netdev events */
struct mutex qp1_proxy_lock[MLX4_MAX_PORTS];
u8 bond_next_port;
+ /* protect resources needed as part of reset flow */
+ spinlock_t reset_flow_resource_lock;
+ struct list_head qp_list;
};
struct ib_event_work {
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 792f9dc..dfc6ca1 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -46,6 +46,11 @@
#include "mlx4_ib.h"
#include "user.h"
+static void mlx4_ib_lock_cqs(struct mlx4_ib_cq *send_cq,
+ struct mlx4_ib_cq *recv_cq);
+static void mlx4_ib_unlock_cqs(struct mlx4_ib_cq *send_cq,
+ struct mlx4_ib_cq *recv_cq);
+
enum {
MLX4_IB_ACK_REQ_FREQ = 8,
};
@@ -618,6 +623,8 @@ static int create_qp_common(struct mlx4_ib_dev *dev, struct ib_pd *pd,
struct mlx4_ib_sqp *sqp;
struct mlx4_ib_qp *qp;
enum mlx4_ib_qp_type qp_type = (enum mlx4_ib_qp_type) init_attr->qp_type;
+ struct mlx4_ib_cq *mcq;
+ unsigned long flags;
/* When tunneling special qps, we use a plain UD qp */
if (sqpn) {
@@ -828,6 +835,24 @@ static int create_qp_common(struct mlx4_ib_dev *dev, struct ib_pd *pd,
qp->mqp.event = mlx4_ib_qp_event;
if (!*caller_qp)
*caller_qp = qp;
+
+ spin_lock_irqsave(&dev->reset_flow_resource_lock, flags);
+ mlx4_ib_lock_cqs(to_mcq(init_attr->send_cq),
+ to_mcq(init_attr->recv_cq));
+ /* Maintain device to QPs access, needed for further handling
+ * via reset flow
+ */
+ list_add_tail(&qp->qps_list, &dev->qp_list);
+ /* Maintain CQ to QPs access, needed for further handling
+ * via reset flow
+ */
+ mcq = to_mcq(init_attr->send_cq);
+ list_add_tail(&qp->cq_send_list, &mcq->send_qp_list);
+ mcq = to_mcq(init_attr->recv_cq);
+ list_add_tail(&qp->cq_recv_list, &mcq->recv_qp_list);
+ mlx4_ib_unlock_cqs(to_mcq(init_attr->send_cq),
+ to_mcq(init_attr->recv_cq));
+ spin_unlock_irqrestore(&dev->reset_flow_resource_lock, flags);
return 0;
err_qpn:
@@ -886,13 +911,13 @@ static void mlx4_ib_lock_cqs(struct mlx4_ib_cq *send_cq, struct mlx4_ib_cq *recv
__acquires(&send_cq->lock) __acquires(&recv_cq->lock)
{
if (send_cq == recv_cq) {
- spin_lock_irq(&send_cq->lock);
+ spin_lock(&send_cq->lock);
__acquire(&recv_cq->lock);
} else if (send_cq->mcq.cqn < recv_cq->mcq.cqn) {
- spin_lock_irq(&send_cq->lock);
+ spin_lock(&send_cq->lock);
spin_lock_nested(&recv_cq->lock, SINGLE_DEPTH_NESTING);
} else {
- spin_lock_irq(&recv_cq->lock);
+ spin_lock(&recv_cq->lock);
spin_lock_nested(&send_cq->lock, SINGLE_DEPTH_NESTING);
}
}
@@ -902,13 +927,13 @@ static void mlx4_ib_unlock_cqs(struct mlx4_ib_cq *send_cq, struct mlx4_ib_cq *re
{
if (send_cq == recv_cq) {
__release(&recv_cq->lock);
- spin_unlock_irq(&send_cq->lock);
+ spin_unlock(&send_cq->lock);
} else if (send_cq->mcq.cqn < recv_cq->mcq.cqn) {
spin_unlock(&recv_cq->lock);
- spin_unlock_irq(&send_cq->lock);
+ spin_unlock(&send_cq->lock);
} else {
spin_unlock(&send_cq->lock);
- spin_unlock_irq(&recv_cq->lock);
+ spin_unlock(&recv_cq->lock);
}
}
@@ -953,6 +978,7 @@ static void destroy_qp_common(struct mlx4_ib_dev *dev, struct mlx4_ib_qp *qp,
int is_user)
{
struct mlx4_ib_cq *send_cq, *recv_cq;
+ unsigned long flags;
if (qp->state != IB_QPS_RESET) {
if (mlx4_qp_modify(dev->dev, NULL, to_mlx4_state(qp->state),
@@ -984,8 +1010,13 @@ static void destroy_qp_common(struct mlx4_ib_dev *dev, struct mlx4_ib_qp *qp,
get_cqs(qp, &send_cq, &recv_cq);
+ spin_lock_irqsave(&dev->reset_flow_resource_lock, flags);
mlx4_ib_lock_cqs(send_cq, recv_cq);
+ /* del from lists under both locks above to protect reset flow paths */
+ list_del(&qp->qps_list);
+ list_del(&qp->cq_send_list);
+ list_del(&qp->cq_recv_list);
if (!is_user) {
__mlx4_ib_cq_clean(recv_cq, qp->mqp.qpn,
qp->ibqp.srq ? to_msrq(qp->ibqp.srq): NULL);
@@ -996,6 +1027,7 @@ static void destroy_qp_common(struct mlx4_ib_dev *dev, struct mlx4_ib_qp *qp,
mlx4_qp_remove(dev->dev, &qp->mqp);
mlx4_ib_unlock_cqs(send_cq, recv_cq);
+ spin_unlock_irqrestore(&dev->reset_flow_resource_lock, flags);
mlx4_qp_free(dev->dev, &qp->mqp);
@@ -2618,8 +2650,15 @@ int mlx4_ib_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
__be32 uninitialized_var(lso_hdr_sz);
__be32 blh;
int i;
+ struct mlx4_ib_dev *mdev = to_mdev(ibqp->device);
spin_lock_irqsave(&qp->sq.lock, flags);
+ if (mdev->dev->persist->state & MLX4_DEVICE_STATE_INTERNAL_ERROR) {
+ err = -EIO;
+ *bad_wr = wr;
+ nreq = 0;
+ goto out;
+ }
ind = qp->sq_next_wqe;
@@ -2917,10 +2956,18 @@ int mlx4_ib_post_recv(struct ib_qp *ibqp, struct ib_recv_wr *wr,
int ind;
int max_gs;
int i;
+ struct mlx4_ib_dev *mdev = to_mdev(ibqp->device);
max_gs = qp->rq.max_gs;
spin_lock_irqsave(&qp->rq.lock, flags);
+ if (mdev->dev->persist->state & MLX4_DEVICE_STATE_INTERNAL_ERROR) {
+ err = -EIO;
+ *bad_wr = wr;
+ nreq = 0;
+ goto out;
+ }
+
ind = qp->rq.head & (qp->rq.wqe_cnt - 1);
for (nreq = 0; wr; ++nreq, wr = wr->next) {
diff --git a/drivers/infiniband/hw/mlx4/srq.c b/drivers/infiniband/hw/mlx4/srq.c
index 62d9285..dce5dfe 100644
--- a/drivers/infiniband/hw/mlx4/srq.c
+++ b/drivers/infiniband/hw/mlx4/srq.c
@@ -316,8 +316,15 @@ int mlx4_ib_post_srq_recv(struct ib_srq *ibsrq, struct ib_recv_wr *wr,
int err = 0;
int nreq;
int i;
+ struct mlx4_ib_dev *mdev = to_mdev(ibsrq->device);
spin_lock_irqsave(&srq->lock, flags);
+ if (mdev->dev->persist->state & MLX4_DEVICE_STATE_INTERNAL_ERROR) {
+ err = -EIO;
+ *bad_wr = wr;
+ nreq = 0;
+ goto out;
+ }
for (nreq = 0; wr; ++nreq, wr = wr->next) {
if (unlikely(wr->num_sge > srq->msrq.max_gs)) {
@@ -362,6 +369,7 @@ int mlx4_ib_post_srq_recv(struct ib_srq *ibsrq, struct ib_recv_wr *wr,
*srq->db.db = cpu_to_be32(srq->wqe_ctr);
}
+out:
spin_unlock_irqrestore(&srq->lock, flags);
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index c116cb0..e4ebff7 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -689,6 +689,8 @@ struct mlx4_cq {
void (*comp)(struct mlx4_cq *);
void *priv;
} tasklet_ctx;
+ int reset_notify_added;
+ struct list_head reset_notify;
};
struct mlx4_qp {
--
1.7.1
^ permalink raw reply related
* [PATCH V1 net-next 0/3] bonding and mlx4 fixes for the HA/LAG support and mlx4 reset flow
From: Or Gerlitz @ 2015-02-08 9:49 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Roland Dreier, Amir Vadai, Tal Alon, Or Gerlitz
Hi Dave,
There are two fixes to the boding + mlx4 HA/LAG support from Moni and a patch from Yishai
which does further hardening of the mlx4 reset support for IB kernel ULPs.
Or.
changes from V0:
- addressed feedback from Sergei Shtylyov and removed un-related tabbing change
Moni Shoua (2):
net/bonding: Fix potential bad memory access during bonding events
IB/mlx4: Always use the correct port for mirrored multicast attachments
Yishai Hadas (1):
IB/mlx4: Reset flow support for IB kernel ULPs
drivers/infiniband/hw/mlx4/cq.c | 57 +++++++++++++++++++++++++++
drivers/infiniband/hw/mlx4/main.c | 70 +++++++++++++++++++++++++++++++++-
drivers/infiniband/hw/mlx4/mlx4_ib.h | 9 ++++
drivers/infiniband/hw/mlx4/qp.c | 59 +++++++++++++++++++++++++---
drivers/infiniband/hw/mlx4/srq.c | 8 ++++
drivers/net/bonding/bond_main.c | 28 +++++--------
include/linux/mlx4/device.h | 2 +
include/net/bonding.h | 2 +-
8 files changed, 210 insertions(+), 25 deletions(-)
^ permalink raw reply
* [PATCH V1 net-next 2/3] IB/mlx4: Always use the correct port for mirrored multicast attachments
From: Or Gerlitz @ 2015-02-08 9:49 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Roland Dreier, Amir Vadai, Tal Alon, Moni Shoua,
Or Gerlitz
In-Reply-To: <1423388974-9635-1-git-send-email-ogerlitz@mellanox.com>
From: Moni Shoua <monis@mellanox.com>
When attaching a QP to a multicast address in bonded mode, there was an
assumption that the port of the QP must be #1. This assumption isn't the
case under the flow which enables maximal usage of the physical ports.
Fix it by always checking the port of the original flow and create the
mirrored flow on the other port.
Fixes: c6215745b66a ('IB/mlx4: Load balance ports in port aggregation mode')
Signed-off-by: Moni Shoua <monis@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/infiniband/hw/mlx4/main.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 2ed5b99..3140da5 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -1186,6 +1186,9 @@ static struct ib_flow *mlx4_ib_create_flow(struct ib_qp *qp,
goto err_create_flow;
i++;
if (is_bonded) {
+ /* Application always sees one port so the mirror rule
+ * must be on port #2
+ */
flow_attr->port = 2;
err = __mlx4_ib_create_flow(qp, flow_attr,
domain, type[j],
@@ -1286,7 +1289,8 @@ static int mlx4_ib_mcg_attach(struct ib_qp *ibqp, union ib_gid *gid, u16 lid)
reg_id.mirror = 0;
if (mlx4_is_bonded(dev)) {
- err = mlx4_multicast_attach(mdev->dev, &mqp->mqp, gid->raw, 2,
+ err = mlx4_multicast_attach(mdev->dev, &mqp->mqp, gid->raw,
+ (mqp->port == 1) ? 2 : 1,
!!(mqp->flags &
MLX4_IB_QP_BLOCK_MULTICAST_LOOPBACK),
prot, ®_id.mirror);
--
1.7.1
^ permalink raw reply related
* [PATCH V1 net-next 1/3] net/bonding: Fix potential bad memory access during bonding events
From: Or Gerlitz @ 2015-02-08 9:49 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Roland Dreier, Amir Vadai, Tal Alon, Moni Shoua,
Or Gerlitz
In-Reply-To: <1423388974-9635-1-git-send-email-ogerlitz@mellanox.com>
From: Moni Shoua <monis@mellanox.com>
When queuing work to send the NETDEV_BONDING_INFO netdev event, it's
possible that when the work is executed, the pointer to the slave
becomes invalid. This can happen if between queuing the event and the
execution of the work, the net-device was un-ensvaled and re-enslaved.
Fix that by queuing a work with the data of the slave instead of the
slave structure.
Fixes: 69e6113343cf ('net/bonding: Notify state change on slaves')
Reported-by: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: Moni Shoua <monis@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/bonding/bond_main.c | 28 +++++++++++-----------------
include/net/bonding.h | 2 +-
2 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 679ef00..b979c26 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1196,18 +1196,11 @@ static void bond_fill_ifslave(struct slave *slave, struct ifslave *info)
info->link_failure_count = slave->link_failure_count;
}
-static void bond_netdev_notify(struct slave *slave, struct net_device *dev)
+static void bond_netdev_notify(struct net_device *dev,
+ struct netdev_bonding_info *info)
{
- struct bonding *bond = slave->bond;
- struct netdev_bonding_info bonding_info;
-
rtnl_lock();
- /* make sure that slave is still valid */
- if (dev->priv_flags & IFF_BONDING) {
- bond_fill_ifslave(slave, &bonding_info.slave);
- bond_fill_ifbond(bond, &bonding_info.master);
- netdev_bonding_info_change(slave->dev, &bonding_info);
- }
+ netdev_bonding_info_change(dev, info);
rtnl_unlock();
}
@@ -1216,25 +1209,26 @@ static void bond_netdev_notify_work(struct work_struct *_work)
struct netdev_notify_work *w =
container_of(_work, struct netdev_notify_work, work.work);
- bond_netdev_notify(w->slave, w->dev);
+ bond_netdev_notify(w->dev, &w->bonding_info);
dev_put(w->dev);
+ kfree(w);
}
void bond_queue_slave_event(struct slave *slave)
{
+ struct bonding *bond = slave->bond;
struct netdev_notify_work *nnw = kzalloc(sizeof(*nnw), GFP_ATOMIC);
if (!nnw)
return;
- INIT_DELAYED_WORK(&nnw->work, bond_netdev_notify_work);
- nnw->slave = slave;
+ dev_hold(slave->dev);
nnw->dev = slave->dev;
+ bond_fill_ifslave(slave, &nnw->bonding_info.slave);
+ bond_fill_ifbond(bond, &nnw->bonding_info.master);
+ INIT_DELAYED_WORK(&nnw->work, bond_netdev_notify_work);
- if (queue_delayed_work(slave->bond->wq, &nnw->work, 0))
- dev_hold(slave->dev);
- else
- kfree(nnw);
+ queue_delayed_work(slave->bond->wq, &nnw->work, 0);
}
/* enslave device <slave> to bond device <master> */
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 4e17095..fda6fee 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -152,8 +152,8 @@ struct bond_parm_tbl {
struct netdev_notify_work {
struct delayed_work work;
- struct slave *slave;
struct net_device *dev;
+ struct netdev_bonding_info bonding_info;
};
struct slave {
--
1.7.1
^ permalink raw reply related
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