* [PATCH 01/25] ipvs: IPv6 MTU checking cleanup and bugfix
From: pablo @ 2012-09-03 23:53 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1346716452-3080-1-git-send-email-pablo@netfilter.org>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Cleaning up the IPv6 MTU checking in the IPVS xmit code, by using
a common helper function __mtu_check_toobig_v6().
The MTU check for tunnel mode can also use this helper as
ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) is qual to
skb->len. And the 'mtu' variable have been adjusted before
calling helper.
Notice, this also fixes a bug, as the the MTU check in ip_vs_dr_xmit_v6()
were missing a check for skb_is_gso().
This bug e.g. caused issues for KVM IPVS setups, where different
Segmentation Offloading techniques are utilized, between guests,
via the virtio driver. This resulted in very bad performance,
due to the ICMPv6 "too big" messages didn't affect the sender.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/ipvs/ip_vs_xmit.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 543a554..67a3978 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -85,6 +85,15 @@ __ip_vs_dst_check(struct ip_vs_dest *dest, u32 rtos)
return dst;
}
+static inline bool
+__mtu_check_toobig_v6(const struct sk_buff *skb, u32 mtu)
+{
+ if (skb->len > mtu && !skb_is_gso(skb)) {
+ return true; /* Packet size violate MTU size */
+ }
+ return false;
+}
+
/* Get route to daddr, update *saddr, optionally bind route to saddr */
static struct rtable *do_output_route4(struct net *net, __be32 daddr,
u32 rtos, int rt_mode, __be32 *saddr)
@@ -491,7 +500,7 @@ ip_vs_bypass_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
/* MTU checking */
mtu = dst_mtu(&rt->dst);
- if (skb->len > mtu && !skb_is_gso(skb)) {
+ if (__mtu_check_toobig_v6(skb, mtu)) {
if (!skb->dev) {
struct net *net = dev_net(skb_dst(skb)->dev);
@@ -712,7 +721,7 @@ ip_vs_nat_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
/* MTU checking */
mtu = dst_mtu(&rt->dst);
- if (skb->len > mtu && !skb_is_gso(skb)) {
+ if (__mtu_check_toobig_v6(skb, mtu)) {
if (!skb->dev) {
struct net *net = dev_net(skb_dst(skb)->dev);
@@ -946,8 +955,8 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
if (skb_dst(skb))
skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu);
- if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&
- !skb_is_gso(skb)) {
+ /* MTU checking: Notice that 'mtu' have been adjusted before hand */
+ if (__mtu_check_toobig_v6(skb, mtu)) {
if (!skb->dev) {
struct net *net = dev_net(skb_dst(skb)->dev);
@@ -1113,7 +1122,7 @@ ip_vs_dr_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
/* MTU checking */
mtu = dst_mtu(&rt->dst);
- if (skb->len > mtu) {
+ if (__mtu_check_toobig_v6(skb, mtu)) {
if (!skb->dev) {
struct net *net = dev_net(skb_dst(skb)->dev);
@@ -1349,7 +1358,7 @@ ip_vs_icmp_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
/* MTU checking */
mtu = dst_mtu(&rt->dst);
- if (skb->len > mtu && !skb_is_gso(skb)) {
+ if (__mtu_check_toobig_v6(skb, mtu)) {
if (!skb->dev) {
struct net *net = dev_net(skb_dst(skb)->dev);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH V2 09/12] net/eipoib: Add main driver functionality
From: Michael S. Tsirkin @ 2012-09-03 21:22 UTC (permalink / raw)
To: Or Gerlitz
Cc: Eric W. Biederman, Or Gerlitz, davem, roland, netdev, sean.hefty,
Erez Shitrit, Ali Ayoub, Doug Ledford
In-Reply-To: <CAJZOPZ+ZHBg=vswgmWWz2D0GyWhD-ghkuY9_7CQB47uDmyzhsA@mail.gmail.com>
On Mon, Sep 03, 2012 at 11:53:56PM +0300, Or Gerlitz wrote:
> Michael S. Tsirkin <mst@redhat.com> wrote:
>
> > [...] so it seems that a sane solution would involve an extra level of
> > indirection, with guest addresses being translated to host IB addresses.
> > As long as you do this, maybe using an ethernet frame format makes sense.
>
> > So far the things that make sense. Here are some that don't, to me:
>
> > - Is a pdf presentation all you have in terms of documentation?
> > We are talking communication protocols here - I would expect a
> > proper spec, and some effort to standardize, otherwise where's the
> > guarantee it won't change in an incompatible way?
> > Other things that I would expect to be addressed in such a spec is
> > interaction with other IPoIB features, such as connected
> > mode, checksum offloading etc, and IB features such as multipath etc.
> >
> > - The way you encode LID/QPN in the MAC seems questionable. IIRC there's
> > more to IB addressing than just the LID. Since everyone on the subnet
> > need access to this translation, I think it makes sense to store it in
> > the SM. I think this would also obviate some IPv4 specific hacks in kernel.
>
> > - IGMP/MAC snooping in a driver is just too hairy.
> > As you point out, bridge currently needs the uplink in promisc mode.
> > I don't think a driver should work around that limitation.
> > For some setups, it might be interesting to remove the promisc
> > mode requirement, failing that, I think you could use macvtap passthrough.
> >
> > - Currently migration works without host kernel help, would be
> > preferable to keep it that way.
>
> Hi Michael,
>
> If we rewind to this point, basically, you had few concerns
I think some other people gave feedback too, you need to address it in
the patch (as opposed to by mail - even if it's in documentation or
comments) don't just focus on what I wrote.
>
> 0. not enough documentation
>
> 1. the sender VM MAC isn't preserved when the packet is received
>
> 2. the IGMP snooping we planned to do within netdevice - isn't good practice
>
> 3. mangling of ARPs within netdevice - isn't good practice as well.
>
> For 0,1,2 we have a way to address (see below)
>
> So we are remained with #3 - the ARPs -- thinking on this a little
> further, FWIW there --are-- components in the kernel which
> mangle/generate ARPs and are exposing netdevice, such as openvswitch,
> anyway:
>
> does it make sense to forward ARPs received into / sent over the
> eIPoIB netdevice (e.g using some sort of rule) to some outer entity
> such as user-space
> daemon for interception and later re-injection into eIPoIB?
>
> Or.
Well if this is all you want to do, you can bind a packet socket to the
interface, and drop them at the nic. It is harder to do for incoming
ARP requests though.
I would do something else: send ARPs out to some defined IB address.
This could be local host or queries from some SA property. Said remote
side could send you the responses in ethernet format so you do not need
to mangle responses at all. Similarly for incoming ARP requests.
The rule to do this can also just redirect non IP packets -
this is IPoIB after all.
> Documentation we will fix,
And just to stress the point, document the limitations as well.
> Preserving remote VM mac at the receiver we have few directions for
> solution, e.g either along your suggestion with SA records and/or with
> using "alias GUIDs" (details TBD when the submission resumes).
>
> Multicast we accept the direction you suggested - implement support
> for multicast non promiscuous in the elements "above" eIPoIB (bridge,
> macvtap, etc).
^ permalink raw reply
* [RFC PATCH 2/2] netfilter: More users of ipv6_find_hdr_cb()
From: Jesper Dangaard Brouer @ 2012-09-03 21:16 UTC (permalink / raw)
To: Patrick McHardy, Hans Schillstrom, Hans Schillstrom
Cc: Jesper Dangaard Brouer, netdev, netfilter-devel,
Pablo Neira Ayuso
In-Reply-To: <20120903211504.8851.6894.stgit@dragon>
Functions tproxy_tg6_v1() and socket_mt6_v1 could use the
CB cached version of ipv6_find_hdr().
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
Could someone with tproxy or socket experience tell me,
if the SKB CB data can survive this far?
net/netfilter/xt_TPROXY.c | 2 +-
net/netfilter/xt_socket.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/xt_TPROXY.c b/net/netfilter/xt_TPROXY.c
index d7f1953..c73b713 100644
--- a/net/netfilter/xt_TPROXY.c
+++ b/net/netfilter/xt_TPROXY.c
@@ -285,7 +285,7 @@ tproxy_tg6_v1(struct sk_buff *skb, const struct xt_action_param *par)
int thoff = 0;
int tproto;
- tproto = ipv6_find_hdr(skb, &thoff, -1, NULL, NULL);
+ tproto = ipv6_find_hdr_cb(skb, &thoff, -1, NULL, NULL);
if (tproto < 0) {
pr_debug("unable to find transport header in IPv6 packet, dropping\n");
return NF_DROP;
diff --git a/net/netfilter/xt_socket.c b/net/netfilter/xt_socket.c
index 9ea482d..4feca50 100644
--- a/net/netfilter/xt_socket.c
+++ b/net/netfilter/xt_socket.c
@@ -266,7 +266,7 @@ socket_mt6_v1(const struct sk_buff *skb, struct xt_action_param *par)
int thoff = 0, tproto;
const struct xt_socket_mtinfo1 *info = (struct xt_socket_mtinfo1 *) par->matchinfo;
- tproto = ipv6_find_hdr(skb, &thoff, -1, NULL, NULL);
+ tproto = ipv6_find_hdr_cb(skb, &thoff, -1, NULL, NULL);
if (tproto < 0) {
pr_debug("unable to find transport header in IPv6 packet, dropping\n");
return NF_DROP;
^ permalink raw reply related
* [RFC PATCH 1/2] net: Cache IPv6 extension header "skip" parsing
From: Jesper Dangaard Brouer @ 2012-09-03 21:16 UTC (permalink / raw)
To: Patrick McHardy, Hans Schillstrom, Hans Schillstrom
Cc: Jesper Dangaard Brouer, netdev, netfilter-devel,
Pablo Neira Ayuso
In-Reply-To: <20120903211504.8851.6894.stgit@dragon>
Store IPv6 exthdr data from ipv6_find_hdr() in skb->cb[].
Introducing struct inet6_skb_exthdr_cache / IP6CB_EXTHDR, as its not
possible to extend inet6_skb_parm directly, or else DCCP dccp_skb_cb
CB size gets too big.
Adding:
__u16 thoff; /* L4 Transport Header offset */
__s16 protocol;/* L4 protocol */
__u16 fragoff; /* packet is a fragment, with offset */
Optimize ip6t_do_table() by using the new ipv6_find_hdr_cb() in
ip6_packet_match(). Before ipv6_find_hdr() were called for each
iptables rule, with a "-p" protocol option (including -p all), which
is very commonly used. The function ipv6_find_hdr_cb() is inlined to
optimize this case and avoid a function call.
The inet6_skb_exthdr_cache / IP6CB_EXTHDR is added to linux/ipv6.h in
the hope that ipv6_skip_exthdr() might also take advantage of this, in
the future.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
Can someone tell me if netfilter (with all of its different HOOKs) can be
allowed to modify data after inet6_skb_parm/IP6CB ???
include/linux/ipv6.h | 15 +++++++++++
include/linux/netfilter_ipv6/ip6_tables.h | 40 +++++++++++++++++++++++++++++
net/ipv6/netfilter/ip6_tables.c | 2 +
3 files changed, 56 insertions(+), 1 deletions(-)
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 0b94e91..21d8cc3 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -266,6 +266,21 @@ struct inet6_skb_parm {
#define IP6CB(skb) ((struct inet6_skb_parm*)((skb)->cb))
#define IP6CBMTU(skb) ((struct ip6_mtuinfo *)((skb)->cb))
+/* Caching of exthdr info
+ * - This is used by netfilter and IPVS.
+ * (cannot extend inet6_skb_parm directly due to DCCP CB size gets too big)
+ */
+struct inet6_skb_exthdr_cache {
+ union {
+ struct inet6_skb_parm h6;
+ } header;
+ /* Caching IPv6 exthdr "skip" info */
+ __u16 thoff; /* L4 Transport Header offset */
+ __s16 protocol;/* L4 protocol */
+ __u16 fragoff; /* packet is a fragment, with offset */
+};
+#define IP6CB_EXTHDR(skb) ((struct inet6_skb_exthdr_cache*)((skb)->cb))
+
static inline int inet6_iif(const struct sk_buff *skb)
{
return IP6CB(skb)->iif;
diff --git a/include/linux/netfilter_ipv6/ip6_tables.h b/include/linux/netfilter_ipv6/ip6_tables.h
index 08c2cbb..5209aff 100644
--- a/include/linux/netfilter_ipv6/ip6_tables.h
+++ b/include/linux/netfilter_ipv6/ip6_tables.h
@@ -303,6 +303,46 @@ enum {
IP6T_FH_F_AUTH = (1 << 1),
};
+/* Function caches IPv6 exthdr data from ipv6_find_hdr() in skb->cb[]
+ * - uses inet6_skb_exthdr_cache / IP6CB_EXTHDR
+ */
+static inline int
+ipv6_find_hdr_cb(const struct sk_buff *skb, unsigned int *offset,
+ int target, unsigned short *fragoff, int *flags)
+{
+ struct inet6_skb_exthdr_cache *exthdr_cache = IP6CB_EXTHDR(skb);
+ __s16 protocol;
+
+ /* Only cache the "skip" exthdr usage */
+ if (target > 0) /* looking for a specific nexthdr target */
+ goto no_cache;
+ if (*offset) /* start at a specific offset */
+ goto no_cache;
+ if (flags) /* not caching flags at the moment */
+ goto no_cache;
+
+ /* Use thoff (transport header) as cache avail indicator */
+ if (exthdr_cache->thoff > 0) {
+ *offset = exthdr_cache->thoff;
+ protocol = exthdr_cache->protocol;
+ if (fragoff)
+ *fragoff = exthdr_cache->fragoff;
+ return protocol;
+ }
+
+ protocol = ipv6_find_hdr(skb, offset, target, fragoff, flags);
+ if (protocol > 0) {
+ /* save a copy in the CB */
+ exthdr_cache->thoff = *offset;
+ exthdr_cache->protocol = protocol;
+ if (fragoff)
+ exthdr_cache->fragoff = *fragoff;
+ }
+ return protocol;
+no_cache:
+ return ipv6_find_hdr(skb, offset, target, fragoff, flags);
+}
+
/* find specified header and get offset to it */
extern int ipv6_find_hdr(const struct sk_buff *skb, unsigned int *offset,
int target, unsigned short *fragoff, int *fragflg);
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index d7cb045..79367f2 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -133,7 +133,7 @@ ip6_packet_match(const struct sk_buff *skb,
int protohdr;
unsigned short _frag_off;
- protohdr = ipv6_find_hdr(skb, protoff, -1, &_frag_off, NULL);
+ protohdr = ipv6_find_hdr_cb(skb, protoff, -1, &_frag_off, NULL);
if (protohdr < 0) {
if (_frag_off == 0)
*hotdrop = true;
^ permalink raw reply related
* [RFC PATCH 0/2] RFC: Caching IPv6 exthdr in skb->cb[]
From: Jesper Dangaard Brouer @ 2012-09-03 21:15 UTC (permalink / raw)
To: Patrick McHardy, Hans Schillstrom, Hans Schillstrom
Cc: Jesper Dangaard Brouer, netdev, netfilter-devel,
Pablo Neira Ayuso
In-Reply-To: <Pine.GSO.4.63.1208291427170.26100@stinky-local.trash.net>
Hi Patrick and Hans,
This is my followup to:
[PATCH 2/3] ipvs: Fix faulty IPv6 extension header handling in IPVS
Where you proposed improving the overall architecture of IPv6
extension header parsing not only for IPVS but for other related
Netfilter subsystems as well.
We discussed using/extending inet6_skb_parm/IP6CB. There was not
enough room for extending inet6_skb_parm directly, so I have
introduced a struct inet6_skb_exthdr_cache/IP6CB_EXTHDR, which extend
IP6CB.
The question is if this approach will work. Can netfilter be allowed
to modify data after inet6_skb_parm/IP6CB, given all the different
HOOKs ?
If we find this is a valid approach, then I'll update the IPVS patches
to also use this.
This patchset is based upon:
Patrick's tree: git://github.com/kaber/nf-next-ipv6-nat.git
---
Jesper Dangaard Brouer (2):
netfilter: More users of ipv6_find_hdr_cb()
net: Cache IPv6 extension header "skip" parsing
include/linux/ipv6.h | 15 +++++++++++
include/linux/netfilter_ipv6/ip6_tables.h | 40 +++++++++++++++++++++++++++++
net/ipv6/netfilter/ip6_tables.c | 2 +
net/netfilter/xt_TPROXY.c | 2 +
net/netfilter/xt_socket.c | 2 +
5 files changed, 58 insertions(+), 3 deletions(-)
--
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
* Re: [PATCH V2 09/12] net/eipoib: Add main driver functionality
From: Or Gerlitz @ 2012-09-03 20:53 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eric W. Biederman, Or Gerlitz, davem, roland, netdev, sean.hefty,
Erez Shitrit, Ali Ayoub, Doug Ledford
In-Reply-To: <20120805185031.GA18640@redhat.com>
Michael S. Tsirkin <mst@redhat.com> wrote:
> [...] so it seems that a sane solution would involve an extra level of
> indirection, with guest addresses being translated to host IB addresses.
> As long as you do this, maybe using an ethernet frame format makes sense.
> So far the things that make sense. Here are some that don't, to me:
> - Is a pdf presentation all you have in terms of documentation?
> We are talking communication protocols here - I would expect a
> proper spec, and some effort to standardize, otherwise where's the
> guarantee it won't change in an incompatible way?
> Other things that I would expect to be addressed in such a spec is
> interaction with other IPoIB features, such as connected
> mode, checksum offloading etc, and IB features such as multipath etc.
>
> - The way you encode LID/QPN in the MAC seems questionable. IIRC there's
> more to IB addressing than just the LID. Since everyone on the subnet
> need access to this translation, I think it makes sense to store it in
> the SM. I think this would also obviate some IPv4 specific hacks in kernel.
> - IGMP/MAC snooping in a driver is just too hairy.
> As you point out, bridge currently needs the uplink in promisc mode.
> I don't think a driver should work around that limitation.
> For some setups, it might be interesting to remove the promisc
> mode requirement, failing that, I think you could use macvtap passthrough.
>
> - Currently migration works without host kernel help, would be
> preferable to keep it that way.
Hi Michael,
If we rewind to this point, basically, you had few concerns
0. not enough documentation
1. the sender VM MAC isn't preserved when the packet is received
2. the IGMP snooping we planned to do within netdevice - isn't good practice
3. mangling of ARPs within netdevice - isn't good practice as well.
For 0,1,2 we have a way to address (see below)
So we are remained with #3 - the ARPs -- thinking on this a little
further, FWIW there --are-- components in the kernel which
mangle/generate ARPs and are exposing netdevice, such as openvswitch,
anyway:
does it make sense to forward ARPs received into / sent over the
eIPoIB netdevice (e.g using some sort of rule) to some outer entity
such as user-space
daemon for interception and later re-injection into eIPoIB?
Or.
Documentation we will fix,
Preserving remote VM mac at the receiver we have few directions for
solution, e.g either along your suggestion with SA records and/or with
using "alias GUIDs" (details TBD when the submission resumes).
Multicast we accept the direction you suggested - implement support
for multicast non promiscuous in the elements "above" eIPoIB (bridge,
macvtap, etc).
^ permalink raw reply
* Re: when the MTU interface is modified, the promiscuous mode is reset in gianfar driver
From: David Miller @ 2012-09-03 19:53 UTC (permalink / raw)
To: chikazawa.akifu; +Cc: netdev, linux-kernel
In-Reply-To: <201208270838.AA02326@VF09040P.jp.fujitsu.com>
From: chikazawa.akifu@jp.fujitsu.com (近沢 哲史)
Date: Mon, 27 Aug 2012 17:38:34 +0900
> I am using the gianfar ethernet driver. I am having a problem with the
> interface settings.
> Under promiscuous mode, when the MTU interface is modified, the promiscuous
> mode setting is turned off in gianfar driver when it should not be.
> The details are as follows:
> After changing MTU with ifconfig, I could see that the interface flag of
> eth0 is still PROMISC.
> However, when I checked value of RCTL register with ethtool, PROM bit of
> RTCL register is cleared.
> It seems to be cause that is the gfar_init_mac() function, it doesn't set
> the PROM bit after the interface MTU is changed.
> This problem was detected on linux-2.6.32.2, but it seems to same on
> linux-3.6.0-rc3.
> Is this behavior on purpose?
>
> I also attach the amended file,I think it would be so.
>
> Signed-off-by: Akifumi Chikazawa <chikazawa.akifu@jp.fujitsu.com>
It seems like we also lose all of the multicast configurations as
well.
Therefore, the thing to do is to simply call gfar_set_multi() at the
appropriate location. That will take care of both the promiscuous
bit, as well as the multicast addresses.
You can then remove some of the code in gfar_init_mac() that does
things like gfar_clear_exact_match(), because gfar_set_multi() will
take care of that if necessary.
^ permalink raw reply
* v3 for tcp friends?
From: David Miller @ 2012-09-03 19:48 UTC (permalink / raw)
To: netdev; +Cc: brutus
Bruce can we get a new spin of the patch? I'd really like to get this in
soon so it gets a lot of testing before the next merge window opens up.
Thanks.
^ permalink raw reply
* Re: [PATCH] mISDN: fix possible memory leak in hfcmulti_init()
From: David Miller @ 2012-09-03 19:37 UTC (permalink / raw)
To: weiyj.lk; +Cc: isdn, yongjun_wei, netdev
In-Reply-To: <CAPgLHd_vRu-ACoVhf2LwL_1pYXuvXArindYOpHh6T-syn_CVUg@mail.gmail.com>
From: Wei Yongjun <weiyj.lk@gmail.com>
Date: Mon, 3 Sep 2012 15:31:00 +0800
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> hc has been allocated in this function and missing free it before
> leaving from some error handling cases.
>
> spatch with a semantic match is used to found this problem.
> (http://coccinelle.lip6.fr/)
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] fq_codel: dont reinit flow state
From: David Miller @ 2012-09-03 18:37 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, dave.taht
In-Reply-To: <1346505597.7996.76.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 01 Sep 2012 06:19:57 -0700
> When fq_codel builds a new flow, it should not reset codel state.
>
> Codel algo needs to get previous values (lastcount, drop_next) to get
> proper behavior.
>
> Signed-off-by: Dave Taht <dave.taht@gmail.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied to 'net', thanks.
^ permalink raw reply
* Re: [PATCH 3/3] tcp: use PRR to reduce cwin in CWR state
From: David Miller @ 2012-09-03 18:34 UTC (permalink / raw)
To: ncardwell; +Cc: ycheng, nanditad, mattmathis, edumazet, netdev
In-Reply-To: <CADVnQyma09UtSLusyQ6986uz2yfy-3xZ+mXrHfn1A2HO+3=X_g@mail.gmail.com>
From: Neal Cardwell <ncardwell@google.com>
Date: Mon, 3 Sep 2012 00:00:44 -0400
> On Sun, Sep 2, 2012 at 11:38 PM, Yuchung Cheng <ycheng@google.com> wrote:
>> Use proportional rate reduction (PRR) algorithm to reduce cwnd in CWR state,
>> in addition to Recovery state. Retire the current rate-halving in CWR.
>> When losses are detected via ACKs in CWR state, the sender enters Recovery
>> state but the cwnd reduction continues and does not restart.
>>
>> Rename and refactor cwnd reduction functions since both CWR and Recovery
>> use the same algorithm:
>> tcp_init_cwnd_reduction() is new and initiates reduction state variables.
>> tcp_cwnd_reduction() is previously tcp_update_cwnd_in_recovery().
>> tcp_ends_cwnd_reduction() is previously tcp_complete_cwr().
>>
>> The rate halving functions and logic such as tcp_cwnd_down(), tcp_min_cwnd(),
>> and the cwnd moderation inside tcp_enter_cwr() are removed. The unused
>> parameter, flag, in tcp_cwnd_reduction() is also removed.
>>
>> Signed-off-by: Yuchung Cheng <ycheng@google.com>
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
Applied to net-next
^ permalink raw reply
* Re: [PATCH 2/3] tcp: move tcp_update_cwnd_in_recovery
From: David Miller @ 2012-09-03 18:34 UTC (permalink / raw)
To: ncardwell; +Cc: ycheng, nanditad, mattmathis, edumazet, netdev
In-Reply-To: <CADVnQy=RH=3bGhfiKpMBCY-6n16FY9iSXBRWDbbT7h+5FbKKWg@mail.gmail.com>
From: Neal Cardwell <ncardwell@google.com>
Date: Sun, 2 Sep 2012 23:57:02 -0400
> On Sun, Sep 2, 2012 at 11:38 PM, Yuchung Cheng <ycheng@google.com> wrote:
>> To prepare replacing rate halving with PRR algorithm in CWR state.
>>
>> Signed-off-by: Yuchung Cheng <ycheng@google.com>
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
Applied to net-next
^ permalink raw reply
* Re: [PATCH 1/3] tcp: move tcp_enter_cwr()
From: David Miller @ 2012-09-03 18:34 UTC (permalink / raw)
To: ncardwell; +Cc: ycheng, nanditad, mattmathis, edumazet, netdev
In-Reply-To: <CADVnQymfu8YdGT_Rc0GBQ5wzncVQi6-BmTLtzD3MwQV8A3WFUQ@mail.gmail.com>
From: Neal Cardwell <ncardwell@google.com>
Date: Sun, 2 Sep 2012 23:56:09 -0400
> On Sun, Sep 2, 2012 at 11:38 PM, Yuchung Cheng <ycheng@google.com> wrote:
>> To prepare replacing rate halving with PRR algorithm in CWR state.
>>
>> Signed-off-by: Yuchung Cheng <ycheng@google.com>
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
Applied to net-next
^ permalink raw reply
* Re: Possible issue with Mellanox mlx4/port handling
From: Marcelo Ricardo Leitner @ 2012-09-03 18:16 UTC (permalink / raw)
To: Yevgeny Petrilin; +Cc: netdev@vger.kernel.org, Or Gerlitz
In-Reply-To: <953B660C027164448AE903364AC447D28720CFEB@MTLDAG01.mtl.com>
On 09/03/2012 02:51 PM, Yevgeny Petrilin wrote:
>> If you can share, what can we expect it to be like? Like the chunk I suggested
>> above or is there anything else needed? I could notice only 6 places calling
>> get_promisc_qp() that way and couldn't find any other issue like that.
>>
>
> The fix is pretty much the same style like what you suggested.
Awesome. Thanks!
Marcelo
^ permalink raw reply
* RE: Possible issue with Mellanox mlx4/port handling
From: Yevgeny Petrilin @ 2012-09-03 17:51 UTC (permalink / raw)
To: mleitner@redhat.com; +Cc: netdev@vger.kernel.org, Or Gerlitz
In-Reply-To: <5044ECA9.5080100@redhat.com>
> If you can share, what can we expect it to be like? Like the chunk I suggested
> above or is there anything else needed? I could notice only 6 places calling
> get_promisc_qp() that way and couldn't find any other issue like that.
>
The fix is pretty much the same style like what you suggested.
Yevgeny
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: sierra_net: rx_urb_size is constant
From: David Miller @ 2012-09-03 17:51 UTC (permalink / raw)
To: bjorn; +Cc: netdev, linux-usb
In-Reply-To: <1346664033-30284-3-git-send-email-bjorn@mork.no>
From: Bjørn Mork <bjorn@mork.no>
Date: Mon, 3 Sep 2012 11:20:33 +0200
> The rx_urb_size is set to the same value for every device
> supported by this driver. No need to keep a per-device
> data structure to do that. Replacing with a macro constant.
>
> This was the last device specific info, and removing it
> allows us to delete the sierra_net_info_data struct.
>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: sierra_net: make private symbols static
From: David Miller @ 2012-09-03 17:51 UTC (permalink / raw)
To: bjorn; +Cc: netdev, linux-usb
In-Reply-To: <1346664033-30284-2-git-send-email-bjorn@mork.no>
From: Bjørn Mork <bjorn@mork.no>
Date: Mon, 3 Sep 2012 11:20:32 +0200
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: cx82310_eth: use common match macro
From: David Miller @ 2012-09-03 17:51 UTC (permalink / raw)
To: bjorn-yOkvZcmFvRU
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1346664033-30284-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
From: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Date: Mon, 3 Sep 2012 11:20:31 +0200
> Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net] net: usbnet: fix softirq storm on suspend
From: David Miller @ 2012-09-03 17:49 UTC (permalink / raw)
To: bjorn-yOkvZcmFvRU
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw, oneukum-l3A5Bk7waGM
In-Reply-To: <1346660778-24539-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
From: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Date: Mon, 3 Sep 2012 10:26:18 +0200
> Suspending an open usbnet device results in constant
> rescheduling of usbnet_bh.
>
> commit 65841fd5 "usbnet: handle remote wakeup asap"
> refactored the usbnet_bh code to allow sharing the
> urb allocate and submit code with usbnet_resume. In
> this process, a test for, and immediate return on,
> ENOLINK from rx_submit was unintentionally dropped.
>
> The rx queue will not grow if rx_submit fails,
> making usbnet_bh reschedule itself. This results
> in a softirq storm if the error is persistent.
> rx_submit translates the usb_submit_urb error
> EHOSTUNREACH into ENOLINK, so this is an expected
> and persistent error for a suspended device. The
> old code tested for this condition and avoided
> rescheduling. Putting this test back.
>
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.5
> Cc: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Cc: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
> Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Possible issue with Mellanox mlx4/port handling
From: Marcelo Ricardo Leitner @ 2012-09-03 17:45 UTC (permalink / raw)
To: Yevgeny Petrilin; +Cc: netdev@vger.kernel.org, Or Gerlitz
In-Reply-To: <953B660C027164448AE903364AC447D28720CF7D@MTLDAG01.mtl.com>
On 09/03/2012 02:32 PM, Yevgeny Petrilin wrote:
>> Commit
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=4c41b3673759d096106e68bce586f103c51d4119
>> inserted changes like:
>>
>> @@ -361,7 +361,7 @@ static int add_promisc_qp(struct mlx4_dev *dev, u8
>> port,
>> int err;
>> struct mlx4_priv *priv = mlx4_priv(dev);
>>
>> - s_steer =&mlx4_priv(dev)->steer[0];
>> + s_steer =&mlx4_priv(dev)->steer[port - 1];
>>
>> mutex_lock(&priv->mcg_table.mutex);
>>
>> But I fear we missed one part of the deal. Concept patch:
>>
>> @@ -365,7 +365,7 @@ static int add_promisc_qp(struct mlx4_dev *dev, u8
>> port,
>>
>> mutex_lock(&priv->mcg_table.mutex);
>>
>> - if (get_promisc_qp(dev, 0, steer, qpn)) {
>> + if (get_promisc_qp(dev, port - 1, steer, qpn)) {
>> err = 0; /* Noting to do, already exists */
>> goto out_mutex;
>> }
>>
> ...
>>
>> As far as I can understand, we are changing a list for a port and checking for
>> duplicates on the other list. Points marked as A, B and C for highlighting. Am I
>> missing something? What do you think?
>>
>> FWIW, this call get_promisc_qp(dev, 0, ...) happens in other places too.
>>
>> Thank you,
>> Marcelo.
>
> Hi Marcelo,
> Thanks for this, You are absolutely right.
> We actually have a fix for this issue which we are now verifying, and it will be sent to the mailing list in a few days.
Hi Yevgeny,
Thanks for the fast confirmation.
If you can share, what can we expect it to be like? Like the chunk I
suggested above or is there anything else needed? I could notice only 6
places calling get_promisc_qp() that way and couldn't find any other
issue like that.
Thanks,
Marcelo.
^ permalink raw reply
* RE: Possible issue with Mellanox mlx4/port handling
From: Yevgeny Petrilin @ 2012-09-03 17:32 UTC (permalink / raw)
To: mleitner@redhat.com; +Cc: netdev@vger.kernel.org, Or Gerlitz
> Commit
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=4c41b3673759d096106e68bce586f103c51d4119
> inserted changes like:
>
> @@ -361,7 +361,7 @@ static int add_promisc_qp(struct mlx4_dev *dev, u8
> port,
> int err;
> struct mlx4_priv *priv = mlx4_priv(dev);
>
> - s_steer = &mlx4_priv(dev)->steer[0];
> + s_steer = &mlx4_priv(dev)->steer[port - 1];
>
> mutex_lock(&priv->mcg_table.mutex);
>
> But I fear we missed one part of the deal. Concept patch:
>
> @@ -365,7 +365,7 @@ static int add_promisc_qp(struct mlx4_dev *dev, u8
> port,
>
> mutex_lock(&priv->mcg_table.mutex);
>
> - if (get_promisc_qp(dev, 0, steer, qpn)) {
> + if (get_promisc_qp(dev, port - 1, steer, qpn)) {
> err = 0; /* Noting to do, already exists */
> goto out_mutex;
> }
>
...
>
> As far as I can understand, we are changing a list for a port and checking for
> duplicates on the other list. Points marked as A, B and C for highlighting. Am I
> missing something? What do you think?
>
> FWIW, this call get_promisc_qp(dev, 0, ...) happens in other places too.
>
> Thank you,
> Marcelo.
Hi Marcelo,
Thanks for this, You are absolutely right.
We actually have a fix for this issue which we are now verifying, and it will be sent to the mailing list in a few days.
Thanks,
Yevgeny
^ permalink raw reply
* Re: [PATCH] sctp: Don't charge for data in sndbuf again when transmitting packet
From: David Miller @ 2012-09-03 17:24 UTC (permalink / raw)
To: vyasevich; +Cc: tgraf, linux-sctp, netdev, vyasevic, nhorman
In-Reply-To: <378685E0-7225-4C9D-AE0F-7B8767ECB16A@gmail.com>
From: Vlad Yasevich <vyasevich@gmail.com>
Date: Mon, 3 Sep 2012 11:02:51 -0400
>
>
> On Sep 3, 2012, at 10:27 AM, Thomas Graf <tgraf@suug.ch> wrote:
>
>> SCTP charges wmem_alloc via sctp_set_owner_w() in sctp_sendmsg() and
>> via
>> skb_set_owner_w() in sctp_packet_transmit(). If a sender runs out of
>> sndbuf it will sleep in sctp_wait_for_sndbuf() and expects to be waken
>> up
>> by __sctp_write_space().
>>
>> Buffer space charged via sctp_set_owner_w() is released in
>> sctp_wfree()
>> which calls __sctp_write_space() directly.
>>
>> Buffer space charged via skb_set_owner_w() is released via
>> sock_wfree()
>> which calls sk->sk_write_space() _if_ SOCK_USE_WRITE_QUEUE is not set.
>> sctp_endpoint_init() sets SOCK_USE_WRITE_QUEUE on all sockets.
>>
>> Therefore if sctp_packet_transmit() manages to queue up more than
>> sndbuf
>> bytes, sctp_wait_for_sndbuf() will never be woken up again unless it
>> is
>> interrupted by a signal.
>>
>> This could be fixed by clearing the SOCK_USE_WRITE_QUEUE flag but ...
>>
>> Charging for the data twice does not make sense in the first place, it
>> leads to overcharging sndbuf by a factor 2. Therefore this patch only
>> charges a single byte in wmem_alloc when transmitting an SCTP packet
>> to
>> ensure that the socket stays alive until the packet has been released.
>>
>> This means that control chunks are no longer accounted for in
>> wmem_alloc
>> which I believe is not a problem as skb->truesize will typically lead
>> to overcharging anyway and thus compensates for any control overhead.
>
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>
Applied and queued up for -stable, thanks everyone.
^ permalink raw reply
* Re: [PATCH v2] netfilter: take care of timewait sockets
From: David Miller @ 2012-09-03 17:23 UTC (permalink / raw)
To: eric.dumazet; +Cc: fw, hvtaifwkbgefbaei, netdev, e1000-devel
In-Reply-To: <1346666238.2563.113.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 03 Sep 2012 11:57:18 +0200
> David, tell me if you prefer to change TCP demux to avoid timewait,
> as I have no strong opinion.
It would be the stupidest thing ever to do the whole hash lookup
just to throw the result away just because it's a timewait socket.
> [PATCH] net: sock_edemux() should take care of timewait sockets
>
> sock_edemux() can handle either a regular socket or a timewait socket
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks.
^ permalink raw reply
* Re: [net-next.git 5/7] stmmac: add sysFs support
From: David Miller @ 2012-09-03 17:20 UTC (permalink / raw)
To: bhutchings; +Cc: peppe.cavallaro, netdev
In-Reply-To: <1346676267.7787.82.camel@deadeye.wl.decadent.org.uk>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 3 Sep 2012 13:44:27 +0100
> On Mon, 2012-09-03 at 09:47 +0200, Giuseppe CAVALLARO wrote:
>> This patch adds the sysFs support.
>> Some internal driver parameters can be tuned by using some
>> entries exposed via sysFS. There parameter currently are,
>> for example, for internal timers used to mitigate the rx/tx
>> interrupts or for EEE.
> [...]
>
> Why are you not exposing these through the standard ethtool operations?
Guiseppe, I'm not appyling driver patches that add sysfs crap like
this. Either use existing ethtool interfaces or create new ones that
provide the necessary functionality.
Adding unique configuration mechanisms to a device driver is always a
bug.
And I'm getting real fed up with driver writers simply not getting the
message. Every time someone adds sysfs or ioctl crap, we push back,
so just don't do it and stop wasting our time.
^ permalink raw reply
* Re: [PATCH 2/2] netfilter: pass 'nf_hook_ops' instead of 'list_head' to nf_queue()
From: Pablo Neira Ayuso @ 2012-09-03 15:31 UTC (permalink / raw)
To: Michael Wang
Cc: LKML, netdev@vger.kernel.org, coreteam, netfilter,
netfilter-devel, David Miller, kaber
In-Reply-To: <5035C6E6.6040001@linux.vnet.ibm.com>
On Thu, Aug 23, 2012 at 02:00:06PM +0800, Michael Wang wrote:
> From: Michael Wang <wangyun@linux.vnet.ibm.com>
>
> Since 'list_for_each_continue_rcu' has already been replaced by
> 'list_for_each_entry_continue_rcu', pass 'list_head' to nf_queue() as a
> parameter can not benefit us any more.
>
> This patch will replace 'list_head' with 'nf_hook_ops' as the parameter of
> nf_queue() and __nf_queue() to save code.
Applied, thanks.
^ 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