* stmmac mitigation and ethtool coalesce parameters
From: Giuseppe CAVALLARO @ 2012-08-22 7:14 UTC (permalink / raw)
To: ML netdev
Hello
I'm reworking the mitigation schema in the stmmac and removing the old
and dead STMMAC_TIMER code.
On new chips we have an HW RX-Watchdog that can be used for mitigating
the Rx-interrupts and first results look promising.
Before posting the patches I have a doubt about the ethtool coalesce
parameters to use in the driver to tune the following parameters:
- On Rx-side I have:
a parameter that is the RI Watchdog Timer count. It indicates the
number of system clock cycles.
- On Tx-side, the mitigation schema I'm using is based on a SW timer
that calls the tx function (stmmac_tx) to reclaim the resource after
transmitting the frames.
Also there is another parameter (like a threshold) used to program
the descriptors avoiding to set the interrupt on completion bit in
when the frame is sent (xmit). This means that the stmmac_tx can be
called by the ISR too. This approach is showing really good figures
as well.
so I wonder which are the appropriate ethtool coalesce parameters to
use? Or could I tune them by using sysfs entries?
Welcome advice.
Peppe
^ permalink raw reply
* Re[2]: [PATCH 05/19] netfilter: nf_conntrack_ipv6: improve fragmentation handling
From: Julian Anastasov @ 2012-08-22 7:07 UTC (permalink / raw)
To: Hans Schillstrom
Cc: Jesper Dangaard Brouer, Patrick McHardy, netfilter-devel, netdev,
Hans Schillstrom
In-Reply-To: <mdaatbj.b2f3561b17fe4f00a6e947611a0959a1@obelix.schillstrom.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 532 bytes --]
Hello,
On Wed, 22 Aug 2012, Hans Schillstrom wrote:
> >Perhaps we could change/fix the MTU check in IPVS?
> >(This would also solve issues I've seen with TSO/GSO frames, hitting
> >this code path).
> >
> I ran into this as well,
> try this for the mtu check.
>
> if ((!skb->local_df && skb->len > mtu && !skb_is_gso(skb)) ||
> (IP6CB(skb)->frag_max_size && IP6CB(skb)->frag_max_size > mtu)) {
Better without local_df check, it is our job to
set it.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH net-next] packet: Protect packet sk list with mutex (v2)
From: Eric Dumazet @ 2012-08-22 6:59 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: David Miller, Linux Netdev List
In-Reply-To: <50336BC7.7030209@parallels.com>
On Tue, 2012-08-21 at 15:06 +0400, Pavel Emelyanov wrote:
> Change since v1:
>
> * Fixed inuse counters access spotted by Eric
>
> In patch eea68e2f (packet: Report socket mclist info via diag module) I've
> introduced a "scheduling in atomic" problem in packet diag module -- the
> socket list is traversed under rcu_read_lock() while performed under it sk
> mclist access requires rtnl lock (i.e. -- mutex) to be taken.
>
> [152363.820563] BUG: scheduling while atomic: crtools/12517/0x10000002
> [152363.820573] 4 locks held by crtools/12517:
> [152363.820581] #0: (sock_diag_mutex){+.+.+.}, at: [<ffffffff81a2dcb5>] sock_diag_rcv+0x1f/0x3e
> [152363.820613] #1: (sock_diag_table_mutex){+.+.+.}, at: [<ffffffff81a2de70>] sock_diag_rcv_msg+0xdb/0x11a
> [152363.820644] #2: (nlk->cb_mutex){+.+.+.}, at: [<ffffffff81a67d01>] netlink_dump+0x23/0x1ab
> [152363.820693] #3: (rcu_read_lock){.+.+..}, at: [<ffffffff81b6a049>] packet_diag_dump+0x0/0x1af
>
> Similar thing was then re-introduced by further packet diag patches (fanount
> mutex and pgvec mutex for rings) :(
>
> Apart from being terribly sorry for the above, I propose to change the packet
> sk list protection from spinlock to mutex. This lock currently protects two
> modifications:
>
> * sklist
> * prot inuse counters
>
> The sklist modifications can be just reprotected with mutex since they already
> occur in a sleeping context. The inuse counters modifications are trickier -- the
> __this_cpu_-s are used inside, thus requiring the caller to handle the potential
> issues with contexts himself. Since packet sockets' counters are modified in two
> places only (packet_create and packet_release) we only need to protect the context
> from being preempted. BH disabling is not required in this case.
>
> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
>
> ---
>
> diff --git a/include/net/netns/packet.h b/include/net/netns/packet.h
> index cb4e894..4780b08 100644
> --- a/include/net/netns/packet.h
> +++ b/include/net/netns/packet.h
> @@ -8,7 +8,7 @@
> #include <linux/spinlock.h>
>
> struct netns_packet {
> - spinlock_t sklist_lock;
> + struct mutex sklist_lock;
> struct hlist_head sklist;
> };
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 226b2cd..79bc69c 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -2308,10 +2308,13 @@ static int packet_release(struct socket *sock)
> net = sock_net(sk);
> po = pkt_sk(sk);
>
> - spin_lock_bh(&net->packet.sklist_lock);
> + mutex_lock(&net->packet.sklist_lock);
> sk_del_node_init_rcu(sk);
> + mutex_unlock(&net->packet.sklist_lock);
I am still a bit uncomfortable : are we allowed to sleep in a release()
handler ?
It seems yes, so :
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH] ipv4: properly update pmtu
From: Eric Dumazet @ 2012-08-22 6:48 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Julian Anastasov, Sylvain Munaut
From: Eric Dumazet <edumazet@google.com>
Sylvain Munault reported following info :
- TCP connection get "stuck" with data in send queue when doing
"large" transfers ( like typing 'ps ax' on a ssh connection )
- Only happens on path where the PMTU is lower than the MTU of
the interface
- Is not present right after boot, it only appears 10-20min after
boot or so. (and that's inside the _same_ TCP connection, it works
fine at first and then in the same ssh session, it'll get stuck)
- Definitely seems related to fragments somehow since I see a router
sending ICMP message saying fragmentation is needed.
- Exact same setup works fine with kernel 3.5.1
Problem happens when the 10 minutes (ip_rt_mtu_expires) expiration
period is over.
ip_rt_update_pmtu() calls dst_set_expires() to rearm a new expiration,
but dst_set_expires() does nothing because dst.expires is already set.
It seems we want to set the expires field to a new value, regardless
of prior one.
With help from Julian Anastasov.
Reported-by: Sylvain Munaut <s.munaut@whatever-company.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
CC: Julian Anastasov <ja@ssi.bg>
---
net/ipv4/route.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index e4ba974..8d6d320 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -956,7 +956,7 @@ static void ip_rt_update_pmtu(struct dst_entry *dst, struct sock *sk,
dst->obsolete = DST_OBSOLETE_KILL;
} else {
rt->rt_pmtu = mtu;
- dst_set_expires(&rt->dst, ip_rt_mtu_expires);
+ rt->dst.expires = max(1UL, jiffies + ip_rt_mtu_expires);
}
}
^ permalink raw reply related
* [PATCH v8] bonding: support for IPv6 transmit hashing
From: John Eaglesham @ 2012-08-22 6:43 UTC (permalink / raw)
To: netdev; +Cc: John Eaglesham
In-Reply-To: <2e01d8f94c42c61af9886683a4c35caf6816bc3d.1345610688.git.linux@8192.net>
From: John Eaglesham <linux@8192.net>
Currently the "bonding" driver does not support load balancing outgoing
traffic in LACP mode for IPv6 traffic. IPv4 (and TCP or UDP over IPv4)
are currently supported; this patch adds transmit hashing for IPv6 (and
TCP or UDP over IPv6), bringing IPv6 up to par with IPv4 support in the
bonding driver. In addition, bounds checking has been added to all
transmit hashing functions.
The algorithm chosen (xor'ing the bottom three quads of the source and
destination addresses together, then xor'ing each byte of that result into
the bottom byte, finally xor'ing with the last bytes of the MAC addresses)
was selected after testing almost 400,000 unique IPv6 addresses harvested
from server logs. This algorithm had the most even distribution for both
big- and little-endian architectures while still using few instructions. Its
behavior also attempts to closely match that of the IPv4 algorithm.
The IPv6 flow label was intentionally not included in the hash as it appears
to be unset in the vast majority of IPv6 traffic sampled, and the current
algorithm not using the flow label already offers a very even distribution.
Fragmented IPv6 packets are handled the same way as fragmented IPv4 packets,
ie, they are not balanced based on layer 4 information. Additionally,
IPv6 packets with intermediate headers are not balanced based on layer
4 information. In practice these intermediate headers are not common and
this should not cause any problems, and the alternative (a packet-parsing
loop and look-up table) seemed slow and complicated for little gain.
Tested-by: John Eaglesham <linux@8192.net>
Signed-off-by: John Eaglesham <linux@8192.net>
---
Changes:
v2)
* Clarify description
* Add bounds checking to more functions
* All functions call bond_xmit_hash_policy_l2 rather than re-
implement the same logic.
v3)
* Patch against net-next.
* Style corrections.
v4)
* Correct indenting.
v5)
* Squash documentation and code patches into one.
v6)
* Modify IPv6 hash to behave more like the IPv4 hash, update
documentation with modified algorithm.
* Clean up formatting.
* Move all variable declaration to the top of the function.
* Minor change to IPv6 layer 4 hash to match IPv4 hash behavior
(mix all hashed address bits together rather than just the
bottom 24 bits).
v7)
* Improve bounds checking code (handle truncated IPv6 header,
removed goto, fewer if statements).
* Re-write pseudocode in documentation to match actual code more
closely.
* Correct indenting, align parentheses, wrap code at <= 80 columns
(based on Jay's changes).
v8)
* Correct patch submission format.
Documentation/networking/bonding.txt | 30 ++++++++++--
drivers/net/bonding/bond_main.c | 89 +++++++++++++++++++++++++-----------
2 files changed, 88 insertions(+), 31 deletions(-)
diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 6b1c711..10a015c 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -752,12 +752,22 @@ xmit_hash_policy
protocol information to generate the hash.
Uses XOR of hardware MAC addresses and IP addresses to
- generate the hash. The formula is
+ generate the hash. The IPv4 formula is
(((source IP XOR dest IP) AND 0xffff) XOR
( source MAC XOR destination MAC ))
modulo slave count
+ The IPv6 formula is
+
+ hash = (source ip quad 2 XOR dest IP quad 2) XOR
+ (source ip quad 3 XOR dest IP quad 3) XOR
+ (source ip quad 4 XOR dest IP quad 4)
+
+ (((hash >> 24) XOR (hash >> 16) XOR (hash >> 8) XOR hash)
+ XOR (source MAC XOR destination MAC))
+ modulo slave count
+
This algorithm will place all traffic to a particular
network peer on the same slave. For non-IP traffic,
the formula is the same as for the layer2 transmit
@@ -778,19 +788,29 @@ xmit_hash_policy
slaves, although a single connection will not span
multiple slaves.
- The formula for unfragmented TCP and UDP packets is
+ The formula for unfragmented IPv4 TCP and UDP packets is
((source port XOR dest port) XOR
((source IP XOR dest IP) AND 0xffff)
modulo slave count
- For fragmented TCP or UDP packets and all other IP
- protocol traffic, the source and destination port
+ The formula for unfragmented IPv6 TCP and UDP packets is
+
+ hash = (source port XOR dest port) XOR
+ ((source ip quad 2 XOR dest IP quad 2) XOR
+ (source ip quad 3 XOR dest IP quad 3) XOR
+ (source ip quad 4 XOR dest IP quad 4))
+
+ ((hash >> 24) XOR (hash >> 16) XOR (hash >> 8) XOR hash)
+ modulo slave count
+
+ For fragmented TCP or UDP packets and all other IPv4 and
+ IPv6 protocol traffic, the source and destination port
information is omitted. For non-IP traffic, the
formula is the same as for the layer2 transmit hash
policy.
- This policy is intended to mimic the behavior of
+ The IPv4 policy is intended to mimic the behavior of
certain switches, notably Cisco switches with PFC2 as
well as some Foundry and IBM products.
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index d95fbc3..4221e57 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3354,56 +3354,93 @@ static struct notifier_block bond_netdev_notifier = {
/*---------------------------- Hashing Policies -----------------------------*/
/*
+ * Hash for the output device based upon layer 2 data
+ */
+static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
+{
+ struct ethhdr *data = (struct ethhdr *)skb->data;
+
+ if (skb_headlen(skb) >= offsetof(struct ethhdr, h_proto))
+ return (data->h_dest[5] ^ data->h_source[5]) % count;
+
+ return 0;
+}
+
+/*
* Hash for the output device based upon layer 2 and layer 3 data. If
- * the packet is not IP mimic bond_xmit_hash_policy_l2()
+ * the packet is not IP, fall back on bond_xmit_hash_policy_l2()
*/
static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count)
{
struct ethhdr *data = (struct ethhdr *)skb->data;
- struct iphdr *iph = ip_hdr(skb);
-
- if (skb->protocol == htons(ETH_P_IP)) {
+ struct iphdr *iph;
+ struct ipv6hdr *ipv6h;
+ u32 v6hash;
+ __be32 *s, *d;
+
+ if (skb->protocol == htons(ETH_P_IP) &&
+ skb_network_header_len(skb) >= sizeof(*iph)) {
+ iph = ip_hdr(skb);
return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^
(data->h_dest[5] ^ data->h_source[5])) % count;
+ } else if (skb->protocol == htons(ETH_P_IPV6) &&
+ skb_network_header_len(skb) >= sizeof(*ipv6h)) {
+ ipv6h = ipv6_hdr(skb);
+ s = &ipv6h->saddr.s6_addr32[0];
+ d = &ipv6h->daddr.s6_addr32[0];
+ v6hash = (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
+ v6hash ^= (v6hash >> 24) ^ (v6hash >> 16) ^ (v6hash >> 8);
+ return (v6hash ^ data->h_dest[5] ^ data->h_source[5]) % count;
}
- return (data->h_dest[5] ^ data->h_source[5]) % count;
+ return bond_xmit_hash_policy_l2(skb, count);
}
/*
* Hash for the output device based upon layer 3 and layer 4 data. If
* the packet is a frag or not TCP or UDP, just use layer 3 data. If it is
- * altogether not IP, mimic bond_xmit_hash_policy_l2()
+ * altogether not IP, fall back on bond_xmit_hash_policy_l2()
*/
static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
{
- struct ethhdr *data = (struct ethhdr *)skb->data;
- struct iphdr *iph = ip_hdr(skb);
- __be16 *layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
- int layer4_xor = 0;
-
- if (skb->protocol == htons(ETH_P_IP)) {
+ u32 layer4_xor = 0;
+ struct iphdr *iph;
+ struct ipv6hdr *ipv6h;
+ __be32 *s, *d;
+ __be16 *layer4hdr;
+
+ if (skb->protocol == htons(ETH_P_IP) &&
+ skb_network_header_len(skb) >= sizeof(*iph)) {
+ iph = ip_hdr(skb);
if (!ip_is_fragment(iph) &&
(iph->protocol == IPPROTO_TCP ||
- iph->protocol == IPPROTO_UDP)) {
- layer4_xor = ntohs((*layer4hdr ^ *(layer4hdr + 1)));
+ iph->protocol == IPPROTO_UDP) &&
+ (skb_headlen(skb) - skb_network_offset(skb) >=
+ iph->ihl * sizeof(u32) + sizeof(*layer4hdr) * 2)) {
+ layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
+ layer4_xor = ntohs(*layer4hdr ^ *(layer4hdr + 1));
}
return (layer4_xor ^
((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
-
+ } else if (skb->protocol == htons(ETH_P_IPV6) &&
+ skb_network_header_len(skb) >= sizeof(*ipv6h)) {
+ ipv6h = ipv6_hdr(skb);
+ if ((ipv6h->nexthdr == IPPROTO_TCP ||
+ ipv6h->nexthdr == IPPROTO_UDP) &&
+ (skb_headlen(skb) - skb_network_offset(skb) >=
+ sizeof(*ipv6h) + sizeof(*layer4hdr) * 2)) {
+ layer4hdr = (__be16 *)(ipv6h + 1);
+ layer4_xor = ntohs(*layer4hdr ^ *(layer4hdr + 1));
+ }
+ s = &ipv6h->saddr.s6_addr32[0];
+ d = &ipv6h->daddr.s6_addr32[0];
+ layer4_xor ^= (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
+ layer4_xor ^= (layer4_xor >> 24) ^ (layer4_xor >> 16) ^
+ (layer4_xor >> 8);
+ return layer4_xor % count;
}
- return (data->h_dest[5] ^ data->h_source[5]) % count;
-}
-
-/*
- * Hash for the output device based upon layer 2 data
- */
-static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
-{
- struct ethhdr *data = (struct ethhdr *)skb->data;
-
- return (data->h_dest[5] ^ data->h_source[5]) % count;
+ return bond_xmit_hash_policy_l2(skb, count);
}
/*-------------------------- Device entry points ----------------------------*/
--
1.7.11
^ permalink raw reply related
* Re: [PATCH 0/3] ipvs: IPv6 fragment handling for IPVS
From: Simon Horman @ 2012-08-22 6:42 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Julian Anastasov, netdev, Patrick McHardy, Hans Schillstrom,
LVS devel, Wensong Zhang, netfilter-devel, Pablo Neira Ayuso
In-Reply-To: <1345535487.3069.281.camel@localhost>
On Tue, Aug 21, 2012 at 09:51:27AM +0200, Jesper Dangaard Brouer wrote:
> On Tue, 2012-08-21 at 14:24 +0900, Simon Horman wrote:
> > I have no objection to these changes, but I would be more comfortable
> > applying them after a review from Hans, Julian or Pablo.
> >
> I would appreciate a review, especially from Julian.
>
> I'm going to do some more extensive testing on the patchset, in this
> week. So, no hurry on applying these, we have time for a good review
> process.
Thanks Jesper, and Julian and Hans who also responded.
Let me know when you feel the series is ready to merge.
I am also in no hurry.
^ permalink raw reply
* Re: smsc75xx & smsc95xx, setting skb->truesize correctly?
From: Eric Dumazet @ 2012-08-22 6:34 UTC (permalink / raw)
To: Jussi Kivilinna; +Cc: netdev, Steve Glendinning
In-Reply-To: <20120822083549.57260n3jdqfgxtj4@www.81.fi>
On Wed, 2012-08-22 at 08:35 +0300, Jussi Kivilinna wrote:
> Quoting Eric Dumazet <eric.dumazet@gmail.com>:
>
> > On Mon, 2012-08-20 at 17:57 +0300, Jussi Kivilinna wrote:
> >> Hello,
> >>
> >> Is setting skb->truesize in smsc75xx and smsc95xx correct?
> >> In smsc75xx/smsc95xx_rx_fixup(), input skb containing multiple packets
> >> is cloned and truesize for each clone is set to packet-size +
> >> sizeof(struct sk_buff), but input skb has minimum allocation size of
> >> 9000 bytes (MAX_SINGLE_PACKET_SIZE) and maximum of 18944 bytes
> >> (DEFAULT_HS_BURST_CAP_SIZE) (+ NET_IP_ALIGN). Doesn't this cause
> >> truesize to be underestimated?
> >
> > This has been discussed in a "TCP transmit performance regression"
> > thread some weeks ago.
> >
> > More generally, skb_clone() is not a good idea in rx path.
>
> So all skb_clone use in drivers/net/usb/ should be removed/replaced
> with following?
Doing a copy might be expensive on some low end hardware, so I can
understand why this skb_clone() idea was deployed years ago.
Gigabit r8169 has to perform the copy because of security issue, and so
far nobody complained of performance impact.
Best thing would be to not use large buffers from the beginning,
and switch to a frag idea.
(A large frame would needs 2 or 3 medium buffers, as done in ath9k)
Check https://gerrit.chromium.org/gerrit/#/c/18412/
and commit 0d95521ea74735826cb2e28bebf6a07392c75bfa (ath9k: use split rx
buffers to get rid of order-1 skb allocations)
^ permalink raw reply
* Re: smsc75xx & smsc95xx, setting skb->truesize correctly?
From: Jussi Kivilinna @ 2012-08-22 5:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Steve Glendinning
In-Reply-To: <1345542398.5158.446.camel@edumazet-glaptop>
Quoting Eric Dumazet <eric.dumazet@gmail.com>:
> On Mon, 2012-08-20 at 17:57 +0300, Jussi Kivilinna wrote:
>> Hello,
>>
>> Is setting skb->truesize in smsc75xx and smsc95xx correct?
>> In smsc75xx/smsc95xx_rx_fixup(), input skb containing multiple packets
>> is cloned and truesize for each clone is set to packet-size +
>> sizeof(struct sk_buff), but input skb has minimum allocation size of
>> 9000 bytes (MAX_SINGLE_PACKET_SIZE) and maximum of 18944 bytes
>> (DEFAULT_HS_BURST_CAP_SIZE) (+ NET_IP_ALIGN). Doesn't this cause
>> truesize to be underestimated?
>
> This has been discussed in a "TCP transmit performance regression"
> thread some weeks ago.
>
> More generally, skb_clone() is not a good idea in rx path.
So all skb_clone use in drivers/net/usb/ should be removed/replaced
with following?
>
> I dont have the hardware so cannot send a formal patch.
Neither do I, was looking for gigabit-usb dongle to buy and ended up
checking various drivers.
I guess I could do and test this for rndis_host/rndis_wlan and add
rx-skb recycling to usbnet core for drivers that do copy-break in
rx_fixup.
-Jussi
>
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index b1112e7..3d9566f 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -1080,30 +1080,17 @@ static int smsc95xx_rx_fixup(struct usbnet
> *dev, struct sk_buff *skb)
> return 0;
> }
>
> - /* last frame in this batch */
> - if (skb->len == size) {
> - if (dev->net->features & NETIF_F_RXCSUM)
> - smsc95xx_rx_csum_offload(skb);
> - skb_trim(skb, skb->len - 4); /* remove fcs */
> - skb->truesize = size + sizeof(struct sk_buff);
> -
> - return 1;
> - }
> -
> - ax_skb = skb_clone(skb, GFP_ATOMIC);
> + ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
> if (unlikely(!ax_skb)) {
> netdev_warn(dev->net, "Error allocating skb\n");
> return 0;
> }
>
> - ax_skb->len = size;
> - ax_skb->data = packet;
> - skb_set_tail_pointer(ax_skb, size);
> + memcpy(skb_put(ax_skb, size), packet, size);
>
> if (dev->net->features & NETIF_F_RXCSUM)
> smsc95xx_rx_csum_offload(ax_skb);
> - skb_trim(ax_skb, ax_skb->len - 4); /* remove fcs */
> - ax_skb->truesize = size + sizeof(struct sk_buff);
> + __skb_trim(ax_skb, ax_skb->len - 4); /* remove fcs */
>
> usbnet_skb_return(dev, ax_skb);
> }
>
>
>
>
^ permalink raw reply
* Re: [PATCH v7] bonding: support for IPv6 transmit hashing
From: David Miller @ 2012-08-22 5:29 UTC (permalink / raw)
To: linux; +Cc: netdev
In-Reply-To: <2e01d8f94c42c61af9886683a4c35caf6816bc3d.1345610688.git.linux@8192.net>
From: John Eaglesham <linux@8192.net>
Date: Tue, 21 Aug 2012 22:12:33 -0700
> Signed-off-by: John Eaglesham
This is not a proper signoff, you must put an email address on this line.
Also, the changelog between the versions doesn't belong in the commit
log message itself. It gets placed in commentary after the "---" line.
^ permalink raw reply
* Re: copy_skb_header() and sk_buff flags (question)
From: Eric Dumazet @ 2012-08-22 5:21 UTC (permalink / raw)
To: Kevin Wilson; +Cc: netdev
In-Reply-To: <CAGXs5wVp2bLnwFiuYEg6ZOv0t3DX4PCCgY-EURbs5bySgEqtdw@mail.gmail.com>
On Wed, 2012-08-22 at 07:55 +0300, Kevin Wilson wrote:
> Hello,
>
> I have a question if I may, as I cannot understand this point:
>
> I try to understand why in __copy_skb_header() we copy various flags
> (like ip_summed,
> local_df, pkt_type, priority, ipvs_property, and more. But there are
> flags which we do
> not copy, like nohdr, or fclone, or peeked.
>
> Aren't these flags part of the sk_buff header?
Caller should take care of said bits.
peeked is probably always 0 at this point...
I tried to use a single memcpy() to speedup things, but found out
it was not a trivial thing...
^ permalink raw reply
* [PATCH v7] bonding: support for IPv6 transmit hashing
From: John Eaglesham @ 2012-08-22 5:12 UTC (permalink / raw)
To: netdev; +Cc: John Eaglesham
In-Reply-To: <eb20e8f67e6aad94c233219e058c3e793ed755cb.1341167171.git.linux@8192.net>
From: John Eaglesham <linux@8192.net>
Currently the "bonding" driver does not support load balancing outgoing
traffic in LACP mode for IPv6 traffic. IPv4 (and TCP or UDP over IPv4)
are currently supported; this patch adds transmit hashing for IPv6 (and
TCP or UDP over IPv6), bringing IPv6 up to par with IPv4 support in the
bonding driver. In addition, bounds checking has been added to all
transmit hashing functions.
The algorithm chosen (xor'ing the bottom three quads of the source and
destination addresses together, then xor'ing each byte of that result into
the bottom byte, finally xor'ing with the last bytes of the MAC addresses)
was selected after testing almost 400,000 unique IPv6 addresses harvested
from server logs. This algorithm had the most even distribution for both
big- and little-endian architectures while still using few instructions. Its
behavior also attempts to closely match that of the IPv4 algorithm.
The IPv6 flow label was intentionally not included in the hash as it appears
to be unset in the vast majority of IPv6 traffic sampled, and the current
algorithm not using the flow label already offers a very even distribution.
Fragmented IPv6 packets are handled the same way as fragmented IPv4 packets,
ie, they are not balanced based on layer 4 information. Additionally,
IPv6 packets with intermediate headers are not balanced based on layer
4 information. In practice these intermediate headers are not common and
this should not cause any problems, and the alternative (a packet-parsing
loop and look-up table) seemed slow and complicated for little gain.
Changes:
v2)
* Clarify description
* Add bounds checking to more functions
* All functions call bond_xmit_hash_policy_l2 rather than re-
implement the same logic.
v3)
* Patch against net-next.
* Style corrections.
v4)
* Correct indenting.
v5)
* Squash documentation and code patches into one.
v6)
* Modify IPv6 hash to behave more like the IPv4 hash, update
documentation with modified algorithm.
* Clean up formatting.
* Move all variable declaration to the top of the function.
* Minor change to IPv6 layer 4 hash to match IPv4 hash behavior
(mix all hashed address bits together rather than just the
bottom 24 bits).
v7)
* Improve bounds checking code (handle truncated IPv6 header,
removed goto, fewer if statements).
* Re-write pseudocode in documentation to match actual code more
closely.
* Correct indenting, align parentheses, wrap code at <= 80 columns
(based on Jay's changes).
Patch has been tested and performs as expected.
Signed-off-by: John Eaglesham
---
Documentation/networking/bonding.txt | 30 ++++++++++--
drivers/net/bonding/bond_main.c | 89 +++++++++++++++++++++++++-----------
2 files changed, 88 insertions(+), 31 deletions(-)
diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 6b1c711..10a015c 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -752,12 +752,22 @@ xmit_hash_policy
protocol information to generate the hash.
Uses XOR of hardware MAC addresses and IP addresses to
- generate the hash. The formula is
+ generate the hash. The IPv4 formula is
(((source IP XOR dest IP) AND 0xffff) XOR
( source MAC XOR destination MAC ))
modulo slave count
+ The IPv6 formula is
+
+ hash = (source ip quad 2 XOR dest IP quad 2) XOR
+ (source ip quad 3 XOR dest IP quad 3) XOR
+ (source ip quad 4 XOR dest IP quad 4)
+
+ (((hash >> 24) XOR (hash >> 16) XOR (hash >> 8) XOR hash)
+ XOR (source MAC XOR destination MAC))
+ modulo slave count
+
This algorithm will place all traffic to a particular
network peer on the same slave. For non-IP traffic,
the formula is the same as for the layer2 transmit
@@ -778,19 +788,29 @@ xmit_hash_policy
slaves, although a single connection will not span
multiple slaves.
- The formula for unfragmented TCP and UDP packets is
+ The formula for unfragmented IPv4 TCP and UDP packets is
((source port XOR dest port) XOR
((source IP XOR dest IP) AND 0xffff)
modulo slave count
- For fragmented TCP or UDP packets and all other IP
- protocol traffic, the source and destination port
+ The formula for unfragmented IPv6 TCP and UDP packets is
+
+ hash = (source port XOR dest port) XOR
+ ((source ip quad 2 XOR dest IP quad 2) XOR
+ (source ip quad 3 XOR dest IP quad 3) XOR
+ (source ip quad 4 XOR dest IP quad 4))
+
+ ((hash >> 24) XOR (hash >> 16) XOR (hash >> 8) XOR hash)
+ modulo slave count
+
+ For fragmented TCP or UDP packets and all other IPv4 and
+ IPv6 protocol traffic, the source and destination port
information is omitted. For non-IP traffic, the
formula is the same as for the layer2 transmit hash
policy.
- This policy is intended to mimic the behavior of
+ The IPv4 policy is intended to mimic the behavior of
certain switches, notably Cisco switches with PFC2 as
well as some Foundry and IBM products.
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index d95fbc3..4221e57 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3354,56 +3354,93 @@ static struct notifier_block bond_netdev_notifier = {
/*---------------------------- Hashing Policies -----------------------------*/
/*
+ * Hash for the output device based upon layer 2 data
+ */
+static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
+{
+ struct ethhdr *data = (struct ethhdr *)skb->data;
+
+ if (skb_headlen(skb) >= offsetof(struct ethhdr, h_proto))
+ return (data->h_dest[5] ^ data->h_source[5]) % count;
+
+ return 0;
+}
+
+/*
* Hash for the output device based upon layer 2 and layer 3 data. If
- * the packet is not IP mimic bond_xmit_hash_policy_l2()
+ * the packet is not IP, fall back on bond_xmit_hash_policy_l2()
*/
static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count)
{
struct ethhdr *data = (struct ethhdr *)skb->data;
- struct iphdr *iph = ip_hdr(skb);
-
- if (skb->protocol == htons(ETH_P_IP)) {
+ struct iphdr *iph;
+ struct ipv6hdr *ipv6h;
+ u32 v6hash;
+ __be32 *s, *d;
+
+ if (skb->protocol == htons(ETH_P_IP) &&
+ skb_network_header_len(skb) >= sizeof(*iph)) {
+ iph = ip_hdr(skb);
return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^
(data->h_dest[5] ^ data->h_source[5])) % count;
+ } else if (skb->protocol == htons(ETH_P_IPV6) &&
+ skb_network_header_len(skb) >= sizeof(*ipv6h)) {
+ ipv6h = ipv6_hdr(skb);
+ s = &ipv6h->saddr.s6_addr32[0];
+ d = &ipv6h->daddr.s6_addr32[0];
+ v6hash = (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
+ v6hash ^= (v6hash >> 24) ^ (v6hash >> 16) ^ (v6hash >> 8);
+ return (v6hash ^ data->h_dest[5] ^ data->h_source[5]) % count;
}
- return (data->h_dest[5] ^ data->h_source[5]) % count;
+ return bond_xmit_hash_policy_l2(skb, count);
}
/*
* Hash for the output device based upon layer 3 and layer 4 data. If
* the packet is a frag or not TCP or UDP, just use layer 3 data. If it is
- * altogether not IP, mimic bond_xmit_hash_policy_l2()
+ * altogether not IP, fall back on bond_xmit_hash_policy_l2()
*/
static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
{
- struct ethhdr *data = (struct ethhdr *)skb->data;
- struct iphdr *iph = ip_hdr(skb);
- __be16 *layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
- int layer4_xor = 0;
-
- if (skb->protocol == htons(ETH_P_IP)) {
+ u32 layer4_xor = 0;
+ struct iphdr *iph;
+ struct ipv6hdr *ipv6h;
+ __be32 *s, *d;
+ __be16 *layer4hdr;
+
+ if (skb->protocol == htons(ETH_P_IP) &&
+ skb_network_header_len(skb) >= sizeof(*iph)) {
+ iph = ip_hdr(skb);
if (!ip_is_fragment(iph) &&
(iph->protocol == IPPROTO_TCP ||
- iph->protocol == IPPROTO_UDP)) {
- layer4_xor = ntohs((*layer4hdr ^ *(layer4hdr + 1)));
+ iph->protocol == IPPROTO_UDP) &&
+ (skb_headlen(skb) - skb_network_offset(skb) >=
+ iph->ihl * sizeof(u32) + sizeof(*layer4hdr) * 2)) {
+ layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
+ layer4_xor = ntohs(*layer4hdr ^ *(layer4hdr + 1));
}
return (layer4_xor ^
((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
-
+ } else if (skb->protocol == htons(ETH_P_IPV6) &&
+ skb_network_header_len(skb) >= sizeof(*ipv6h)) {
+ ipv6h = ipv6_hdr(skb);
+ if ((ipv6h->nexthdr == IPPROTO_TCP ||
+ ipv6h->nexthdr == IPPROTO_UDP) &&
+ (skb_headlen(skb) - skb_network_offset(skb) >=
+ sizeof(*ipv6h) + sizeof(*layer4hdr) * 2)) {
+ layer4hdr = (__be16 *)(ipv6h + 1);
+ layer4_xor = ntohs(*layer4hdr ^ *(layer4hdr + 1));
+ }
+ s = &ipv6h->saddr.s6_addr32[0];
+ d = &ipv6h->daddr.s6_addr32[0];
+ layer4_xor ^= (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
+ layer4_xor ^= (layer4_xor >> 24) ^ (layer4_xor >> 16) ^
+ (layer4_xor >> 8);
+ return layer4_xor % count;
}
- return (data->h_dest[5] ^ data->h_source[5]) % count;
-}
-
-/*
- * Hash for the output device based upon layer 2 data
- */
-static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
-{
- struct ethhdr *data = (struct ethhdr *)skb->data;
-
- return (data->h_dest[5] ^ data->h_source[5]) % count;
+ return bond_xmit_hash_policy_l2(skb, count);
}
/*-------------------------- Device entry points ----------------------------*/
--
1.7.11
^ permalink raw reply related
* Re[2]: [PATCH 05/19] netfilter: nf_conntrack_ipv6: improve fragmentation handling
From: Hans Schillstrom @ 2012-08-22 5:01 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Patrick McHardy, netfilter-devel, netdev, Julian Anastasov,
Hans Schillstrom
Hi
>
>On Sun, 2012-08-19 at 21:44 +0200, Patrick McHardy wrote:
>> On Sun, 19 Aug 2012, Jesper Dangaard Brouer wrote:
>> > On Sat, 2012-08-18 at 14:26 +0200, Patrick McHardy wrote:
>[...]
>
>> > Don't I need to load some of the helper modules, or just the
>> > nf_conntrack_ipv6 module, or perhaps only nf_defrag_ipv6 ?
>>
>> Not with the entire patchset, just IPv6 conntrack is enough. Aith IPv6 NAT
>> the first packet of a connection must always be defragemented, independant
>> of an assigned helper.
>
>When loading "nf_conntrack_ipv6" I run into issues.
>
>When sending a fragmented UDP packet. With these patches, the IPVS
>stack will no longer see the fragmented packets, but instead see one
>large SKB. This will trigger a MTU path check in e.g.
>ip_vs_dr_xmit_v6() and an ICMPv6 too big packet is send back.
>
> IPVS: ip_vs_dr_xmit_v6(): frag needed
>
>Perhaps we could change/fix the MTU check in IPVS?
>(This would also solve issues I've seen with TSO/GSO frames, hitting
>this code path).
>
I ran into this as well,
try this for the mtu check.
if ((!skb->local_df && skb->len > mtu && !skb_is_gso(skb)) ||
(IP6CB(skb)->frag_max_size && IP6CB(skb)->frag_max_size > mtu)) {
/Hans
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: David Miller @ 2012-08-22 5:00 UTC (permalink / raw)
To: torvalds; +Cc: hpa, bhutchings, tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <CA+55aFz39tCSO7sf7+czNEBcEx7q4=KbnAcKEoS-KBwr6D3Psg@mail.gmail.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Tue, 21 Aug 2012 21:35:22 -0700
> My biggest reason to question this all is that I don't think it's
> worth it. Why would we ever care to do all this in the first place?
> There's no really sane use for it.
All the x86 crypto code hits this case all the time, easiest example
is doing a dm-crypt on a block device when an IPSEC packet arrives.
The crypto code has all of this special code and layering that is
there purely so it can fall back to the slow non-optimized version
of the crypto operation when it hits this can't-nest-fpu-saving
situation.
^ permalink raw reply
* copy_skb_header() and sk_buff flags (question)
From: Kevin Wilson @ 2012-08-22 4:55 UTC (permalink / raw)
To: netdev
Hello,
I have a question if I may, as I cannot understand this point:
I try to understand why in __copy_skb_header() we copy various flags
(like ip_summed,
local_df, pkt_type, priority, ipvs_property, and more. But there are
flags which we do
not copy, like nohdr, or fclone, or peeked.
Aren't these flags part of the sk_buff header?
regards,
Kevin
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 4:42 UTC (permalink / raw)
To: H. Peter Anvin
Cc: David Miller, bhutchings, tglx, mingo, netdev, linux-net-drivers,
x86
In-Reply-To: <CA+55aFy=ohFoZ1yZGdj1DCcNXU9gsndgH9Qod4q8+s=sbGKUzQ@mail.gmail.com>
On Tue, Aug 21, 2012 at 8:52 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I haven't seen the patch being discussed, or the rationale for it. But
> I doubt it makes sense to do 128-bit MMIO and expect any kind of
> atomicity things anyway, and I very much doubt that using SSE would
> make all that much sense. What's the background, and why would you
> want to do this crap?
Btw, for the 64-bit case, we did have ordering issues, and maybe some
128-bit model has similar ordering issues. Fine. You can't rely on
128-bit atomic accesses anyway on 99% of all hardware - either the CPU
itself cannot do it, or it's too damn inconvenient with XMM only
registers, or the bus itself is limited to 64 bits at a time anyway,
so the CPU or the IO interface would split such an access *anyway*.
So the whole concept of "we rely on atomic 128-bit MMIO accesses"
seems terminally broken. Any driver that thinks it needs that is just
crazy.
And those issues have nothing to do with x86 kernel_fpu_begin/end()
what-so-ever.
So judging by that, I would say that some driver writer needs to take
a few pills, clear up their head, and take another look at their life.
Tell them to look at
include/asm-generic/io-64-nonatomic-hi-lo.h
(and *-lo-hi.h) instead, and ponder the wisdom of just doing it that
way. Tell them to go f*ck themselves if they think they need XMM
registers. They are wrong, for one reason or another.
Linus
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 4:35 UTC (permalink / raw)
To: H. Peter Anvin
Cc: David Miller, bhutchings, tglx, mingo, netdev, linux-net-drivers,
x86
In-Reply-To: <5034591E.3040908@zytor.com>
On Tue, Aug 21, 2012 at 8:59 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>
> kernel_fpu_end() would still have to re-enable preemption (and
> preemption would have to check the work flag), but that should be cheap.
No, done right, you don't even have to disable preemption. Sure, you
might disable preemption inside of kernel_fpu_begin() itself (around
the test for "do we need to save FPU state and set the flag to restore
it at return-to-user-mode time"), but I think that you should be able
to run the code afterwards with preemption enabled.
Why? Because you've saved the FPU state, and you've set the per-thread
flag saying "I will restore at return to user mode". And that, coupled
with teaching the scheduler about this case ("don't set TS, only save
xmm0 in a special kernel-xmm0-save-area") means that the thing can
preempt fine.
The nesting case would be about just saving (again) that xmm0 register
in any nested kernel_fpu_begin/end pair. But that doesn't need
preemption, that just needs local storage for the kernel_fpu_begin/end
(and it can do the xmm0 save/restore with regular "mov" instructions,
because the kernel owns the FPU at that point, since it's nested).
So it's doable. One question is how many registers to save for kernel
use. It might be that we'd need to make that a "actual FPU user needs
to save/restore the registers it uses", and not do the saving in
kernel_fpu_begin()/end() at all.
My biggest reason to question this all is that I don't think it's
worth it. Why would we ever care to do all this in the first place?
There's no really sane use for it.
Judging from the subject line, some crazy person wants to do 128-bit
MMIO. That's just crap. Nobody sane cares. Do it non-atomically with
regular accesses. If it's high-performance IO, it uses DMA. If it
doesn't use DMA and depends on CPU MMIO accesses, WHO THE F*CK CARES?
Nobody.
And if some crazy device depends on 128-bit atomic writes, the device
is shit. Forget about it. Tell people to stop digging themselves
deeper down a hole it's simply not worth going.
Linus
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: David Miller @ 2012-08-22 4:14 UTC (permalink / raw)
To: hpa; +Cc: torvalds, bhutchings, tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <5034591E.3040908@zytor.com>
From: "H. Peter Anvin" <hpa@zytor.com>
Date: Tue, 21 Aug 2012 20:59:26 -0700
> kernel_fpu_end() would still have to re-enable preemption (and
> preemption would have to check the work flag), but that should be cheap.
>
> We could allow the FPU in the kernel to have preemption, if we allocated
> space for two xstates per thread instead of one. That is, however, a
> fair hunk of memory.
Once you have done the first FPU save for the sake of the kernel, you
can minimize what you save for any deeper nesting because the kernel
only cares about a very limited part of that FPU state not the whole
1K thing.
Those bits you can save by hand with a bunch of explicit stores of the
XMM registers, or something like that.
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: H. Peter Anvin @ 2012-08-22 3:59 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Miller, bhutchings, tglx, mingo, netdev, linux-net-drivers,
x86
In-Reply-To: <CA+55aFy=ohFoZ1yZGdj1DCcNXU9gsndgH9Qod4q8+s=sbGKUzQ@mail.gmail.com>
On 08/21/2012 08:52 PM, Linus Torvalds wrote:
>
> That said, we might *try* to make the kernel FPU use be cheaper. We
> could do something like:
>
> - if we need to restore FPU state for user mode, don't do it
> synchronously in kernel_fpu_end(), just set a work-flag, and do it
> just once before returning to user mode
>
> - that makes kernel_fpu_end() a no-op, and while we can't get rid of
> kernel_fpu_begin(), we could at least make it cheap to do many times
> consecutively (ie we'd have to check "who owns FPU state" and perhaps
> save it, but the save would have to be done only the first time).
>
The zeroeth order thing we should do is to make it nest; there are cases
where we do multiple kernel_fpu_begin/_end in sequence for no good reason.
kernel_fpu_end() would still have to re-enable preemption (and
preemption would have to check the work flag), but that should be cheap.
We could allow the FPU in the kernel to have preemption, if we allocated
space for two xstates per thread instead of one. That is, however, a
fair hunk of memory.
-hpa
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 3:52 UTC (permalink / raw)
To: H. Peter Anvin
Cc: David Miller, bhutchings, tglx, mingo, netdev, linux-net-drivers,
x86
In-Reply-To: <503450E2.2040504@zytor.com>
On Tue, Aug 21, 2012 at 8:24 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>
>> I continually see more and more code that has to check this
>> irq_fpu_usable() thing, and have ugly fallback code, and therefore is
>> a sign that this really needs to be fixed properly.
>>
>
> [Cc: Linus, since he has had very strong opinions on this in the past.]
I have strong opinions, because I don't think you *can* do it any
other way on x86.
Saving FPU state on kernel entry IS NOT GOING TO HAPPEN. End of story.
It's too f*cking slow.
And if we don't save it, we cannot use the FP state in kernel mode
without explicitly checking, because the FPU state may not be there,
and touching FP registers can cause exceptions etc. In fact, under
many loads the exception is going to be the common case.
Keeping the FP state live all the time (and then perhaps just saving a
single MMX register for kernel use) and switching it synchronously on
every task switch sounds like a really really bad idea too. It
apparently works better on modern CPU's, but it sucked horribly on
older ones. The FPU state save is horribly expensive, to the point
that we saw it very clearly on benchmarks for signal handling and task
switching.
So the normal operation is going to be "touching the FPU causes a fault".
That said, we might *try* to make the kernel FPU use be cheaper. We
could do something like:
- if we need to restore FPU state for user mode, don't do it
synchronously in kernel_fpu_end(), just set a work-flag, and do it
just once before returning to user mode
- that makes kernel_fpu_end() a no-op, and while we can't get rid of
kernel_fpu_begin(), we could at least make it cheap to do many times
consecutively (ie we'd have to check "who owns FPU state" and perhaps
save it, but the save would have to be done only the first time).
I haven't seen the patch being discussed, or the rationale for it. But
I doubt it makes sense to do 128-bit MMIO and expect any kind of
atomicity things anyway, and I very much doubt that using SSE would
make all that much sense. What's the background, and why would you
want to do this crap?
Linus
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: H. Peter Anvin @ 2012-08-22 3:49 UTC (permalink / raw)
To: David Miller
Cc: bhutchings, tglx, mingo, netdev, linux-net-drivers, x86, torvalds
In-Reply-To: <20120821.202945.2278895156403194101.davem@davemloft.net>
On 08/21/2012 08:29 PM, David Miller wrote:
>
> What we do is we have a FPU stack that grows up from the end of the
> thread_info struct, towards the bottom of the kernel stack.
>
We have 8K of kernel stack, and an xstate which is pushing a kilobyte
already. This seems like a nightmare. Even if we allocate a larger
stack for this sole purpose, we'd have to put a pretty hard cap on how
far it could grow.
> Slot 0 is always the user FPU state.
>
> Slot 1 and further are kernel FPU state save areas.
>
> We hold a counter which keep track of how far deeply saved we are
> in the stack.
>
> Not for the purpose of space saving, but for overhead reduction we
> sometimes can get away with only saving away half of the FPU
> registers. The chip provides a pair of dirty bits, one for the lower
> half of the FPU register file and one for the upper half. We only
> save the bits that are actually dirty.
>
> Furthermore, when we have FPU using code in the kernel that only uses
> the lower half of the registers, we only save away that part of the
> state around the routine.
This is messy on x86; it is somewhat doable but it gets really hairy
because of the monolithic [f]xsave/[f]xrstor instruction.
-hpa
^ permalink raw reply
* [PATCH RFC] ipv6: Add net.ipv6.conf.*.accept_ra_defrtr_table sysctl.
From: Ben Jencks @ 2012-08-22 3:48 UTC (permalink / raw)
To: netdev
Allows configuration of the routing table to which default routes from
RAs are added on a per-interface basis.
One use case where this is required is a multi-homed router advertising
multiple prefixes, one for each upstream. The proper routing policy is
something like:
0: from all lookup local
10: from all lookup main
20: from <prefix1> lookup provider1
30: from <prefix2> lookup provider2
with directly connected and internal routes in main, and default routes
from providers 1 and 2 in their respective tables. With static routes
this is straightforward; if the route for either or both providers is
received via RA this configuration option becomes necessary to put the
routes in the appropriate tables.
Signed-off-by: Ben Jencks <ben@bjencks.net>
---
Patch is against v3.5.2, but applies cleanly to master as of today.
Looking for feedback on:
* Is this a worthwhile feature? Did I miss a good way to do this in
userspace?
* Is a sysctl the right place to configure it? I know it's discouraged
to add new ones, but it seems to fit best with the other similar
configuration options.
* Is the rt6_purge_dflt_routers behavior correct? Should
addrconf_fixup_forwarding be modified so that in the single device
case it only purges from that device's table?
Documentation/networking/ip-sysctl.txt | 6 +++++
include/linux/ipv6.h | 2 ++
include/linux/sysctl.h | 1 +
kernel/sysctl_binary.c | 1 +
net/ipv6/addrconf.c | 10 +++++++
net/ipv6/route.c | 45 ++++++++++++++++++++++----------
6 files changed, 51 insertions(+), 14 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 6f896b9..0d0947a 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1085,6 +1085,12 @@ accept_ra_defrtr - BOOLEAN
Functional default: enabled if accept_ra is enabled.
disabled if accept_ra is disabled.
+accept_ra_defrtr_table - INTEGER
+ Routing table into which to put default route learned via Router
+ Advertisement.
+
+ Functional default: "main" routing table (254)
+
accept_ra_pinfo - BOOLEAN
Learn Prefix Information in Router Advertisement.
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 8260ef7..fa9b4fe 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -153,6 +153,7 @@ struct ipv6_devconf {
#endif
__s32 max_addresses;
__s32 accept_ra_defrtr;
+ __s32 accept_ra_defrtr_table;
__s32 accept_ra_pinfo;
#ifdef CONFIG_IPV6_ROUTER_PREF
__s32 accept_ra_rtr_pref;
@@ -202,6 +203,7 @@ enum {
DEVCONF_MAX_ADDRESSES,
DEVCONF_FORCE_MLD_VERSION,
DEVCONF_ACCEPT_RA_DEFRTR,
+ DEVCONF_ACCEPT_RA_DEFRTR_TABLE,
DEVCONF_ACCEPT_RA_PINFO,
DEVCONF_ACCEPT_RA_RTR_PREF,
DEVCONF_RTR_PROBE_INTERVAL,
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index c34b4c8..80115c8 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -568,6 +568,7 @@ enum {
NET_IPV6_ACCEPT_RA_RT_INFO_MAX_PLEN=22,
NET_IPV6_PROXY_NDP=23,
NET_IPV6_ACCEPT_SOURCE_ROUTE=25,
+ NET_IPV6_ACCEPT_RA_DEFRTR_TABLE=26,
__NET_IPV6_MAX
};
diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index a650694..da964f3 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -517,6 +517,7 @@ static const struct bin_table bin_net_ipv6_conf_var_table[] = {
{ CTL_INT, NET_IPV6_MAX_ADDRESSES, "max_addresses" },
{ CTL_INT, NET_IPV6_FORCE_MLD_VERSION, "force_mld_version" },
{ CTL_INT, NET_IPV6_ACCEPT_RA_DEFRTR, "accept_ra_defrtr" },
+ { CTL_INT, NET_IPV6_ACCEPT_RA_DEFRTR_TABLE, "accept_ra_defrtr_table" },
{ CTL_INT, NET_IPV6_ACCEPT_RA_PINFO, "accept_ra_pinfo" },
{ CTL_INT, NET_IPV6_ACCEPT_RA_RTR_PREF, "accept_ra_rtr_pref" },
{ CTL_INT, NET_IPV6_RTR_PROBE_INTERVAL, "router_probe_interval" },
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 8f6411c..2bdb96c 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -188,6 +188,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
#endif
.max_addresses = IPV6_MAX_ADDRESSES,
.accept_ra_defrtr = 1,
+ .accept_ra_defrtr_table = RT6_TABLE_DFLT,
.accept_ra_pinfo = 1,
#ifdef CONFIG_IPV6_ROUTER_PREF
.accept_ra_rtr_pref = 1,
@@ -222,6 +223,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
#endif
.max_addresses = IPV6_MAX_ADDRESSES,
.accept_ra_defrtr = 1,
+ .accept_ra_defrtr_table = RT6_TABLE_DFLT,
.accept_ra_pinfo = 1,
#ifdef CONFIG_IPV6_ROUTER_PREF
.accept_ra_rtr_pref = 1,
@@ -3894,6 +3896,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
#endif
array[DEVCONF_MAX_ADDRESSES] = cnf->max_addresses;
array[DEVCONF_ACCEPT_RA_DEFRTR] = cnf->accept_ra_defrtr;
+ array[DEVCONF_ACCEPT_RA_DEFRTR_TABLE] = cnf->accept_ra_defrtr_table;
array[DEVCONF_ACCEPT_RA_PINFO] = cnf->accept_ra_pinfo;
#ifdef CONFIG_IPV6_ROUTER_PREF
array[DEVCONF_ACCEPT_RA_RTR_PREF] = cnf->accept_ra_rtr_pref;
@@ -4496,6 +4499,13 @@ static struct addrconf_sysctl_table
.proc_handler = proc_dointvec,
},
{
+ .procname = "accept_ra_defrtr_table",
+ .data = &ipv6_devconf.accept_ra_defrtr_table,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ {
.procname = "accept_ra_pinfo",
.data = &ipv6_devconf.accept_ra_pinfo,
.maxlen = sizeof(int),
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index becb048..dafcb61 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1916,8 +1916,12 @@ struct rt6_info *rt6_get_dflt_router(const struct in6_addr *addr, struct net_dev
{
struct rt6_info *rt;
struct fib6_table *table;
+ struct inet6_dev *idev;
- table = fib6_get_table(dev_net(dev), RT6_TABLE_DFLT);
+ idev = __in6_dev_get(dev);
+ if (!idev)
+ return NULL;
+ table = fib6_get_table(dev_net(dev), idev->cnf.accept_ra_defrtr_table);
if (!table)
return NULL;
@@ -1939,7 +1943,6 @@ struct rt6_info *rt6_add_dflt_router(const struct in6_addr *gwaddr,
unsigned int pref)
{
struct fib6_config cfg = {
- .fc_table = RT6_TABLE_DFLT,
.fc_metric = IP6_RT_PRIO_USER,
.fc_ifindex = dev->ifindex,
.fc_flags = RTF_GATEWAY | RTF_ADDRCONF | RTF_DEFAULT |
@@ -1948,7 +1951,12 @@ struct rt6_info *rt6_add_dflt_router(const struct in6_addr *gwaddr,
.fc_nlinfo.nlh = NULL,
.fc_nlinfo.nl_net = dev_net(dev),
};
+ struct inet6_dev *idev;
+ /* idev should not be null, since we were called from
+ * ndisc_router_discovery which already checked it */
+ idev = __in6_dev_get(dev);
+ cfg.fc_table = idev->cnf.accept_ra_defrtr_table;
cfg.fc_gateway = *gwaddr;
ip6_route_add(&cfg);
@@ -1960,23 +1968,32 @@ void rt6_purge_dflt_routers(struct net *net)
{
struct rt6_info *rt;
struct fib6_table *table;
+ struct net_device *dev;
+ struct inet6_dev *idev;
- /* NOTE: Keep consistent with rt6_get_dflt_router */
- table = fib6_get_table(net, RT6_TABLE_DFLT);
- if (!table)
- return;
+ rcu_read_lock();
+ for_each_netdev_rcu(net, dev) {
+ idev = __in6_dev_get(dev);
+ if (idev == NULL) {
+ continue;
+ }
+ table = fib6_get_table(net, idev->cnf.accept_ra_defrtr_table);
+ if (!table)
+ continue;
restart:
- read_lock_bh(&table->tb6_lock);
- for (rt = table->tb6_root.leaf; rt; rt = rt->dst.rt6_next) {
- if (rt->rt6i_flags & (RTF_DEFAULT | RTF_ADDRCONF)) {
- dst_hold(&rt->dst);
- read_unlock_bh(&table->tb6_lock);
- ip6_del_rt(rt);
- goto restart;
+ read_lock_bh(&table->tb6_lock);
+ for (rt = table->tb6_root.leaf; rt; rt = rt->dst.rt6_next) {
+ if (rt->rt6i_flags & (RTF_DEFAULT | RTF_ADDRCONF)) {
+ dst_hold(&rt->dst);
+ read_unlock_bh(&table->tb6_lock);
+ ip6_del_rt(rt);
+ goto restart;
+ }
}
+ read_unlock_bh(&table->tb6_lock);
}
- read_unlock_bh(&table->tb6_lock);
+ rcu_read_unlock();
}
static void rtmsg_to_fib6_config(struct net *net,
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: David Miller @ 2012-08-22 3:29 UTC (permalink / raw)
To: hpa; +Cc: bhutchings, tglx, mingo, netdev, linux-net-drivers, x86, torvalds
In-Reply-To: <503450E2.2040504@zytor.com>
From: "H. Peter Anvin" <hpa@zytor.com>
Date: Tue, 21 Aug 2012 20:24:18 -0700
> I'm all ears... tell me how sparc64 deals with this, maybe we can
> implement something similar. At the same time, do keep in mind that on
> x86 this is not just a matter of the FPU state, but the entire "extended
> state" which can be very large.
Sparc's state is pretty huge too. 256 bytes worth of FPU registers,
plus a set of 64-bit control registers.
What we do is we have a FPU stack that grows up from the end of the
thread_info struct, towards the bottom of the kernel stack.
Slot 0 is always the user FPU state.
Slot 1 and further are kernel FPU state save areas.
We hold a counter which keep track of how far deeply saved we are
in the stack.
Not for the purpose of space saving, but for overhead reduction we
sometimes can get away with only saving away half of the FPU
registers. The chip provides a pair of dirty bits, one for the lower
half of the FPU register file and one for the upper half. We only
save the bits that are actually dirty.
Furthermore, when we have FPU using code in the kernel that only uses
the lower half of the registers, we only save away that part of the
state around the routine.
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: H. Peter Anvin @ 2012-08-22 3:24 UTC (permalink / raw)
To: David Miller
Cc: bhutchings, tglx, mingo, netdev, linux-net-drivers, x86,
Linus Torvalds
In-Reply-To: <20120821.193446.1534561579811962053.davem@davemloft.net>
On 08/21/2012 07:34 PM, David Miller wrote:
>
> I really hope we eventually get rid of this rediculous restriction the
> x86 code has.
>
> It really needs a proper stack of FPU state saves like sparc64 has.
>
> Half of the code and complexity in arch/x86/crypto/ would just
> disappear, because most of it has to do with handling this obtuse
> FPU usage restriction which shouldn't even be an issue in the first
> place.
>
> I continually see more and more code that has to check this
> irq_fpu_usable() thing, and have ugly fallback code, and therefore is
> a sign that this really needs to be fixed properly.
>
[Cc: Linus, since he has had very strong opinions on this in the past.]
I'm all ears... tell me how sparc64 deals with this, maybe we can
implement something similar. At the same time, do keep in mind that on
x86 this is not just a matter of the FPU state, but the entire "extended
state" which can be very large.
Given the cost of state save/enable, however, nothing is going to change
the need for kernel_fpu_begin/end, nor the fact that those things will
want to bracket large regions.
-hpa
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: David Miller @ 2012-08-22 2:34 UTC (permalink / raw)
To: bhutchings; +Cc: hpa, tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <1345601051.2659.93.camel@bwh-desktop.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 22 Aug 2012 03:04:11 +0100
> On Tue, 2012-08-21 at 18:37 -0700, H. Peter Anvin wrote:
>> On 08/21/2012 06:23 PM, Ben Hutchings wrote:
>> > Define reado(), writeo() and their raw counterparts using SSE.
>> >
>> > Based on work by Stuart Hodgson <smhodgson@solarflare.com>.
>>
>> It would be vastly better if we explicitly controlled this with
>> kernel_fpu_begin()/kernel_fpu_end() rather than hiding it in primitives
>> than might tempt the user to do very much the wrong thing.
>>
>> Also, it needs to be extremely clear to the user that these operations
>> use the FPU, and all the requirements there need to be met, including
>> not using them at interrupt time.
>
> Well we can sometimes use the FPU state at IRQ time, can't we
> (irq_fpu_usable())? So we might need, say, try_reado() and
> try_writeo() with callers expected to fall back to alternatives. (Which
> they must have anyway for any architecture that doesn't support this.)
I really hope we eventually get rid of this rediculous restriction the
x86 code has.
It really needs a proper stack of FPU state saves like sparc64 has.
Half of the code and complexity in arch/x86/crypto/ would just
disappear, because most of it has to do with handling this obtuse
FPU usage restriction which shouldn't even be an issue in the first
place.
I continually see more and more code that has to check this
irq_fpu_usable() thing, and have ugly fallback code, and therefore is
a sign that this really needs to be fixed properly.
^ permalink raw reply
* Re: [PATCH 0/3] x86_64, sfc: 128-bit memory-mapped I/O
From: H. Peter Anvin @ 2012-08-22 2:31 UTC (permalink / raw)
To: Ben Hutchings
Cc: Thomas Gleixner, Ingo Molnar, netdev, linux-net-drivers, x86
In-Reply-To: <1345601423.2659.100.camel@bwh-desktop.uk.solarflarecom.com>
On 08/21/2012 07:10 PM, Ben Hutchings wrote:
>>
>> Yes, you have to make sure you properly enforce the necessary ordering
>> requirements manually (I think you can do that with sfence).
>
> We did put an sfence after the writes to each register. But some
> systems only want to combine writes that cover an entire cache line, and
> the writes covering a 128-bit register get broken back up into multiple
> writes at the PCIe level. And on some systems these are sent in
> decreasing address order, which breaks the rules for writing to
> TX_DESC_UPD.
>
> To avoid this we'd have to put an sfence in between the writes to a
> register, leaving us back where we started.
>
You realize the same applies to 128-bit writes, right? Some cores
and/or systems will break them up.
-hpa
^ 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