* 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: [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: rfs: add hash collision detection
From: David Miller @ 2015-02-09 0:54 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, therbert, ycai, willemb
In-Reply-To: <1423256341.31870.160.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 06 Feb 2015 12:59:01 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> Receive Flow Steering is a nice solution but suffers from
> hash collisions when a mix of connected and unconnected traffic
> is received on the host, when flow hash table is populated.
>
> Also, clearing flow in inet_release() makes RFS not very good
> for short lived flows, as many packets can follow close().
> (FIN , ACK packets, ...)
>
> This patch extends the information stored into global hash table
> to not only include cpu number, but upper part of the hash value.
>
> I use a 32bit value, and dynamically split it in two parts.
>
> For host with less than 64 possible cpus, this gives 6 bits for the
> cpu number, and 26 (32-6) bits for the upper part of the hash.
>
> Since hash bucket selection use low order bits of the hash, we have
> a full hash match, if /proc/sys/net/core/rps_sock_flow_entries is big
> enough.
>
> If the hash found in flow table does not match, we fallback to RPS (if
> it is enabled for the rxqueue).
>
> This means that a packet for an non connected flow can avoid the
> IPI through a unrelated/victim CPU.
>
> This also means we no longer have to clear the table at socket
> close time, and this helps short lived flows performance.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH net-next] Driver: Vmxnet3: Change the hex constant to its decimal equivalent
From: David Miller @ 2015-02-09 0:55 UTC (permalink / raw)
To: skhare; +Cc: sbhatewara, pv-drivers, netdev, linux-kernel
In-Reply-To: <1423259308-4667-1-git-send-email-skhare@vmware.com>
From: Shrikrishna Khare <skhare@vmware.com>
Date: Fri, 6 Feb 2015 13:48:28 -0800
> The hex constant chosen for VMXNET3_REV1_MAGIC is offensive,
> replace it with its decimal equivalent.
>
> Signed-off-by: Shrikrishna Khare <skhare@vmware.com>
> Reviewed-by: Shreyas Bhatewara <sbhatewara@vmware.com>
Applied.
^ permalink raw reply
* Re: [PATCH] vxlan: Wrong type passed to %pIS
From: David Miller @ 2015-02-09 1:15 UTC (permalink / raw)
To: linux; +Cc: pshelar, nicolas.dichtel, netdev, linux-kernel
In-Reply-To: <1423275451-3663-1-git-send-email-linux@rasmusvillemoes.dk>
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Date: Sat, 7 Feb 2015 03:17:31 +0100
> src_ip is a pointer to a union vxlan_addr, one member of which is a
> struct sockaddr. Passing a pointer to src_ip is wrong; one should pass
> the value of src_ip itself. Since %pIS formally expects something of
> type struct sockaddr*, let's pass a pointer to the appropriate union
> member, though this of course doesn't change the generated code.
>
> Fixes: e4c7ed415387 ("vxlan: add ipv6 support")
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net:ethernet:intel:Remove outdated fix me comment in the function, gb_acquire_swfw_sync_i210
From: Jeff Kirsher @ 2015-02-09 1:21 UTC (permalink / raw)
To: Nicholas Krause
Cc: linux.nics, e1000-devel, bruce.w.allan, jesse.brandeburg,
linux-kernel, john.ronciak, netdev
In-Reply-To: <1423372910-29183-1-git-send-email-xerofoify@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 901 bytes --]
On Sun, 2015-02-08 at 00:21 -0500, Nicholas Krause wrote:
> Removes the outdated fix me comment in the
> function,gb_acquire_swfw_sync_i210
> for setting the variables, i and timeout to the intended correct
> values for
> the function,gb_acquire_swfw_sync_i210 to function correctly. This
> comment is
> no longer due to these values having been no changed in the last few
> years and
> no known issues have been found for these variables being set to their
> current
> values. Due to this the comment can now be removed as the values set
> for these
> variables are known to be correct after years of no known issues or
> bugs related
> to their current values.
>
> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> ---
> drivers/net/ethernet/intel/igb/e1000_i210.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Thank Nick, I will add your patch to my queue.
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 441 bytes --]
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
[-- Attachment #3: Type: text/plain, Size: 257 bytes --]
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH] net:ethernet:intel:Remove outdated fix me comment in the function, gb_acquire_swfw_sync_i210
From: Jeff Kirsher @ 2015-02-09 1:26 UTC (permalink / raw)
To: Nicholas Krause
Cc: linux.nics, e1000-devel, netdev, bruce.w.allan, jesse.brandeburg,
linux-kernel, john.ronciak
In-Reply-To: <1423444889.2589.165.camel@jtkirshe-mobl>
[-- Attachment #1.1: Type: text/plain, Size: 1099 bytes --]
On Sun, 2015-02-08 at 17:21 -0800, Jeff Kirsher wrote:
> On Sun, 2015-02-08 at 00:21 -0500, Nicholas Krause wrote:
> > Removes the outdated fix me comment in the
> > function,gb_acquire_swfw_sync_i210
> > for setting the variables, i and timeout to the intended correct
> > values for
> > the function,gb_acquire_swfw_sync_i210 to function correctly. This
> > comment is
> > no longer due to these values having been no changed in the last few
> > years and
> > no known issues have been found for these variables being set to their
> > current
> > values. Due to this the comment can now be removed as the values set
> > for these
> > variables are known to be correct after years of no known issues or
> > bugs related
> > to their current values.
> >
> > Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> > ---
> > drivers/net/ethernet/intel/igb/e1000_i210.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Thank Nick, I will add your patch to my queue.
Oh and the title should be "igb: Remove outdated ...", not
"net:ethernet:intel: Remove ..."
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 441 bytes --]
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
[-- Attachment #3: Type: text/plain, Size: 257 bytes --]
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH] net:ethernet:intel:Remove outdated fix me comment in the function, gb_acquire_swfw_sync_i210
From: nick @ 2015-02-09 1:28 UTC (permalink / raw)
To: Jeff Kirsher
Cc: linux.nics, e1000-devel, netdev, bruce.w.allan, jesse.brandeburg,
linux-kernel, john.ronciak
In-Reply-To: <1423445167.2589.167.camel@jtkirshe-mobl>
On 2015-02-08 08:26 PM, Jeff Kirsher wrote:
> On Sun, 2015-02-08 at 17:21 -0800, Jeff Kirsher wrote:
>> On Sun, 2015-02-08 at 00:21 -0500, Nicholas Krause wrote:
>>> Removes the outdated fix me comment in the
>>> function,gb_acquire_swfw_sync_i210
>>> for setting the variables, i and timeout to the intended correct
>>> values for
>>> the function,gb_acquire_swfw_sync_i210 to function correctly. This
>>> comment is
>>> no longer due to these values having been no changed in the last few
>>> years and
>>> no known issues have been found for these variables being set to their
>>> current
>>> values. Due to this the comment can now be removed as the values set
>>> for these
>>> variables are known to be correct after years of no known issues or
>>> bugs related
>>> to their current values.
>>>
>>> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
>>> ---
>>> drivers/net/ethernet/intel/igb/e1000_i210.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Thank Nick, I will add your patch to my queue.
>
> Oh and the title should be "igb: Remove outdated ...", not
> "net:ethernet:intel: Remove ..."
>
Jeff,
Sorry about the title. Good to known for next time. Should I resend or
will you fix the title for me?
Cheers,
Nick
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [E1000-devel] [PATCH] net:ethernet:intel:Remove outdated fix me comment in the function, gb_acquire_swfw_sync_i210
From: Jeff Kirsher @ 2015-02-09 1:29 UTC (permalink / raw)
To: nick
Cc: linux.nics, e1000-devel, bruce.w.allan, jesse.brandeburg,
linux-kernel, john.ronciak, netdev
In-Reply-To: <54D80D38.3080708@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1429 bytes --]
On Sun, 2015-02-08 at 20:28 -0500, nick wrote:
>
> On 2015-02-08 08:26 PM, Jeff Kirsher wrote:
> > On Sun, 2015-02-08 at 17:21 -0800, Jeff Kirsher wrote:
> >> On Sun, 2015-02-08 at 00:21 -0500, Nicholas Krause wrote:
> >>> Removes the outdated fix me comment in the
> >>> function,gb_acquire_swfw_sync_i210
> >>> for setting the variables, i and timeout to the intended correct
> >>> values for
> >>> the function,gb_acquire_swfw_sync_i210 to function correctly. This
> >>> comment is
> >>> no longer due to these values having been no changed in the last few
> >>> years and
> >>> no known issues have been found for these variables being set to their
> >>> current
> >>> values. Due to this the comment can now be removed as the values set
> >>> for these
> >>> variables are known to be correct after years of no known issues or
> >>> bugs related
> >>> to their current values.
> >>>
> >>> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> >>> ---
> >>> drivers/net/ethernet/intel/igb/e1000_i210.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> Thank Nick, I will add your patch to my queue.
> >
> > Oh and the title should be "igb: Remove outdated ...", not
> > "net:ethernet:intel: Remove ..."
> >
> Jeff,
> Sorry about the title. Good to known for next time. Should I resend or
> will you fix the title for me?
> Cheers,
> Nick
I will fix the title.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] net: dsa: Remove redundant phy_attach()
From: Andrew Lunn @ 2015-02-09 1:29 UTC (permalink / raw)
To: davem, f.fainelli; +Cc: netdev, Andrew Lunn
dsa_slave_phy_setup() finds 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(). Either
way, if a phy is found, phy_attach_direct() is called to attach the
phy to the slave device.
In dsa_slave_create(), a second call to phy_attach() is made. This
results in the warning "PHY already attached". Remove this second,
redundant attaching of the phy.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
net/dsa/slave.c | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 589aafd01fc5..d104ae15836f 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -676,18 +676,5 @@ dsa_slave_create(struct dsa_switch *ds, struct device *parent,
netif_carrier_off(slave_dev);
- 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;
- }
-
return slave_dev;
}
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net-next] rhashtable: Fix remove logic to avoid cross references between buckets
From: Ying Xue @ 2015-02-09 2:44 UTC (permalink / raw)
To: Thomas Graf; +Cc: davem, netdev, herbert
In-Reply-To: <20150206160843.GA31371@casper.infradead.org>
On 02/07/2015 12:08 AM, Thomas Graf wrote:
> The remove logic properly searched the remaining chain for a matching
> entry with an identical hash but it did this while searching from both
> the old and new table. Instead in order to not leave stale references
> behind we need to:
>
> 1. When growing and searching from the new table:
> Search remaining chain for entry with same hash to avoid having
> the new table directly point to a entry with a different hash.
>
> 2. When shrinking and searching from the old table:
> Check if the element after the removed would create a cross
> reference and avoid it if so.
>
> These bugs were present from the beginning in nft_hash.
>
> Also, both insert functions calculated the hash based on the mask of
> the new table. This worked while growing. Wwhile shrinking, the mask
> of the inew table is smaller than the mask of the old table. This lead
> to a bit not being taken into account when selecting the bucket lock
> and thus caused the wrong bucket to be locked eventually.
>
Good Job!
My previously reported issue has been gone, thanks for your hard work!
Regards,
Ying
> Fixes: 7e1e77636e36 ("lib: Resizable, Scalable, Concurrent Hash Table")
> Fixes: 97defe1ecf86 ("rhashtable: Per bucket locks & deferred expansion/shrinking")
> Reported-by: Ying Xue <ying.xue@windriver.com>
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
> ---
> lib/rhashtable.c | 28 +++++++++++++++++-----------
> 1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/lib/rhashtable.c b/lib/rhashtable.c
> index 5919d63..e96fc00 100644
> --- a/lib/rhashtable.c
> +++ b/lib/rhashtable.c
> @@ -552,8 +552,10 @@ static void rhashtable_wakeup_worker(struct rhashtable *ht)
> static void __rhashtable_insert(struct rhashtable *ht, struct rhash_head *obj,
> struct bucket_table *tbl, u32 hash)
> {
> - struct rhash_head *head = rht_dereference_bucket(tbl->buckets[hash],
> - tbl, hash);
> + struct rhash_head *head;
> +
> + hash = rht_bucket_index(tbl, hash);
> + head = rht_dereference_bucket(tbl->buckets[hash], tbl, hash);
>
> ASSERT_BUCKET_LOCK(ht, tbl, hash);
>
> @@ -593,7 +595,7 @@ void rhashtable_insert(struct rhashtable *ht, struct rhash_head *obj)
>
> tbl = rht_dereference_rcu(ht->future_tbl, ht);
> old_tbl = rht_dereference_rcu(ht->tbl, ht);
> - hash = head_hashfn(ht, tbl, obj);
> + hash = obj_raw_hashfn(ht, rht_obj(ht, obj));
>
> lock_buckets(tbl, old_tbl, hash);
> __rhashtable_insert(ht, obj, tbl, hash);
> @@ -627,8 +629,8 @@ bool rhashtable_remove(struct rhashtable *ht, struct rhash_head *obj)
> bool ret = false;
>
> rcu_read_lock();
> - tbl = old_tbl = rht_dereference_rcu(ht->tbl, ht);
> - new_tbl = rht_dereference_rcu(ht->future_tbl, ht);
> + old_tbl = rht_dereference_rcu(ht->tbl, ht);
> + tbl = new_tbl = rht_dereference_rcu(ht->future_tbl, ht);
> new_hash = obj_raw_hashfn(ht, rht_obj(ht, obj));
>
> lock_buckets(new_tbl, old_tbl, new_hash);
> @@ -643,15 +645,19 @@ restart:
>
> ASSERT_BUCKET_LOCK(ht, tbl, hash);
>
> - if (unlikely(new_tbl != tbl)) {
> - rht_for_each_continue(he2, he->next, tbl, hash) {
> + if (old_tbl->size > new_tbl->size && tbl == old_tbl &&
> + !rht_is_a_nulls(obj->next) &&
> + head_hashfn(ht, tbl, obj->next) != hash) {
> + rcu_assign_pointer(*pprev, (struct rhash_head *) rht_marker(ht, hash));
> + } else if (unlikely(old_tbl->size < new_tbl->size && tbl == new_tbl)) {
> + rht_for_each_continue(he2, obj->next, tbl, hash) {
> if (head_hashfn(ht, tbl, he2) == hash) {
> rcu_assign_pointer(*pprev, he2);
> goto found;
> }
> }
>
> - INIT_RHT_NULLS_HEAD(*pprev, ht, hash);
> + rcu_assign_pointer(*pprev, (struct rhash_head *) rht_marker(ht, hash));
> } else {
> rcu_assign_pointer(*pprev, obj->next);
> }
> @@ -666,8 +672,8 @@ found:
> * resizing. Thus traversing both is fine and the added cost is
> * very rare.
> */
> - if (tbl != new_tbl) {
> - tbl = new_tbl;
> + if (tbl != old_tbl) {
> + tbl = old_tbl;
> goto restart;
> }
>
> @@ -835,7 +841,7 @@ bool rhashtable_lookup_compare_insert(struct rhashtable *ht,
> rcu_read_lock();
> old_tbl = rht_dereference_rcu(ht->tbl, ht);
> new_tbl = rht_dereference_rcu(ht->future_tbl, ht);
> - new_hash = head_hashfn(ht, new_tbl, obj);
> + new_hash = obj_raw_hashfn(ht, rht_obj(ht, obj));
>
> lock_buckets(new_tbl, old_tbl, new_hash);
>
>
^ permalink raw reply
* Re: [PATCH next 4/6] bonding: Allow userspace to set system_priority
From: Jay Vosburgh @ 2015-02-08 7:46 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Andy Gospodarek, Andy Gospodarek, Veaceslav Falico,
Nikolay Aleksandrov, David Miller, netdev, Eric Dumazet
In-Reply-To: <CAF2d9ji-m6Suw_Rw6dBqs=Ky41h786rCrPh-J7tv7JA+4L84zw@mail.gmail.com>
Mahesh Bandewar <maheshb@google.com> wrote:
>On Fri, Feb 6, 2015 at 10:19 PM, Jay Vosburgh
><jay.vosburgh@canonical.com> wrote:
>> Andy Gospodarek <gospo@cumulusnetworks.com> wrote:
>>
>>>On Fri, Feb 06, 2015 at 04:51:54PM -0800, Mahesh Bandewar wrote:
>>>> This patch allows user to randomize the system-priority in an ad-system.
>>>> The allowed range is 1 - 0xFFFF while default value is 0xFFFF. If user
>>>> does not specify this value, the system defaults to 0xFFFF, which is
>>>> what it was before this patch.
>>>>
>>>> Following example code could set the value -
>>>> # modprobe bonding mode=4
>>>> # sys_prio=$(( 1 + RANDOM + RANDOM ))
>>>> # echo $sys_prio > /sys/class/net/bond0/bonding/ad_actor_system_priority
>>>
>>>If I can convince you to change 'ad_actor_system_macaddr' to something
>>>different can I also convince you to change this to something shorter,
>>>too? :)
>>>
>>>Maybe 'ad_sys_priority' or something?
>>
>> The name, verbose as it is, is from the 802.1AX standard, and
>> there's also a "partner_system_priority" (which is not a user-settable
>> thing, it's a field in the LACPDUs). My suggestion would therefore be
>> "ad_actor_sys_prio" for this one, as I think there's some value in
>> continuity with the names from the standard.
>>
>> The MAC address one in the standard is just "actor_system";
>> there's a "partner_system" here, too, which is also a field in the
>> LACPDU. I'm ok with calling that one just "actor_system," as presumably
>> anyone changing it will know what it means.
>>
>Thank you guys for the suggestions. I didn't like those very long
>names either but when there is something that already has name
>similar, I defaulted to being verbose. I will have the name changed to
>-
>
>ad_actor_system_priority - ad_actor_sys_prio
>ad_actor_system_mac_address - ad_actor_system
>ad_actor_user_port_key - ad_user_portkey
>
>Is this reasonable enough?
Perhaps nitpicking, but I'd call it ad_user_port_key.
I agree that this one should not have "actor" in it, as this
particular value is an invention of bonding and isn't directly part of
the standard. In bonding, the "user key" (always 0 prior to this
patch), speed, and duplex are used to generate the actor_admin_port_key
and actor_oper_port_key. Those latter two are part of the standard, but
have no required format.
-J
---
-Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* Re: [PATCH next 2/6] bonding: implement bond_poll_controller()
From: Jay Vosburgh @ 2015-02-08 7:31 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Veaceslav Falico, Andy Gospodarek, Nikolay Aleksandrov,
David Miller, netdev, Eric Dumazet
In-Reply-To: <CAF2d9jj_P5PA2Op-MD5+i0gZj2GW_nnbcMVsxu93kMvtwBNkYA@mail.gmail.com>
Mahesh Bandewar <maheshb@google.com> wrote:
>On Sat, Feb 7, 2015 at 1:04 AM, Veaceslav Falico <vfalico@gmail.com> wrote:
>> On Fri, Feb 06, 2015 at 04:51:51PM -0800, Mahesh Bandewar wrote:
>>>
>>> This patches implements the poll_controller support for all
>>> bonding driver. If the slaves have poll_controller net_op defined,
>>> this implementation calls them. This is mode agnostic implementation
>>> and iterates through all slaves (based on mode) and calls respective
>>> handler.
>>>
>>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>>
>>
>> Really good patchset, thank you. Two nitpicks below, nothing serious. Also,
>> please, add Doc/bonding.txt changes for every user-visible
>> change/enhancement, this doc is a go-to for a lot of users.
>>
>> Otherwise, with the doc and other issues, pointed by Jay and Andy, fixed, I
>> guess it's good to go. The 0/ patch would be welcome too, btw.
>>
>>
>>> ---
>>> drivers/net/bonding/bond_3ad.c | 24 +++++++++++++++++++++
>>> drivers/net/bonding/bond_main.c | 47
>>> +++++++++++++++++++++++++++++++++++++++++
>>> include/net/bond_3ad.h | 1 +
>>> 3 files changed, 72 insertions(+)
>>>
>>> diff --git a/drivers/net/bonding/bond_3ad.c
>>> b/drivers/net/bonding/bond_3ad.c
>>> index 9b436696b95e..14f2ebe786c5 100644
>>> --- a/drivers/net/bonding/bond_3ad.c
>>> +++ b/drivers/net/bonding/bond_3ad.c
>>> @@ -2477,6 +2477,30 @@ int bond_3ad_get_active_agg_info(struct bonding
>>> *bond, struct ad_info *ad_info)
>>> return ret;
>>> }
>>>
>>> +#define BOND_3AD_PORT_OPERATIONAL \
>>> + (AD_STATE_DISTRIBUTING | AD_STATE_COLLECTING | \
>>> + AD_STATE_SYNCHRONIZATION | AD_STATE_AGGREGATION)
>>> +
>>> +static int bond_3ad_port_operational(struct slave *slave)
>>> +{
>>> + port_t *port = &SLAVE_AD_INFO(slave)->port;
>>> +
>>> + return bond_slave_can_tx(slave) &&
>>> + (port->actor_oper_port_state &
>>> port->partner_oper.port_state &
>>> + BOND_3AD_PORT_OPERATIONAL) == BOND_3AD_PORT_OPERATIONAL;
>>> +}
>>> +
>>> +/* bond_3ad_port_is_active - check if a slave port is active or not. A
>>> port
>>> + * is active when it can forward traffic.
>>> + *
>>> + * @slave: slave port to check state for.
>>> + * Returns: 0 if not active else is active.
>>> + */
>>> +int bond_3ad_port_is_active(struct slave *slave)
>>> +{
>>> + return bond_3ad_port_operational(slave);
>>> +}
>>> +
>>> int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
>>> struct slave *slave)
>>> {
>>> diff --git a/drivers/net/bonding/bond_main.c
>>> b/drivers/net/bonding/bond_main.c
>>> index c9e519cb9214..a50ec87486f3 100644
>>> --- a/drivers/net/bonding/bond_main.c
>>> +++ b/drivers/net/bonding/bond_main.c
>>> @@ -928,6 +928,53 @@ static inline void slave_disable_netpoll(struct slave
>>> *slave)
>>>
>>> static void bond_poll_controller(struct net_device *bond_dev)
>>> {
>>> + struct bonding *bond = netdev_priv(bond_dev);
>>> + struct slave *slave = NULL;
>>> + struct list_head *iter;
>>> + struct ad_info ad_info;
>>> + struct aggregator *agg;
>>> + const struct net_device_ops *ops;
>>> + bool call_slave_netpoll;
>>> +
>>> + if (BOND_MODE(bond) == BOND_MODE_8023AD)
>>> + if (bond_3ad_get_active_agg_info(bond, &ad_info))
>>> + return;
>>> +
>>> + bond_for_each_slave(bond, slave, iter) {
>>> + call_slave_netpoll = false;
>>> + switch (BOND_MODE(bond)) {
>>> + case BOND_MODE_8023AD:
>>> + agg = SLAVE_AD_INFO(slave)->port.aggregator;
>>> + if (!bond_slave_is_up(slave))
>>
>>
>> bond_3ad_port_is_active() already contains the check for being up.
>>
>>> + break;
>>> + if (agg && agg->aggregator_identifier !=
>>
>>
>> Hm, should we poll it without an aggregator?
>>
>I think I missed this comment earlier, but it would be wrong. I mean
>we have to perform this check otherwise it would be wrong.
This looks to be a check to determine if the slave is a member
of the active aggregator. I don't believe we'll want to transmit on a
slave that's not a member of the active agg. FWIW, bond_3ad_xmit_xor
has similar logic.
Slaves will generally be a member of some aggregator (active or
otherwise), except for short periods of time immediately after
enslavement (before the next run of the state machine) and during port
selection logic during transit from one agg to another. Since the
port.aggregator can be NULL, I believe the test as-is is necessary.
-J
>>> + ad_info.aggregator_id)
>>> + break;
>>> + if (!bond_3ad_port_is_active(slave) &&
>>> + ad_info.ports != 1)
>>> + break;
>>> +
>>> + call_slave_netpoll = true;
>>> + break;
>>> + default:
>>> + if (bond_slave_is_up(slave))
>>> + call_slave_netpoll = true;
>>> + break;
>>> + }
>>> +
>>> + if (call_slave_netpoll) {
>>> + ops = slave->dev->netdev_ops;
>>> + if (ops->ndo_poll_controller) {
>>
>>
>> It's weird to see this check so down the road. Maybe reorg the logic to
>> something like:
>>
>> bond_for_each_slave() {
>> if (!bond_slave_is_up() || !bond_slave_has_ndo_poll())
>> continue;
>>
>> if (BOND_MODE(bond) == 3AD && !bond_3ad_port_is_active())
>> continue;
>>
>> slave-do_ndo_poll_stuff();
>> }
>>
>> Just as a thought, might be easier to read/shorter.
>>
>I tried this and it does save one bool variable but not much
>different. Here it is -
>
> bond_for_each_slave(bond, slave, iter) {
> ops = slave->dev->netdev_ops;
> if (!bond_slave_is_up(slave) || !ops->ndo_poll_controller)
> continue;
>
> if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> struct aggregator *agg =
> SLAVE_AD_INFO(slave)->port.aggregator;
>
> if (agg && agg->aggregator_identifier !=
> ad_info.aggregator_id)
> continue;
> if (!bond_3ad_port_is_active(slave) || ad_info.ports != 1)
> continue;
> }
>
> ni = rcu_dereference_bh(slave->dev->npinfo);
> if (down_trylock(&ni->dev_lock))
> continue;
> ops->ndo_poll_controller(slave->dev);
> up(&ni->dev_lock);
> }
>
>I'm not sure if it's any different! BTW I did keep the aggregator
>check since I'm not convinced that it's OK to eliminate the check.
>
>
>>
>>> + struct netpoll_info *ni =
>>> +
>>> rcu_dereference_bh(slave->dev->npinfo);
>>> +
>>> + if (down_trylock(&ni->dev_lock))
>>> + continue;
>>> + ops->ndo_poll_controller(slave->dev);
>>> + up(&ni->dev_lock);
>>> + }
>>> + }
>>> + }
>>> }
>>>
>>> static void bond_netpoll_cleanup(struct net_device *bond_dev)
>>> diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
>>> index f04cdbb7848e..6c455c646d61 100644
>>> --- a/include/net/bond_3ad.h
>>> +++ b/include/net/bond_3ad.h
>>> @@ -278,5 +278,6 @@ int bond_3ad_lacpdu_recv(const struct sk_buff *skb,
>>> struct bonding *bond,
>>> struct slave *slave);
>>> int bond_3ad_set_carrier(struct bonding *bond);
>>> void bond_3ad_update_lacp_rate(struct bonding *bond);
>>> +int bond_3ad_port_is_active(struct slave *slave);
>>> #endif /* _NET_BOND_3AD_H */
>>>
>>> --
>>> 2.2.0.rc0.207.ga3a616c
>>>
>>
^ permalink raw reply
* linux-next: build failure after merge of the net-next tree
From: Stephen Rothwell @ 2015-02-09 3:07 UTC (permalink / raw)
To: David Miller, netdev; +Cc: linux-next, linux-kernel, Herbert Xu
[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]
Hi all,
After merging the net-next tree, today's linux-next build (arm
multi_v7_defconfig) failed like this:
lib/rhashtable.c: In function 'rhashtable_walk_next':
lib/rhashtable.c:1002:3: error: implicit declaration of function 'ERR_PTR' [-Werror=implicit-function-declaration]
return ERR_PTR(-EAGAIN);
^
lib/rhashtable.c:1002:3: warning: return makes pointer from integer without a cast
Caused by commit f2dba9c6ff0d ("rhashtable: Introduce
rhashtable_walk_*"). See Rule 1 in Documentation/SubmitChecklist.
I have added the following fix patch for today:
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 9 Feb 2015 14:04:03 +1100
Subject: [PATCH] rhashtable: using ERR_PTR requires linux/err.h
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
lib/rhashtable.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index e96fc00208bc..9cc4c4a90d00 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -23,6 +23,7 @@
#include <linux/jhash.h>
#include <linux/random.h>
#include <linux/rhashtable.h>
+#include <linux/err.h>
#define HASH_DEFAULT_SIZE 64UL
#define HASH_MIN_SIZE 4UL
--
2.1.4
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ 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-09 3:46 UTC (permalink / raw)
To: Marcel Holtmann
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: <BD721CC9-DD4D-480D-98E6-89CA868BA285@holtmann.org>
On 2/9/15 04:17, Marcel Holtmann wrote:
> 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.
>
If it is really the API which can be used outside kernel, what you said
sounds reasonable to me. But I guess, except the related orgnizations/
company/members, most of kernel members can not give a suitable test:
- It is an API, but we only know kernel part implementation, and we
also know that the kernel part implementation intends to use
"unsigned long event_mask[2]" and "u32 *" type cast.
- We don't know the other part implementation (we event don't know
whether it is open source). And also it is out of most of kernel
members' current border (e.g. me).
- If the other part implementation match what kernel part has done, it
is OK, else it should cause issue.
So at present, in kernel part, we can only say the original authors
intended to do like this. And only within kernel part, it can not cause
issue. I guess, original authors originally knew what we talk about.
This patch is for fixing building warnings without any negative effect.
And most of us are not the suitable members to continue analyzing. So
at present, for me, we can accept this patch.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply
* [PATCH net-next] cxgb4: Fix trace observed while dumping clip_tbl
From: Hariprasad Shenai @ 2015-02-09 4:17 UTC (permalink / raw)
To: netdev; +Cc: davem, leedom, anish, nirranjan, kumaras, Hariprasad Shenai
Handle clip_tbl debugfs entry, when clip_tbl isn't allocated.
In commit b5a02f503caa0837 ("cxgb4: Update ipv6 address handling api") wrong
argument was passed for single_open for clip_tbl debugfs entry, which led to
below trace. Fixing it.
======
call Trace:
[<ffffffffa073c606>] clip_tbl_open+0x16/0x30 [cxgb4]
[<ffffffff8119e2fa>] do_dentry_open+0x21a/0x370
[<ffffffff8119e499>] vfs_open+0x49/0x50
[<ffffffff811b0d0e>] do_last+0x21e/0x800
[<ffffffff811b1382>] path_openat+0x92/0x470
[<ffffffff8110569f>] ? rb_reserve_next_event+0xaf/0x380
[<ffffffff8110569f>] ? rb_reserve_next_event+0xaf/0x380
[<ffffffff811b189a>] do_filp_open+0x4a/0xa0
[<ffffffff811bdc5d>] ? __alloc_fd+0xcd/0x140
[<ffffffff8119fa4a>] do_sys_open+0x11a/0x230
[<ffffffff8101219f>] ? syscall_trace_enter_phase2+0xaf/0x1b0
[<ffffffff8119fb9e>] SyS_open+0x1e/0x20
[<ffffffff815bf6f0>] tracesys_phase2+0xd4/0xd9
Code: 89 e5 66 66 66 66 90 48 8b 47 e0 48 8b 40 30 48 8b 40 58 c9 c3 66 0f 1f
84 00 00 00 00 00 55 48 89 e5 66 66 66 66 90 48 8b 47 e0 <48> 8b 40 58 c9 c3 66
66 66 66 2e 0f 1f 84 00 00 00 00 00 55 48
RIP [<ffffffff8120898d>] PDE_DATA+0xd/0x20
RSP <ffff8800b08c3c48>
CR2: 0000000000000058
=====
Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c | 3 +++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 2 +-
2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c b/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c
index 2b407b6..9062a84 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c
@@ -81,6 +81,9 @@ int cxgb4_clip_get(const struct net_device *dev, const u32 *lip, u8 v6)
int addr_len;
int ret = 0;
+ if (!ctbl)
+ return 0;
+
if (v6)
addr_len = 16;
else
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
index a1029ee..d221f6b 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
@@ -1237,7 +1237,7 @@ DEFINE_SIMPLE_DEBUGFS_FILE(sensors);
#if IS_ENABLED(CONFIG_IPV6)
static int clip_tbl_open(struct inode *inode, struct file *file)
{
- return single_open(file, clip_tbl_show, PDE_DATA(inode));
+ return single_open(file, clip_tbl_show, inode->i_private);
}
static const struct file_operations clip_tbl_debugfs_fops = {
--
1.7.1
^ permalink raw reply related
* [PATCH net-next] net:rfs: adjust table size checking
From: Eric Dumazet @ 2015-02-09 4:39 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
Make sure root user does not try something stupid.
Also make sure mask field in struct rps_sock_flow_table
does not share a cache line with the potentially often dirtied
flow table.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Fixes: 567e4b79731c ("net: rfs: add hash collision detection")
---
include/linux/netdevice.h | 3 ++-
net/core/sysctl_net_core.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ab3b7cef4638ceba92b749bc35564a60b0ff2a75..d115256ed5a209fe28ce971bdbde7ca421d048aa 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -653,7 +653,8 @@ struct rps_dev_flow_table {
*/
struct rps_sock_flow_table {
u32 mask;
- u32 ents[0];
+
+ u32 ents[0] ____cacheline_aligned_in_smp;
};
#define RPS_SOCK_FLOW_TABLE_SIZE(_num) (offsetof(struct rps_sock_flow_table, ents[_num]))
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 7a31be5e361fbb06d4a5063bf68da507bab3e8ec..eaa51ddf2368747c21399bf44f5a1993c0a0e39b 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -52,7 +52,7 @@ static int rps_sock_flow_sysctl(struct ctl_table *table, int write,
if (write) {
if (size) {
- if (size > 1<<30) {
+ if (size > 1<<29) {
/* Enforce limit to prevent overflow */
mutex_unlock(&sock_flow_mutex);
return -EINVAL;
^ permalink raw reply related
* [PATCH v2] brcmfmac: avoid duplicated suspend/resume operation
From: Fu, Zhonghui @ 2015-02-09 4:43 UTC (permalink / raw)
To: Kalle Valo, brudley, Arend van Spriel, Franky Lin, meuleman,
linville, pieterpg, hdegoede, wens, linux-wireless,
brcm80211-dev-list, netdev, linux-kernel@vger.kernel.org
>From ff39ed4af9f1c50358fe92ec4c8eaac9db183e00 Mon Sep 17 00:00:00 2001
From: Zhonghui Fu <zhonghui.fu@linux.intel.com>
Date: Mon, 26 Jan 2015 10:13:21 +0800
Subject: [PATCH v2] brcmfmac: avoid duplicated suspend/resume operation
WiFi chip has 2 SDIO functions, and PM core will trigger
twice suspend/resume operations for one WiFi chip to do
the same things. This patch avoid this case.
Acked-by: Arend van Spriel<arend@broadcom.com>
Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
---
Changes in v2:
- Remove two "Acked-by" lines
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 9880dae..618b545 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -1139,11 +1139,17 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
static int brcmf_ops_sdio_suspend(struct device *dev)
{
struct brcmf_bus *bus_if = dev_get_drvdata(dev);
- struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+ struct brcmf_sdio_dev *sdiodev;
mmc_pm_flag_t sdio_flags;
+ struct sdio_func *func = dev_to_sdio_func(dev);
brcmf_dbg(SDIO, "Enter\n");
+ if (func->num == 2)
+ return 0;
+
+ sdiodev = bus_if->bus_priv.sdio;
+
atomic_set(&sdiodev->suspend, true);
if (sdiodev->wowl_enabled) {
@@ -1164,9 +1170,16 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
static int brcmf_ops_sdio_resume(struct device *dev)
{
struct brcmf_bus *bus_if = dev_get_drvdata(dev);
- struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+ struct brcmf_sdio_dev *sdiodev;
+ struct sdio_func *func = dev_to_sdio_func(dev);
brcmf_dbg(SDIO, "Enter\n");
+
+ if (func->num == 2)
+ return 0;
+
+ sdiodev = bus_if->bus_priv.sdio;
+
if (sdiodev->pdata && sdiodev->pdata->oob_irq_supported)
disable_irq_wake(sdiodev->pdata->oob_irq_nr);
brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS);
-- 1.7.1
^ permalink raw reply related
* Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation
From: Fu, Zhonghui @ 2015-02-09 4:49 UTC (permalink / raw)
To: Arend van Spriel
Cc: brudley, Franky Lin, meuleman, Kalle Valo, linville, pieterpg,
hdegoede, wens, linux-wireless, brcm80211-dev-list, netdev,
linux-kernel@vger.kernel.org
In-Reply-To: <54D35814.4070503@broadcom.com>
I have re-sent the patch with the subject "[PATCH v2] brcmfmac: avoid duplicated suspend/resume operation" in another mail.
If this patch can be accepted, please tell me.
Thanks,
Zhonghui
On 2015/2/5 19:46, Arend van Spriel wrote:
> On 02/05/15 12:34, Fu, Zhonghui wrote:
>> What comments about the new patch? Can this new patch be accepted?
>
> Hi Zhonghui
>
> Last reply from Kalle was that it did not apply to his tree and recommended to use version numbering so "[PATCH V2] <subject>".
>
>> Thanks,
>> Zhonghui
>>
>> On 2015/1/26 10:46, Fu, Zhonghui wrote:
>>> From ff39ed4af9f1c50358fe92ec4c8eaac9db183e00 Mon Sep 17 00:00:00 2001
>>> From: Zhonghui Fu<zhonghui.fu@linux.intel.com>
>>> Date: Mon, 26 Jan 2015 10:13:21 +0800
>>> Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation
>>>
>>> WiFi chip has 2 SDIO functions, and PM core will trigger
>>> twice suspend/resume operations for one WiFi chip to do
>>> the same things. This patch avoid this case.
>>>
>>> Acked-by: Arend van Spriel<arend@broadcom.com>
>>> Signed-off-by: Zhonghui Fu<zhonghui.fu@linux.intel.com>
>>> ---
> And when using version info a change log here is even better. Although admittedly I lost track which version this would be ;-)
>
> Regards,
> Arend
> ---
>>> drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 17 +++++++++++++++--
>>> 1 files changed, 15 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>> index 9880dae..618b545 100644
>>> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>> @@ -1139,11 +1139,17 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
>>> static int brcmf_ops_sdio_suspend(struct device *dev)
>>> {
>>> struct brcmf_bus *bus_if = dev_get_drvdata(dev);
>>> - struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
>>> + struct brcmf_sdio_dev *sdiodev;
>>> mmc_pm_flag_t sdio_flags;
>>> + struct sdio_func *func = dev_to_sdio_func(dev);
>>>
>>> brcmf_dbg(SDIO, "Enter\n");
>>>
>>> + if (func->num == 2)
>>> + return 0;
>>> +
>>> + sdiodev = bus_if->bus_priv.sdio;
>>> +
>>> atomic_set(&sdiodev->suspend, true);
>>>
>>> if (sdiodev->wowl_enabled) {
>>> @@ -1164,9 +1170,16 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
>>> static int brcmf_ops_sdio_resume(struct device *dev)
>>> {
>>> struct brcmf_bus *bus_if = dev_get_drvdata(dev);
>>> - struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
>>> + struct brcmf_sdio_dev *sdiodev;
>>> + struct sdio_func *func = dev_to_sdio_func(dev);
>>>
>>> brcmf_dbg(SDIO, "Enter\n");
>>> +
>>> + if (func->num == 2)
>>> + return 0;
>>> +
>>> + sdiodev = bus_if->bus_priv.sdio;
>>> +
>>> if (sdiodev->pdata&& sdiodev->pdata->oob_irq_supported)
>>> disable_irq_wake(sdiodev->pdata->oob_irq_nr);
>>> brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS);
>>> -- 1.7.1
>>>
>>
>
^ permalink raw reply
* Re: [PATCH 3/3][v2] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions
From: Alexandre Courbot @ 2015-02-09 5:18 UTC (permalink / raw)
To: Rojhalat Ibrahim
Cc: linux-gpio@vger.kernel.org, Alexandre Courbot, Linus Walleij,
David Miller, netdev
In-Reply-To: <2017147.PF4WHTmJS4@pcimr>
On Thu, Jan 22, 2015 at 1:46 AM, Rojhalat Ibrahim <imr@rtschenk.de> wrote:
> Use the new gpiod_get_array and gpiod_put_array functions for obtaining and
> disposing of GPIO descriptors.
>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
> ---
> This patch depends on my previous patch "gpiolib: add gpiod_get_array and
> gpiod_put_array functions".
>
> v2: use the new interface
>
> Only compile-tested.
>
> drivers/net/phy/mdio-mux-gpio.c | 61 +++++++++++-----------------------------
> 1 file changed, 18 insertions(+), 43 deletions(-)
Oh yeah.
It would be great to have a tested-by for the next revision though.
Rojhalat, have you been able to test these new interfaces outside of
this driver?
^ permalink raw reply
* [GIT PULL nf] IPVS Fixes for v3.20
From: Simon Horman @ 2015-02-09 5:25 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Simon Horman
Hi Pablo,
please consider this fix for v3.20
This patch prevents the kernel getting into a situation whereby a
real-server may not be removed from a heterogeneous IPVS virtual server.
This problem was introduced by bc18d37f676f ("ipvs: Allow heterogeneous
pools now that we support them") in v3.18. This patch seems appropriate
for stable and I have checked that it applies cleanly to both v3.19 and
v3.18.6.
The following changes since commit 42b5212fee4f57907e9415b18fe19c13e65574bc:
xen-netback: stop the guest rx thread after a fatal error (2015-02-02 19:39:04 -0800)
are available in the git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git tags/ipvs-fixes-for-v3.20
for you to fetch changes up to dd3733b3e798daf778a1ec08557f388f00fdc2f6:
ipvs: fix inability to remove a mixed-family RS (2015-02-09 14:13:30 +0900)
----------------------------------------------------------------
Alexey Andriyanov (1):
ipvs: fix inability to remove a mixed-family RS
net/netfilter/ipvs/ip_vs_ctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
^ permalink raw reply
* [PATCH nf] ipvs: fix inability to remove a mixed-family RS
From: Simon Horman @ 2015-02-09 5:25 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Alexey Andriyanov, Simon Horman
In-Reply-To: <1423459508-15517-1-git-send-email-horms@verge.net.au>
From: Alexey Andriyanov <alan@al-an.info>
The current code prevents any operation with a mixed-family dest
unless IP_VS_CONN_F_TUNNEL flag is set. The problem is that it's impossible
for the client to follow this rule, because ip_vs_genl_parse_dest does
not even read the destination conn_flags when cmd = IPVS_CMD_DEL_DEST
(need_full_dest = 0).
Also, not every client can pass this flag when removing a dest. ipvsadm,
for example, does not support the "-i" command line option together with
the "-d" option.
This change disables any checks for mixed-family on IPVS_CMD_DEL_DEST command.
Signed-off-by: Alexey Andriyanov <alan@al-an.info>
Fixes: bc18d37f676f ("ipvs: Allow heterogeneous pools now that we support them")
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_ctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index b8295a4..fdcda8b 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -3399,7 +3399,7 @@ static int ip_vs_genl_set_cmd(struct sk_buff *skb, struct genl_info *info)
if (udest.af == 0)
udest.af = svc->af;
- if (udest.af != svc->af) {
+ if (udest.af != svc->af && cmd != IPVS_CMD_DEL_DEST) {
/* The synchronization protocol is incompatible
* with mixed family services
*/
--
2.1.4
^ permalink raw reply related
* Re: [PATCH next 4/6] bonding: Allow userspace to set system_priority
From: Mahesh Bandewar @ 2015-02-09 5:44 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Andy Gospodarek, Andy Gospodarek, Veaceslav Falico,
Nikolay Aleksandrov, David Miller, netdev, Eric Dumazet
In-Reply-To: <13353.1423381584@famine>
On Sat, Feb 7, 2015 at 11:46 PM, Jay Vosburgh
<jay.vosburgh@canonical.com> wrote:
> Mahesh Bandewar <maheshb@google.com> wrote:
>
>>On Fri, Feb 6, 2015 at 10:19 PM, Jay Vosburgh
>><jay.vosburgh@canonical.com> wrote:
>>> Andy Gospodarek <gospo@cumulusnetworks.com> wrote:
>>>
>>>>On Fri, Feb 06, 2015 at 04:51:54PM -0800, Mahesh Bandewar wrote:
>>>>> This patch allows user to randomize the system-priority in an ad-system.
>>>>> The allowed range is 1 - 0xFFFF while default value is 0xFFFF. If user
>>>>> does not specify this value, the system defaults to 0xFFFF, which is
>>>>> what it was before this patch.
>>>>>
>>>>> Following example code could set the value -
>>>>> # modprobe bonding mode=4
>>>>> # sys_prio=$(( 1 + RANDOM + RANDOM ))
>>>>> # echo $sys_prio > /sys/class/net/bond0/bonding/ad_actor_system_priority
>>>>
>>>>If I can convince you to change 'ad_actor_system_macaddr' to something
>>>>different can I also convince you to change this to something shorter,
>>>>too? :)
>>>>
>>>>Maybe 'ad_sys_priority' or something?
>>>
>>> The name, verbose as it is, is from the 802.1AX standard, and
>>> there's also a "partner_system_priority" (which is not a user-settable
>>> thing, it's a field in the LACPDUs). My suggestion would therefore be
>>> "ad_actor_sys_prio" for this one, as I think there's some value in
>>> continuity with the names from the standard.
>>>
>>> The MAC address one in the standard is just "actor_system";
>>> there's a "partner_system" here, too, which is also a field in the
>>> LACPDU. I'm ok with calling that one just "actor_system," as presumably
>>> anyone changing it will know what it means.
>>>
>>Thank you guys for the suggestions. I didn't like those very long
>>names either but when there is something that already has name
>>similar, I defaulted to being verbose. I will have the name changed to
>>-
>>
>>ad_actor_system_priority - ad_actor_sys_prio
>>ad_actor_system_mac_address - ad_actor_system
>>ad_actor_user_port_key - ad_user_portkey
>>
>>Is this reasonable enough?
>
> Perhaps nitpicking, but I'd call it ad_user_port_key.
>
> I agree that this one should not have "actor" in it, as this
> particular value is an invention of bonding and isn't directly part of
> the standard. In bonding, the "user key" (always 0 prior to this
> patch), speed, and duplex are used to generate the actor_admin_port_key
> and actor_oper_port_key. Those latter two are part of the standard, but
> have no required format.
>
Actually I was thinking of making ad_actor_sysprio instead of making
ad_user_port_key (feels like having more underscores makes it
unnecessary longer). That way all three look similar. So
ad_actor_sysprio
ad_actor_system
ad_user_portkey
All carry the same theme of meaning. Otherwise we could do something like -
ad_actor_sys_prio
ad_actor_sys_mac
ad_user_port_key
Which one seems more logical / reasonable?
> -J
>
> ---
> -Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* Re: linux-next: build failure after merge of the net-next tree
From: David Miller @ 2015-02-09 5:52 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-next, linux-kernel, herbert
In-Reply-To: <20150209140745.24859389@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 9 Feb 2015 14:07:45 +1100
> diff --git a/lib/rhashtable.c b/lib/rhashtable.c
> index e96fc00208bc..9cc4c4a90d00 100644
> --- a/lib/rhashtable.c
> +++ b/lib/rhashtable.c
> @@ -23,6 +23,7 @@
> #include <linux/jhash.h>
> #include <linux/random.h>
> #include <linux/rhashtable.h>
> +#include <linux/err.h>
>
> #define HASH_DEFAULT_SIZE 64UL
> #define HASH_MIN_SIZE 4UL
Applied, thanks Stephen.
^ permalink raw reply
* [ANNOUNCE] ipvsadm release v1.28
From: Jesper Dangaard Brouer @ 2015-02-09 5:55 UTC (permalink / raw)
To: lvs-devel, lvs-users
Cc: brouer, netdev, linux-kernel, Ryan O'Hara, Julian Anastasov,
overcastsky.zhao, formorer, mt, logan, support, Daniel Borkmann,
Wensong Zhang, Simon Horman, Alex Gartrell
We are happy to announce the release of ipvsadm v1.28.
ipvsadm is a utility to administer the kernels IPVS/LVS load-balancer service
It has been quite a while since the previous release, v1.27. A number of
fixes and improvements have been made; most noticeably in the area of IPv6.
One big addition is the support for heterogeneous pools (v4 and v6
mixed pools in 3.18), by Alex Gartrell (facebook).
Feature wise the SCTP protocol has been added which is available via the
cmdline parameter "--sctp-service". The kernel has supported SCTP since
kernel 2.6.34 but up until now ipvsadm users have been restricted to using
fwmark-based virtual services for SCTP.
This release is based on the kernel.org git tree:
https://git.kernel.org/cgit/utils/kernel/ipvsadm/ipvsadm.git/
You can download the tarballs from:
https://kernel.org/pub/linux/utils/kernel/ipvsadm/
Git tree:
git://git.kernel.org/pub/scm/utils/kernel/ipvsadm/ipvsadm.git
Shortlog:
Alex Gartrell (2):
ipvsadm: specify real server address family to netlink socket
ipvsadm: do not truncate ipv6 members of v4 services
Daniel Borkmann (2):
ipvsadm: fix compile warning in print_largenum
ipvsadm: fix compile warning in modprobe_ipvs
Hibari Michiro (1):
ipvsadm: enable displaying of IPv6 hostnames in listing
Jesper Dangaard Brouer (1):
Release: Version 1.28
Julian Anastasov (3):
ipvsadm: restrict different address family
ipvsadm: allow different address family in connection listing
ipvsadm: add SCTP support
Ryan O'Hara (2):
libipvs: Initialize ipvs_service_t variable
ipvsadm: Fix list daemon to show backup daemon
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ 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