* Re: [RFC net-next] ipip: Add room for custom tunnel header
From: Kristian Evensen @ 2013-08-16 13:11 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1375977658.4004.106.camel@edumazet-glaptop>
Hi,
Thank you very much for your feedback and sorry for not replying
earlier. I modified the patch and implemented setting a custom header
length as a netlink message attribute instead
(http://patchwork.ozlabs.org/patch/266456/). It was initially rejected
on the basis that I needed to provide some use-cases, which I did, so
lets see what happens. However, your comments apply to both patches,
so it would be interesting to discuss this.
> What happens if hlen is bigger than available headroom ?
>
> hlen comes from userspace and there is no safety check, right ?
Based on my understanding of the code, hlen will not be bigger than
available headroom. dev->needed_headroom is set to the tunnel header
length in ip_tunnel_bind_dev(). hlen is only read when adding a new
link, so this assignment cannot be skipped. However, I see that a
check along the lines of "hlen > 0 && hlen < (ETH_DATA_LEN -
LL_MAX_HDR - sizeof(iphdr)" should be added.
One potential side-effect is that one can overflow the mtu by
providing a large hlen. Will that be considered a security risk or
misconfiguration? Since you have to have the same rights to create a
tunnel as to create a link, a similar behavior can be accomplished by
creating a new link with a large mtu.
> What guarantee do we have _something_ will fill the bytes ?
> (We do not want to leak prior content of those bytes to the wire)
In the current patch, none. I assumed this was ok as inserting the
custom header does not make sense without another module filling in
the content. I do agree though, a memset should be added to zero out
the custom header.
Thanks again,
Kristian
^ permalink raw reply
* Re: [PATCH v4 2/5] ARM: davinci: da850: add DT node for mdio device
From: Sekhar Nori @ 2013-08-16 12:41 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, DLOS, LKML,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1376546497-26931-3-git-send-email-prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Thursday 15 August 2013 11:31 AM, Lad, Prabhakar wrote:
> From: "Lad, Prabhakar" <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Add mdio device tree node information to da850 by
> providing register details and bus frequency of mdio.
>
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Queuing for v3.12
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH v4 1/5] ARM: davinci: fix clock lookup for mdio device
From: Sekhar Nori @ 2013-08-16 12:35 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
netdev-u79uwXL29TY76Z2rM5mHXA, DLOS, LKML,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1376546497-26931-2-git-send-email-prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Thursday 15 August 2013 11:31 AM, Lad, Prabhakar wrote:
> From: "Lad, Prabhakar" <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> This patch removes the clock alias for mdio device and adds a entry
> in clock lookup table, this entry can now be used by both DT and NON
> DT case.
>
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Queuing for v3.12
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH][net-next] gianfar: Add backwards compatible Single Queue mode polling
From: Lutz Jaenicke @ 2013-08-16 11:54 UTC (permalink / raw)
To: Michael Guntsche; +Cc: Claudiu Manoil, netdev
In-Reply-To: <CALG0vJtQkM65kr2-u5G578uzAFeCk5=bkKNU5BaBheXTHmc7+g@mail.gmail.com>
On Mon, Jun 10, 2013 at 07:35:38PM +0200, Michael Guntsche wrote:
> Hello Claudiu,
>
> On Mon, Jun 10, 2013 at 7:19 PM, Claudiu Manoil
> <claudiu.manoil@freescale.com> wrote:
> > Older Single Queue (SQ_SG_MODE) devices like TSEC (i.e. mpc83xx)
> > don't feature the frame receive indication bits (RXF) in RSTAT.
> > For these and for the rest of the SQ_SG_MODE devices, provide the
> > appropiate polling routine that handles a single pair of Rx/Tx
> > BD rings, removing the overhead incurred by the multiple queues/
> > multiple interrupt group devices (veTSEC/ eTSEC2.0 devices).
> > So this is primarily a fix for the TSEC devices.
> >
> > Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
> > ---
> > drivers/net/ethernet/freescale/gianfar.c | 51 ++++++++++++++++++++++++++++++--
> > 1 file changed, 49 insertions(+), 2 deletions(-)
> >
> <snip>
> I can confirm that this fixes the problem for my TSEC based board.
I need this fix for 3.10.7 as well, so this seems to be stable-stuff.
Best regards,
Lutz
--
Dr.-Ing. Lutz Jänicke
CTO
Innominate Security Technologies AG /protecting industrial networks/
tel: +49.30.921028-200
fax: +49.30.921028-020
Rudower Chaussee 13
D-12489 Berlin, Germany
www.innominate.com
Register Court: AG Charlottenburg, HR B 81603
Management Board: Dirk Seewald
Chairman of the Supervisory Board: Christoph Leifer
^ permalink raw reply
* [PATCH net] ipv6: drop packets with multiple fragmentation headers
From: Hannes Frederic Sowa @ 2013-08-16 11:30 UTC (permalink / raw)
To: netdev; +Cc: yoshfuji
It is not allowed for an ipv6 packet to contain multiple fragmentation
headers. So discard packets which were already reassembled by
fragmentation logic and send back a parameter problem icmp.
The updates for RFC 6980 will come in later, I have to do a bit more
research here.
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
include/linux/ipv6.h | 1 +
net/ipv6/reassembly.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 77a4784..9ac5047 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -103,6 +103,7 @@ struct inet6_skb_parm {
#define IP6SKB_FORWARDED 2
#define IP6SKB_REROUTED 4
#define IP6SKB_ROUTERALERT 8
+#define IP6SKB_FRAGMENTED 16
};
#define IP6CB(skb) ((struct inet6_skb_parm*)((skb)->cb))
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 790d9f4..1aeb473 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -490,6 +490,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
ipv6_hdr(head)->payload_len = htons(payload_len);
ipv6_change_dsfield(ipv6_hdr(head), 0xff, ecn);
IP6CB(head)->nhoff = nhoff;
+ IP6CB(head)->flags |= IP6SKB_FRAGMENTED;
/* Yes, and fold redundant checksum back. 8) */
if (head->ip_summed == CHECKSUM_COMPLETE)
@@ -524,6 +525,9 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
struct net *net = dev_net(skb_dst(skb)->dev);
int evicted;
+ if (IP6CB(skb)->flags & IP6SKB_FRAGMENTED)
+ goto fail_hdr;
+
IP6_INC_STATS_BH(net, ip6_dst_idev(skb_dst(skb)), IPSTATS_MIB_REASMREQDS);
/* Jumbo payload inhibits frag. header */
@@ -544,6 +548,7 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
ip6_dst_idev(skb_dst(skb)), IPSTATS_MIB_REASMOKS);
IP6CB(skb)->nhoff = (u8 *)fhdr - skb_network_header(skb);
+ IP6CB(skb)->flags |= IP6SKB_FRAGMENTED;
return 1;
}
--
1.8.3.1
^ permalink raw reply related
* Re: MASQUERADE/SNAT and multiple interfaces with the same IP
From: Kristian Evensen @ 2013-08-16 11:23 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter, netdev
In-Reply-To: <20130816111826.GC5154@breakpoint.cc>
On Fri, Aug 16, 2013 at 1:18 PM, Florian Westphal <fw@strlen.de> wrote:
>> Does this mean that even when using SNAT, the old mapping is deleted
>> and a new created?
>
> No.
Thank you very much for the help, I see now that I was a little bit
too fast and missed a detail in nf_nat_oif_changed(). If anyone ever
stumbles across the question in the future: SNAT does not set
masq_index and, thus, nf_nat_oif_changed() will always return false.
Have a nice weekend!
-Kristian
^ permalink raw reply
* Re: MASQUERADE/SNAT and multiple interfaces with the same IP
From: Florian Westphal @ 2013-08-16 11:18 UTC (permalink / raw)
To: Kristian Evensen; +Cc: Florian Westphal, netfilter, netdev
In-Reply-To: <CAKfDRXgNtPgQjtrSL=oVkws6GB_sTv+h+-_GLVvVvpWoEHBRnw@mail.gmail.com>
Kristian Evensen <kristian.evensen@gmail.com> wrote:
> I added a pr_info() here to see when this function was called. It is
> triggered when the link goes down, as expected, but not when I just
> change the mark and move the flows to another interface. I still see
> the DESTROY/NEW-behavior though.
Yes, its handled by iptable_nat.c
> > Also, the nat core zaps conntracks when they are sent out on an
> > interface other than the original interface.
> >
> > See nf_nat_oif_changed() in iptable_nat.c
>
> Thanks for letting me know, I will start looking into this function.
> Does this mean that even when using SNAT, the old mapping is deleted
> and a new created?
No.
> And does this event not notify conntrack?
It generates a destroy event.
^ permalink raw reply
* Re: MASQUERADE/SNAT and multiple interfaces with the same IP
From: Kristian Evensen @ 2013-08-16 11:14 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter, netdev
In-Reply-To: <20130816110443.GB5154@breakpoint.cc>
Thank you very much for your reply.
On Fri, Aug 16, 2013 at 1:04 PM, Florian Westphal <fw@strlen.de> wrote:>
> The MASQUERADE target zaps conntracks that it "owned" when the
> associated link goes down, see masq_device_event() in ipt_MASQUERADE.c
I added a pr_info() here to see when this function was called. It is
triggered when the link goes down, as expected, but not when I just
change the mark and move the flows to another interface. I still see
the DESTROY/NEW-behavior though.
> Also, the nat core zaps conntracks when they are sent out on an
> interface other than the original interface.
>
> See nf_nat_oif_changed() in iptable_nat.c
Thanks for letting me know, I will start looking into this function.
Does this mean that even when using SNAT, the old mapping is deleted
and a new created? And does this event not notify conntrack? I have so
far used conntrack -E to monitor changes to nat, but that is perhaps
incorrect?
-Kristian
^ permalink raw reply
* [PATCH net-next] sis900: don't restart auto-negotiation each time after link resume.
From: Denis Kirjanov @ 2013-08-16 7:20 UTC (permalink / raw)
To: netdev, davem; +Cc: Denis Kirjanov
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
drivers/net/ethernet/sis/sis900.c | 28 +++++++++++-----------------
1 file changed, 11 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/sis/sis900.c b/drivers/net/ethernet/sis/sis900.c
index f5d7ad7..b7a3930 100644
--- a/drivers/net/ethernet/sis/sis900.c
+++ b/drivers/net/ethernet/sis/sis900.c
@@ -1309,23 +1309,9 @@ static void sis900_timer(unsigned long data)
struct sis900_private *sis_priv = netdev_priv(net_dev);
struct mii_phy *mii_phy = sis_priv->mii;
static const int next_tick = 5*HZ;
+ int speed = 0, duplex = 0;
u16 status;
- if (!sis_priv->autong_complete){
- int uninitialized_var(speed), duplex = 0;
-
- sis900_read_mode(net_dev, &speed, &duplex);
- if (duplex){
- sis900_set_mode(sis_priv, speed, duplex);
- sis630_set_eq(net_dev, sis_priv->chipset_rev);
- netif_carrier_on(net_dev);
- }
-
- sis_priv->timer.expires = jiffies + HZ;
- add_timer(&sis_priv->timer);
- return;
- }
-
status = mdio_read(net_dev, sis_priv->cur_phy, MII_STATUS);
status = mdio_read(net_dev, sis_priv->cur_phy, MII_STATUS);
@@ -1336,8 +1322,16 @@ static void sis900_timer(unsigned long data)
status = sis900_default_phy(net_dev);
mii_phy = sis_priv->mii;
- if (status & MII_STAT_LINK)
- sis900_check_mode(net_dev, mii_phy);
+ if (status & MII_STAT_LINK) {
+ WARN_ON(!(status & MII_STAT_AUTO_DONE));
+
+ sis900_read_mode(net_dev, &speed, &duplex);
+ if (duplex) {
+ sis900_set_mode(sis_priv, speed, duplex);
+ sis630_set_eq(net_dev, sis_priv->chipset_rev);
+ netif_carrier_on(net_dev);
+ }
+ }
} else {
/* Link ON -> OFF */
if (!(status & MII_STAT_LINK)){
--
1.8.0.2
^ permalink raw reply related
* Re: [PATCH 1/3 v4] ipv6: do not disable temp_address when reaching max_address
From: Hannes Frederic Sowa @ 2013-08-16 11:07 UTC (permalink / raw)
To: Ding Tianhong
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Jon Maloy, Eric Dumazet,
Netdev, kargig, ppandit
In-Reply-To: <520D76EC.6000205@huawei.com>
On Fri, Aug 16, 2013 at 08:48:44AM +0800, Ding Tianhong wrote:
> On 2013/8/16 3:07, Hannes Frederic Sowa wrote:
> > On Thu, Aug 15, 2013 at 07:36:03PM +0200, Hannes Frederic Sowa wrote:
> >> Now we have to check why these addresses don't go out of tentative state.
> >
> > Just looked at it. flood_router26 just emits pretty high values for
> > RetransTime:
> >
> > 21:05:52.050159 IP6 (hlim 255, next-header ICMPv6 (58) payload length: 192) fe80::c:2a47:1360:1101 > ff02::1: [icmp6 sum ok] ICMP6, router advertisement, length 192
> > hop limit 255, Flags [none], pref high, router lifetime 65535s, reachable time 16384000ms, retrans time 1966080ms
> > mtu option (5), length 8 (1): 1500
> > 0x0000: 0000 0000 05dc
> >
> > We are completly in spec here. Just DAD needs a lot of time to finish.
> >
> > Greetings,
> >
> > Hannes
> >
> >
> Great job, more and more close to the truth, let's find a better way to finish it.
Thanks! :)
I just sent out a patch with the removal of the max_addresses
check. Please have a look!
I tested it with flood_router26 and modified it to also change the
retrans_time, because it affects in which interval addresses get
regenerated. We always were on the safe side.
Greetings,
Hannes
^ permalink raw reply
* Re: MASQUERADE/SNAT and multiple interfaces with the same IP
From: Florian Westphal @ 2013-08-16 11:04 UTC (permalink / raw)
To: Kristian Evensen; +Cc: netfilter, netdev
In-Reply-To: <CAKfDRXjXkY--PjXAa1m=As+Vf_1__WBokkTAE5BHg+gxsM9Yxw@mail.gmail.com>
Kristian Evensen <kristian.evensen@gmail.com> wrote:
> After spending long time looking at the source code, I can't figure
> out why this happens. Once the MASQUERADE/SNAT rule has been inserted,
> to me everything looks the same.
The MASQUERADE target zaps conntracks that it "owned" when the
associated link goes down, see masq_device_event() in ipt_MASQUERADE.c
Also, the nat core zaps conntracks when they are sent out on an
interface other than the original interface.
See nf_nat_oif_changed() in iptable_nat.c
^ permalink raw reply
* [PATCH net] ipv6: remove max_addresses check from ipv6_create_tempaddr
From: Hannes Frederic Sowa @ 2013-08-16 11:02 UTC (permalink / raw)
To: netdev; +Cc: dingtianhong, kargig, ppandit, yoshfuji
Because of the max_addresses check attackers were able to disable privacy
extensions on an interface by creating enough autoconfigured addresses:
<http://seclists.org/oss-sec/2012/q4/292>
But the check is not actually needed: max_addresses protects the
kernel to install too many ipv6 addresses on an interface and guards
addrconf_prefix_rcv to install further addresses as soon as this limit
is reached. We only generate temporary addresses in direct response of
a new address showing up. As soon as we filled up the maximum number of
addresses of an interface, we stop installing more addresses and thus
also stop generating more temp addresses.
Even if the attacker tries to generate a lot of temporary addresses
by announcing a prefix and removing it again (lifetime == 0) we won't
install more temp addresses, because the temporary addresses do count
to the maximum number of addresses, thus we would stop installing new
autoconfigured addresses when the limit is reached.
This patch fixes CVE-2013-0343 (but other layer-2 attacks are still
possible).
Thanks to Ding Tianhong to bring this topic up again.
Cc: Ding Tianhong <dingtianhong@huawei.com>
Cc: George Kargiotakis <kargig@void.gr>
Cc: P J P <ppandit@redhat.com>
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/ipv6/addrconf.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index ad12f7c..6926b56 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1131,12 +1131,10 @@ retry:
if (ifp->flags & IFA_F_OPTIMISTIC)
addr_flags |= IFA_F_OPTIMISTIC;
- ift = !max_addresses ||
- ipv6_count_addresses(idev) < max_addresses ?
- ipv6_add_addr(idev, &addr, NULL, tmp_plen,
- ipv6_addr_scope(&addr), addr_flags,
- tmp_valid_lft, tmp_prefered_lft) : NULL;
- if (IS_ERR_OR_NULL(ift)) {
+ ift = ipv6_add_addr(idev, &addr, NULL, tmp_plen,
+ ipv6_addr_scope(&addr), addr_flags,
+ tmp_valid_lft, tmp_prefered_lft);
+ if (IS_ERR(ift)) {
in6_ifa_put(ifp);
in6_dev_put(idev);
pr_info("%s: retry temporary address regeneration\n", __func__);
--
1.8.3.1
^ permalink raw reply related
* MASQUERADE/SNAT and multiple interfaces with the same IP
From: Kristian Evensen @ 2013-08-16 10:55 UTC (permalink / raw)
To: netfilter, netdev
Hello,
I am currently experimenting with load-balancing traffic between
multiple tunnels. I have two ip-in-ip tunnels between a router and a
gateway, each tunnel given the same IP in order to simplify address
distribution. In order to route traffic through different tunnels, I
use policy based routing. MASQUERADE/SNAT is used to NAT the packets
coming from the network behind the router.
As long as each flow is sent through the same tunnel, everything works
as expected. However, when I move a flow from one tunnel to another
(for example when a link goes down), there is a difference in behavior
between MASQUERADE and SNAT that I haven't been able to figure out.
When MASQUERADE is used, the NAT mapping is destroyed, one packet is
dropped and then a new mapping is created. With SNAT, this does not
happen and the same mapping is used. The reason keeping the same
mapping on the tunneled packets is important, is to avoid confusing
the remote peer.
After spending long time looking at the source code, I can't figure
out why this happens. Once the MASQUERADE/SNAT rule has been inserted,
to me everything looks the same. One theory I had was that since
MASQUERADE rules are "bound" to an interface, moving the flow to
another interface would cause a new rule to be created and the old one
to eventually time out. However, I always see the DESTROY-message from
conntrack before NEW. I tried tracing the origin of the
DESTROY-message and it seems to be generated by death_by_timeout(). I
have a suspicion that the change of links is detected in early_drop(),
but I have not been able to figure out why.
Does anyone have some hints on where to keep looking, or know the cause?
Thanks in advance for any help,
Kristian
^ permalink raw reply
* Re: [PATCH 6/6] vhost_net: remove the max pending check
From: Michael S. Tsirkin @ 2013-08-16 10:02 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1376630190-5912-7-git-send-email-jasowang@redhat.com>
On Fri, Aug 16, 2013 at 01:16:30PM +0800, Jason Wang wrote:
> We used to limit the max pending DMAs to prevent guest from pinning too many
> pages. But this could be removed since:
>
> - We have the sk_wmem_alloc check in both tun/macvtap to do the same work
> - This max pending check were almost useless since it was one done when there's
> no new buffers coming from guest. Guest can easily exceeds the limitation.
> - We've already check upend_idx != done_idx and switch to non zerocopy then. So
> even if all vq->heads were used, we can still does the packet transmission.
We can but performance will suffer.
>
> So remove this check completely.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/vhost/net.c | 13 -------------
> 1 files changed, 0 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index a035a89..ed3f165 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -38,8 +38,6 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
> * Using this limit prevents one virtqueue from starving others. */
> #define VHOST_NET_WEIGHT 0x80000
>
> -/* MAX number of TX used buffers for outstanding zerocopy */
> -#define VHOST_MAX_PEND 128
> #define VHOST_GOODCOPY_LEN 256
>
> /*
> @@ -372,17 +370,6 @@ static void handle_tx(struct vhost_net *net)
> break;
> /* Nothing new? Wait for eventfd to tell us they refilled. */
> if (head == vq->num) {
> - int num_pends;
> -
> - /* If more outstanding DMAs, queue the work.
> - * Handle upend_idx wrap around
> - */
> - num_pends = likely(nvq->upend_idx >= nvq->done_idx) ?
> - (nvq->upend_idx - nvq->done_idx) :
> - (nvq->upend_idx + UIO_MAXIOV -
> - nvq->done_idx);
> - if (unlikely(num_pends > VHOST_MAX_PEND))
> - break;
> if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> vhost_disable_notify(&net->dev, vq);
> continue;
> --
> 1.7.1
^ permalink raw reply
* Re: [PATCH 5/6] vhost_net: poll vhost queue after marking DMA is done
From: Michael S. Tsirkin @ 2013-08-16 10:00 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1376630190-5912-6-git-send-email-jasowang@redhat.com>
On Fri, Aug 16, 2013 at 01:16:29PM +0800, Jason Wang wrote:
> We used to poll vhost queue before making DMA is done, this is racy if vhost
> thread were waked up before marking DMA is done which can result the signal to
> be missed. Fix this by always poll the vhost thread before DMA is done.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Indeed, but vhost_net_ubuf_put should be the last thing we do:
it can cause the device to go away and we'll get
a user after free.
> ---
> drivers/vhost/net.c | 9 +++++----
> 1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 70cab75..a035a89 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -308,6 +308,11 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
> struct vhost_virtqueue *vq = ubufs->vq;
> int cnt = atomic_read(&ubufs->kref.refcount);
>
> + /* set len to mark this desc buffers done DMA */
> + vq->heads[ubuf->desc].len = success ?
> + VHOST_DMA_DONE_LEN : VHOST_DMA_FAILED_LEN;
> + vhost_net_ubuf_put(ubufs);
> +
> /*
> * Trigger polling thread if guest stopped submitting new buffers:
> * in this case, the refcount after decrement will eventually reach 1
> @@ -318,10 +323,6 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
> */
> if (cnt <= 2 || !(cnt % 16))
> vhost_poll_queue(&vq->poll);
> - /* set len to mark this desc buffers done DMA */
> - vq->heads[ubuf->desc].len = success ?
> - VHOST_DMA_DONE_LEN : VHOST_DMA_FAILED_LEN;
> - vhost_net_ubuf_put(ubufs);
> }
>
> /* Expects to be always run from workqueue - which acts as
> --
> 1.7.1
^ permalink raw reply
* Re: [PATCH 3/6] vhost: switch to use vhost_add_used_n()
From: Michael S. Tsirkin @ 2013-08-16 9:56 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1376630190-5912-4-git-send-email-jasowang@redhat.com>
On Fri, Aug 16, 2013 at 01:16:27PM +0800, Jason Wang wrote:
> Let vhost_add_used() to use vhost_add_used_n() to reduce the code duplication.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Does compiler inline it then?
Reason I ask, last time I checked put_user inside vhost_add_used
was much cheaper than copy_to_user inside vhost_add_used_n,
so I wouldn't be surprised if this hurt performance.
Did you check?
> ---
> drivers/vhost/vhost.c | 43 ++-----------------------------------------
> 1 files changed, 2 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index e58cf00..c479452 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1332,48 +1332,9 @@ EXPORT_SYMBOL_GPL(vhost_discard_vq_desc);
> * want to notify the guest, using eventfd. */
> int vhost_add_used(struct vhost_virtqueue *vq, unsigned int head, int len)
> {
> - struct vring_used_elem __user *used;
> + struct vring_used_elem heads = { head, len };
>
> - /* The virtqueue contains a ring of used buffers. Get a pointer to the
> - * next entry in that used ring. */
> - used = &vq->used->ring[vq->last_used_idx % vq->num];
> - if (__put_user(head, &used->id)) {
> - vq_err(vq, "Failed to write used id");
> - return -EFAULT;
> - }
> - if (__put_user(len, &used->len)) {
> - vq_err(vq, "Failed to write used len");
> - return -EFAULT;
> - }
> - /* Make sure buffer is written before we update index. */
> - smp_wmb();
> - if (__put_user(vq->last_used_idx + 1, &vq->used->idx)) {
> - vq_err(vq, "Failed to increment used idx");
> - return -EFAULT;
> - }
> - if (unlikely(vq->log_used)) {
> - /* Make sure data is seen before log. */
> - smp_wmb();
> - /* Log used ring entry write. */
> - log_write(vq->log_base,
> - vq->log_addr +
> - ((void __user *)used - (void __user *)vq->used),
> - sizeof *used);
> - /* Log used index update. */
> - log_write(vq->log_base,
> - vq->log_addr + offsetof(struct vring_used, idx),
> - sizeof vq->used->idx);
> - if (vq->log_ctx)
> - eventfd_signal(vq->log_ctx, 1);
> - }
> - vq->last_used_idx++;
> - /* If the driver never bothers to signal in a very long while,
> - * used index might wrap around. If that happens, invalidate
> - * signalled_used index we stored. TODO: make sure driver
> - * signals at least once in 2^16 and remove this. */
> - if (unlikely(vq->last_used_idx == vq->signalled_used))
> - vq->signalled_used_valid = false;
> - return 0;
> + return vhost_add_used_n(vq, &heads, 1);
> }
> EXPORT_SYMBOL_GPL(vhost_add_used);
>
> --
> 1.7.1
^ permalink raw reply
* Re: [PATCH 2/6] vhost_net: use vhost_add_used_and_signal_n() in vhost_zerocopy_signal_used()
From: Michael S. Tsirkin @ 2013-08-16 9:54 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1376630190-5912-3-git-send-email-jasowang@redhat.com>
On Fri, Aug 16, 2013 at 01:16:26PM +0800, Jason Wang wrote:
> Switch to use vhost_add_used_and_signal_n() to avoid multiple calls to
> vhost_add_used_and_signal(). With the patch we will call at most 2 times
> (consider done_idx warp around) compared to N times w/o this patch.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
So? Does this help performance then?
> ---
> drivers/vhost/net.c | 13 ++++++++-----
> 1 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 280ee66..8a6dd0d 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -281,7 +281,7 @@ static void vhost_zerocopy_signal_used(struct vhost_net *net,
> {
> struct vhost_net_virtqueue *nvq =
> container_of(vq, struct vhost_net_virtqueue, vq);
> - int i;
> + int i, add;
> int j = 0;
>
> for (i = nvq->done_idx; i != nvq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
> @@ -289,14 +289,17 @@ static void vhost_zerocopy_signal_used(struct vhost_net *net,
> vhost_net_tx_err(net);
> if (VHOST_DMA_IS_DONE(vq->heads[i].len)) {
> vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
> - vhost_add_used_and_signal(vq->dev, vq,
> - vq->heads[i].id, 0);
> ++j;
> } else
> break;
> }
> - if (j)
> - nvq->done_idx = i;
> + while (j) {
> + add = min(UIO_MAXIOV - nvq->done_idx, j);
> + vhost_add_used_and_signal_n(vq->dev, vq,
> + &vq->heads[nvq->done_idx], add);
> + nvq->done_idx = (nvq->done_idx + add) % UIO_MAXIOV;
> + j -= add;
> + }
> }
>
> static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
> --
> 1.7.1
^ permalink raw reply
* Re: [PATCH] vhost: Drop linux/socket.h
From: Michael S. Tsirkin @ 2013-08-16 9:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev, kvm, virtualization
In-Reply-To: <20130816.003159.2033932881805410431.davem@davemloft.net>
On Fri, Aug 16, 2013 at 12:31:59AM -0700, David Miller wrote:
> From: Asias He <asias@redhat.com>
> Date: Fri, 16 Aug 2013 09:27:43 +0800
>
> > On Thu, Aug 15, 2013 at 02:07:40PM -0700, David Miller wrote:
> >> From: Asias He <asias@redhat.com>
> >> Date: Thu, 15 Aug 2013 11:20:16 +0800
> >>
> >> > memcpy_fromiovec is moved to lib/iovec.c. No need to include
> >> > linux/socket.h for it.
> >> >
> >> > Signed-off-by: Asias He <asias@redhat.com>
> >>
> >> You can't do this.
> >>
> >> Because this file doesn't include the header file that
> >> provides the declaration, which is linux/uio.h
> >
> > vhost.c includes drivers/vhost/vhost.h. In drivers/vhost/vhost.h, we
> > have linux/uio.h included.
>
> Nothing in vhost.h needs linux/uio.h right? That's very poor style,
> include the header where the dependency exists which is vhost.c
It needs struct iovec, which is in include/uapi/linux/uio.h.
Do you think it's better to include uapi/linux/uio.h directly?
In that case maybe we should update linux/socket.h
to pull in uapi/linux/uio.h directly too.
Or even just forward-declare struct iovec.
I tried this last option and the only fall-out seems to be in security/
- though I didn't do an allyes config build yet. Good idea? If yes let
me know and I'll do that.
-->
socket: forward-declare struct iovec
We can simplify header dependencies by using a forward declaration
of struct iovec in socket.h - it's always used through a pointer there.
Do this and fix up users that rely on socket.h including uio.h.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 230c04b..3ad6f52 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -4,7 +4,6 @@
#include <asm/socket.h> /* arch-dependent defines */
#include <linux/sockios.h> /* the SIOCxxx I/O controls */
-#include <linux/uio.h> /* iovec support */
#include <linux/types.h> /* pid_t */
#include <linux/compiler.h> /* __user */
#include <uapi/linux/socket.h>
@@ -44,6 +43,8 @@ struct linger {
* belong in an obscure libc emulation or the bin.
*/
+struct iovec;
+
struct msghdr {
void * msg_name; /* Socket name */
int msg_namelen; /* Length of name */
diff --git a/security/keys/compat.c b/security/keys/compat.c
index d65fa7f..9c2723f 100644
--- a/security/keys/compat.c
+++ b/security/keys/compat.c
@@ -13,6 +13,7 @@
#include <linux/keyctl.h>
#include <linux/compat.h>
#include <linux/slab.h>
+#include <uapi/linux/uio.h>
#include "internal.h"
/*
--
MST
^ permalink raw reply related
* Re: [PATCH] driver:net:stmmac: Disable DMA store and forward mode if platform data force_sf_dma_mode is negative.
From: Sonic Zhang @ 2013-08-16 9:37 UTC (permalink / raw)
To: David Miller; +Cc: peppe.cavallaro, netdev, adi-buildroot-devel, Zhang, Sonic
In-Reply-To: <20130815.142005.816948927496553829.davem@davemloft.net>
Hi David,
On Fri, Aug 16, 2013 at 5:20 AM, David Miller <davem@davemloft.net> wrote:
> From: Sonic Zhang <sonic.adi@gmail.com>
> Date: Thu, 15 Aug 2013 15:37:36 +0800
>
>> @@ -1157,7 +1157,9 @@ static void free_dma_desc_resources(struct stmmac_priv *priv)
>> */
>> static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
>> {
>> - if (likely(priv->plat->force_sf_dma_mode ||
>> + if (priv->plat->force_sf_dma_mode < 0)
>> + priv->hw->dma->dma_mode(priv->ioaddr, tc, tc);
>> + else if (likely(priv->plat->force_sf_dma_mode > 0 ||
>> ((priv->plat->tx_coe) && (!priv->no_csum_insertion)))) {
>
> You need to properly re-indent the last line here so that
> the openning parenthesis lines up with the first column
> after the openning parenthesis on the "else if" line.
The last line is the original source code in master branch of the
Linus's kernel git tree. Do you mean I should fix the ident issue in
this patch as well?
Regards,
Sonic
^ permalink raw reply
* Re: [PATCHv2 net-next] xfrm: Make xfrm_state timer monotonic
From: Steffen Klassert @ 2013-08-16 9:30 UTC (permalink / raw)
To: David Miller; +Cc: fan.du, netdev
In-Reply-To: <20130815.142334.1736006937556027032.davem@davemloft.net>
On Thu, Aug 15, 2013 at 02:23:34PM -0700, David Miller wrote:
> From: Fan Du <fan.du@windriver.com>
> Date: Thu, 15 Aug 2013 15:49:06 +0800
>
> > xfrm_state timer should be independent of system clock change,
> > so switch to CLOCK_BOOTTIME base which is not only monotonic but
> > also counting suspend time.
> >
> > Thus issue reported in commit: 9e0d57fd6dad37d72a3ca6db00ca8c76f2215454
> > ("xfrm: SAD entries do not expire correctly after suspend-resume")
> > could ALSO be avoided.
> >
> > Signed-off-by: Fan Du <fan.du@windriver.com>
> >
> > v2: Use CLOCK_BOOTTIME to count suspend time, but still monotonic.
>
> This seems like a good fix:
>
> Acked-by: David S. Miller <davem@davemloft.net>
Applied to ipsec-next, thanks!
^ permalink raw reply
* Re: [PATCH] vhost: Drop linux/socket.h
From: Asias He @ 2013-08-16 9:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev, virtualization, kvm, mst
In-Reply-To: <20130816.003159.2033932881805410431.davem@davemloft.net>
On Fri, Aug 16, 2013 at 12:31:59AM -0700, David Miller wrote:
> From: Asias He <asias@redhat.com>
> Date: Fri, 16 Aug 2013 09:27:43 +0800
>
> > On Thu, Aug 15, 2013 at 02:07:40PM -0700, David Miller wrote:
> >> From: Asias He <asias@redhat.com>
> >> Date: Thu, 15 Aug 2013 11:20:16 +0800
> >>
> >> > memcpy_fromiovec is moved to lib/iovec.c. No need to include
> >> > linux/socket.h for it.
> >> >
> >> > Signed-off-by: Asias He <asias@redhat.com>
> >>
> >> You can't do this.
> >>
> >> Because this file doesn't include the header file that
> >> provides the declaration, which is linux/uio.h
> >
> > vhost.c includes drivers/vhost/vhost.h. In drivers/vhost/vhost.h, we
> > have linux/uio.h included.
>
> Nothing in vhost.h needs linux/uio.h right? That's very poor style,
> include the header where the dependency exists which is vhost.c
We use 'struct iovec' in vhost.h which needs linux/uio.h, no?
So, how about including linux/uio.h in both vhost.c and vhost.h.
--
Asias
^ permalink raw reply
* Re: Quick Blind TCP Connection Spoofing with SYN Cookies
From: Florian Westphal @ 2013-08-16 9:21 UTC (permalink / raw)
To: Jiri Bohac; +Cc: Jakob Lell, netdev, davem
In-Reply-To: <20130815235743.GA25665@midget.suse.cz>
Jiri Bohac <jbohac@suse.cz> wrote:
> > The simplification of TCP Connection Spoofing described here is an
> > inherent problem of TCP SYN Cookies and so there won't be a simple
> > patch which just solves the issue and makes the Spoofing Attack as
> > hard as it is without SYN Cookies. It is only possible to gradually
> > increase the required effort for successfully spoofing a connection
> > e.g. by only accepting the last two instead of four counter values
> > (which will lead to a 60-120s
>
> If the counter is slowed down 4 times, accepting only two
> values should result in similar behaviour as we have today.
>
> Can anyone think of a reason this should not be done?
I was also working on patches that mitigate this (not ready yet),
lets compare notes.
There are two problems.
1) current scheme is dangerous with HZ=1000 on 32
bit platforms due to jiffies overflow, it needs to be fixed.
Else, cookies can be re-used exactly after 49 day period.
I did straighforward replacement first to not change current timer:
Use getnstimeofday(), take "second" value, shift result by 6 (64-second
granular timer).
If you have time to work on it, I would appreciate if you could take
care of this.
> Additionally, I believe we should reduce the number of possible MSS
> values. I think 3 values should be enough - not supporting jumbo
> frames and wasting a few bytes on sub-optimal MSS around 1400
> bytes should be acceptable when a system is under a DoS attack.
Agreed. I had a (untested patch) that just kicks out the unlikely
values. I used 536, 1200, 1440, 1460 for ipv4, and 536, 1220, 1460,
8940 for ipv6. I think this would be sufficient to keep decent
connectivity for legitimate clients.
We could add an alternate fallback table with more exotic values
and use a bit in the tcp timestamp to indicate use of fallback table
however since that only workswhen ts are used I would not do this unless
there is evidence that this is useful.
As for the cookie lifetime: I agree it should be reduced, allowing
delta for more than one minute seems way too long given that we never
retransmit lost syn/ack.
A conservative change would be to reject delta <= 2, with 64 second
timer.
Cheers,
Florian
^ permalink raw reply
* Re: [PATCH v2 1/3] [RFC] TCP syncookies: slow down timer to mitigate spoofing attacks
From: Jiri Bohac @ 2013-08-16 8:20 UTC (permalink / raw)
To: Neal Cardwell; +Cc: Jakob Lell, Netdev, David Miller
In-Reply-To: <CADVnQykQLHBgDLwMd63b5dwnFUbikeQ_-Jp9mjDTNQObS0Z7=Q@mail.gmail.com>
On Thu, Aug 15, 2013 at 08:34:09PM -0400, Neal Cardwell wrote:
> On Thu, Aug 15, 2013 at 8:00 PM, Jiri Bohac <jbohac@suse.cz> wrote:
> > /*
> > - * This (misnamed) value is the age of syncookie which is permitted.
> > + * This value is the age (in seconds) of syncookies which will always be
>
> I believe (hope?) you mean minutes here, rather than seconds. :-) Same
> typo occurs in 2 spots each for IPv4 and IPv6.
Oh, of course, thanks for noticing! So let's change the constant
and its use to actually be in seconds - fixed patch below:
(compile-tested only)
Jakob Lell discovered that the sequence number that needs to be guessed to
successfully spoof a TCP connection with syncookies only has 27 bits of
entropy. Of the 32 bits, 2 bits are wasted by the four differrent timestamps
accepted and 3 are wasted by the 8 differrent RSS values. [1]
This patch slows down the timer used in syncookies from 1/60 Hz to 1/60/4 Hz
so that at any moment only two differrent timer values can be accepted.
As a result, 1 bit of sequence number entropy is gained.
This changes the maximum cookie age limit from 4 - 5 minutes to 4 - 8 minutes.
[1]: http://www.jakoblell.com/blog/2013/08/13/quick-blind-tcp-connection-spoofing-with-syn-cookies/
Signed-off-by: Jiri Bohac <jbohac@suse.cz>
---
net/ipv4/syncookies.c | 30 ++++++++++++++++--------------
net/ipv6/syncookies.c | 15 ++++++++-------
2 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index b05c96e..cf1b720 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -114,13 +114,13 @@ static __u32 secure_tcp_syn_cookie(__be32 saddr, __be32 daddr, __be16 sport,
* If the syncookie is bad, the data returned will be out of
* range. This must be checked by the caller.
*
- * The count value used to generate the cookie must be within
- * "maxdiff" if the current (passed-in) "count". The return value
+ * The count value used to generate the cookie must be the same or
+ * one less than the current (passed-in) "count". The return value
* is (__u32)-1 if this test fails.
*/
static __u32 check_tcp_syn_cookie(__u32 cookie, __be32 saddr, __be32 daddr,
__be16 sport, __be16 dport, __u32 sseq,
- __u32 count, __u32 maxdiff)
+ __u32 count)
{
__u32 diff;
@@ -129,7 +129,7 @@ static __u32 check_tcp_syn_cookie(__u32 cookie, __be32 saddr, __be32 daddr,
/* Cookie is now reduced to (count * 2^24) ^ (hash % 2^24) */
diff = (count - (cookie >> COOKIEBITS)) & ((__u32) - 1 >> COOKIEBITS);
- if (diff >= maxdiff)
+ if (diff >= 2)
return (__u32)-1;
return (cookie -
@@ -157,6 +157,16 @@ static __u16 const msstab[] = {
};
/*
+ * This value is the age (in seconds) of syncookies which will always be
+ * permitted. Cookies aged up to twice this value may be permitted as
+ * a result of rounding errors.
+ * Its ideal value should be dependent on TCP_TIMEOUT_INIT and
+ * sysctl_tcp_retries1. It's a rather complicated formula (exponential
+ * backoff) to compute at runtime so it's currently hardcoded here.
+ */
+#define COOKIE_LIFETIME (4 * 60) /* 4 to 8 minutes */
+
+/*
* Generate a syncookie. mssp points to the mss, which is returned
* rounded down to the value encoded in the cookie.
*/
@@ -178,17 +188,10 @@ __u32 cookie_v4_init_sequence(struct sock *sk, struct sk_buff *skb, __u16 *mssp)
return secure_tcp_syn_cookie(iph->saddr, iph->daddr,
th->source, th->dest, ntohl(th->seq),
- jiffies / (HZ * 60), mssind);
+ jiffies / (HZ * COOKIE_LIFETIME), mssind);
}
/*
- * This (misnamed) value is the age of syncookie which is permitted.
- * Its ideal value should be dependent on TCP_TIMEOUT_INIT and
- * sysctl_tcp_retries1. It's a rather complicated formula (exponential
- * backoff) to compute at runtime so it's currently hardcoded here.
- */
-#define COUNTER_TRIES 4
-/*
* Check if a ack sequence number is a valid syncookie.
* Return the decoded mss if it is, or 0 if not.
*/
@@ -199,8 +202,7 @@ static inline int cookie_check(struct sk_buff *skb, __u32 cookie)
__u32 seq = ntohl(th->seq) - 1;
__u32 mssind = check_tcp_syn_cookie(cookie, iph->saddr, iph->daddr,
th->source, th->dest, seq,
- jiffies / (HZ * 60),
- COUNTER_TRIES);
+ jiffies / (HZ * COOKIE_LIFETIME));
return mssind < ARRAY_SIZE(msstab) ? msstab[mssind] : 0;
}
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index d5dda20..46e8b27 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -37,12 +37,14 @@ static __u16 const msstab[] = {
};
/*
- * This (misnamed) value is the age of syncookie which is permitted.
+ * This value is the age (in seconds) of syncookies which will always be
+ * permitted. Cookies aged up to twice this value may be permitted as
+ * a result of rounding errors.
* Its ideal value should be dependent on TCP_TIMEOUT_INIT and
* sysctl_tcp_retries1. It's a rather complicated formula (exponential
* backoff) to compute at runtime so it's currently hardcoded here.
*/
-#define COUNTER_TRIES 4
+#define COOKIE_LIFETIME (4 * 60) /* 4 to 8 minutes */
static inline struct sock *get_cookie_sock(struct sock *sk, struct sk_buff *skb,
struct request_sock *req,
@@ -96,15 +98,14 @@ static __u32 secure_tcp_syn_cookie(const struct in6_addr *saddr,
static __u32 check_tcp_syn_cookie(__u32 cookie, const struct in6_addr *saddr,
const struct in6_addr *daddr, __be16 sport,
- __be16 dport, __u32 sseq, __u32 count,
- __u32 maxdiff)
+ __be16 dport, __u32 sseq, __u32 count)
{
__u32 diff;
cookie -= cookie_hash(saddr, daddr, sport, dport, 0, 0) + sseq;
diff = (count - (cookie >> COOKIEBITS)) & ((__u32) -1 >> COOKIEBITS);
- if (diff >= maxdiff)
+ if (diff >= 2)
return (__u32)-1;
return (cookie -
@@ -131,7 +132,7 @@ __u32 cookie_v6_init_sequence(struct sock *sk, const struct sk_buff *skb, __u16
return secure_tcp_syn_cookie(&iph->saddr, &iph->daddr, th->source,
th->dest, ntohl(th->seq),
- jiffies / (HZ * 60), mssind);
+ jiffies / (HZ * COOKIE_LIFETIME), mssind);
}
static inline int cookie_check(const struct sk_buff *skb, __u32 cookie)
@@ -141,7 +142,7 @@ static inline int cookie_check(const struct sk_buff *skb, __u32 cookie)
__u32 seq = ntohl(th->seq) - 1;
__u32 mssind = check_tcp_syn_cookie(cookie, &iph->saddr, &iph->daddr,
th->source, th->dest, seq,
- jiffies / (HZ * 60), COUNTER_TRIES);
+ jiffies / (HZ * COOKIE_LIFETIME));
return mssind < ARRAY_SIZE(msstab) ? msstab[mssind] : 0;
}
--
1.8.3.1
--
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ
^ permalink raw reply related
* [PATCH net-next 6/7] r8152: adjust tx_bottom function
From: Hayes Wang @ 2013-08-16 8:09 UTC (permalink / raw)
To: romieu, netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1376640578-4258-1-git-send-email-hayeswang@realtek.com>
Split some parts of code into another function to simplify
tx_bottom(). Use while loop to replace the goto loop.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 134 ++++++++++++++++++++++++------------------------
1 file changed, 68 insertions(+), 66 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0a88f64..825edfe 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1131,6 +1131,51 @@ r8152_tx_csum(struct r8152 *tp, struct tx_desc *desc, struct sk_buff *skb)
}
}
+static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
+{
+ u32 remain;
+ u8 *tx_data;
+
+ tx_data = agg->head;
+ agg->skb_num = agg->skb_len = 0;
+ remain = rx_buf_sz - sizeof(struct tx_desc);
+
+ while (remain >= ETH_ZLEN) {
+ struct tx_desc *tx_desc;
+ struct sk_buff *skb;
+ unsigned int len;
+
+ skb = skb_dequeue(&tp->tx_queue);
+ if (!skb)
+ break;
+
+ len = skb->len;
+ if (remain < len) {
+ skb_queue_head(&tp->tx_queue, skb);
+ break;
+ }
+
+ tx_desc = (struct tx_desc *)tx_data;
+ tx_data += sizeof(*tx_desc);
+
+ r8152_tx_csum(tp, tx_desc, skb);
+ memcpy(tx_data, skb->data, len);
+ agg->skb_num++;
+ agg->skb_len += len;
+ dev_kfree_skb_any(skb);
+
+ tx_data = tx_agg_align(tx_data + len);
+ remain = rx_buf_sz - sizeof(*tx_desc) -
+ (u32)((void *)tx_data - agg->head);
+ }
+
+ usb_fill_bulk_urb(agg->urb, tp->udev, usb_sndbulkpipe(tp->udev, 2),
+ agg->head, (int)(tx_data - (u8 *)agg->head),
+ (usb_complete_t)write_bulk_callback, agg);
+
+ return usb_submit_urb(agg->urb, GFP_ATOMIC);
+}
+
static void rx_bottom(struct r8152 *tp)
{
unsigned long flags;
@@ -1204,82 +1249,39 @@ submit:
static void tx_bottom(struct r8152 *tp)
{
- struct net_device_stats *stats;
- struct net_device *netdev;
- struct tx_agg *agg;
- unsigned long flags;
- u32 remain, total;
- u8 *tx_data;
int res;
- netdev = tp->netdev;
-
-next_agg:
- agg = NULL;
- if (skb_queue_empty(&tp->tx_queue))
- return;
+ do {
+ struct tx_agg *agg;
- agg = r8152_get_tx_agg(tp);
- if (!agg)
- return;
-
- tx_data = agg->head;
- agg->skb_num = agg->skb_len = 0;
- remain = rx_buf_sz - sizeof(struct tx_desc);
- total = 0;
-
- while (remain >= ETH_ZLEN) {
- struct tx_desc *tx_desc;
- struct sk_buff *skb;
- unsigned int len;
-
- skb = skb_dequeue(&tp->tx_queue);
- if (!skb)
+ if (skb_queue_empty(&tp->tx_queue))
break;
- len = skb->len;
- if (remain < len) {
- skb_queue_head(&tp->tx_queue, skb);
+ agg = r8152_get_tx_agg(tp);
+ if (!agg)
break;
- }
-
- tx_data = tx_agg_align(tx_data);
- tx_desc = (struct tx_desc *)tx_data;
- tx_data += sizeof(*tx_desc);
- r8152_tx_csum(tp, tx_desc, skb);
- memcpy(tx_data, skb->data, len);
- agg->skb_num++;
- agg->skb_len += len;
- dev_kfree_skb_any(skb);
-
- tx_data += len;
- remain = rx_buf_sz - sizeof(*tx_desc) -
- (u32)(tx_agg_align(tx_data) - agg->head);
- }
-
- usb_fill_bulk_urb(agg->urb, tp->udev, usb_sndbulkpipe(tp->udev, 2),
- agg->head, (int)(tx_data - (u8 *)agg->head),
- (usb_complete_t)write_bulk_callback, agg);
- res = usb_submit_urb(agg->urb, GFP_ATOMIC);
+ res = r8152_tx_agg_fill(tp, agg);
+ if (res) {
+ struct net_device_stats *stats;
+ struct net_device *netdev;
+ unsigned long flags;
- stats = rtl8152_get_stats(netdev);
+ netdev = tp->netdev;
+ stats = rtl8152_get_stats(netdev);
- if (res) {
- /* Can we get/handle EPIPE here? */
- if (res == -ENODEV) {
- netif_device_detach(netdev);
- } else {
- netif_warn(tp, tx_err, netdev,
- "failed tx_urb %d\n", res);
- stats->tx_dropped += agg->skb_num;
- spin_lock_irqsave(&tp->tx_lock, flags);
- list_add_tail(&agg->list, &tp->tx_free);
- spin_unlock_irqrestore(&tp->tx_lock, flags);
+ if (res == -ENODEV) {
+ netif_device_detach(netdev);
+ } else {
+ netif_warn(tp, tx_err, netdev,
+ "failed tx_urb %d\n", res);
+ stats->tx_dropped += agg->skb_num;
+ spin_lock_irqsave(&tp->tx_lock, flags);
+ list_add_tail(&agg->list, &tp->tx_free);
+ spin_unlock_irqrestore(&tp->tx_lock, flags);
+ }
}
- return;
- }
- goto next_agg;
+ } while (res == 0);
}
static void bottom_half(unsigned long data)
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 7/7] r8152: add comments
From: Hayes Wang @ 2013-08-16 8:09 UTC (permalink / raw)
To: romieu, netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1376640578-4258-1-git-send-email-hayeswang@realtek.com>
Add comments.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 825edfe..f3fce41 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -790,6 +790,9 @@ static void read_bulk_callback(struct urb *urb)
return;
netdev = tp->netdev;
+
+ /* When link down, the driver would cancel all bulks. */
+ /* This avoid the re-submitting bulk */
if (!netif_carrier_ok(netdev))
return;
@@ -1296,6 +1299,8 @@ static void bottom_half(unsigned long data)
if (!test_bit(WORK_ENABLE, &tp->flags))
return;
+ /* When link down, the driver would cancel all bulks. */
+ /* This avoid the re-submitting bulk */
if (!netif_carrier_ok(tp->netdev))
return;
--
1.8.3.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox