* Re: [PATCH] net: factorize rt_do_flush for batch device unregistering
From: Eric Dumazet @ 2009-11-16 22:15 UTC (permalink / raw)
To: Octavian Purdila; +Cc: netdev
In-Reply-To: <200911170003.44099.opurdila@ixiacom.com>
Octavian Purdila a écrit :
> On Monday 16 November 2009 23:32:55 you wrote:
>
>>> @@ -5374,6 +5395,9 @@ EXPORT_SYMBOL(unregister_netdevice_queue);
>>> * unregister_netdevice_many - unregister many devices
>>> * @head: list of devices
>>> *
>>> + * WARNING: This function modifies the list. It may change the order of
>>> the + * elements in the list. However, you can assume it does not add or
>>> delete + * elements to/from the list.
>> Sorry I dont understand this comment
>>
>
> The list passed to unregister_netdevice_many(), as the "head" parameter, may
> be altered, e.g. order may change between the elements.
>
> That is because we temporarily move the items from the list to the
> rt_flush_list for the flush. When we add the items back they may not be added in
> the same place.
>
Ah, I got it now, confusion is that comment makes more sense for
rollback_registered_many() because when reading unregister_netdevice_many()
it is clear it doesnt change the list...
void unregister_netdevice_many(struct list_head *head)
{
struct net_device *dev;
if (!list_empty(head)) {
rollback_registered_many(head);
list_for_each_entry(dev, head, unreg_list)
net_set_todo(dev);
}
}
^ permalink raw reply
* Re: [net-next-2.6 PATCH v6 4/7 RFC] TCPCT part 1d: define TCP cookie option, extend existing struct's
From: William Allen Simpson @ 2009-11-16 22:09 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Kernel Network Developers, Joe Perches
In-Reply-To: <4B01BF65.1010203@gmail.com>
Eric Dumazet wrote:
> I am a bit uneasy on this patch, since apparently you have infrastructure
> to send DATA payload on SYN, but I thought it was an optional part of your 'RFC'
> and as such, being implemented later ?
>
There is nothing yet in this patch series to send data with a SYN. Back in
early October, David required that the various s_data and cookie structures
be compressed and consolidated. So, for the client side, the cookie_*
fields are filled and the s_data_* fields are zero (ignored), while the
server side can have both filled.
Moreover, *this* patch does nothing other than allocate and deallocate the
structure, zero filled by kzalloc().
SYN data will be implemented (much) later.
> I remember a previous remark from David that our skb queues would not contain
> DATA on SYN packets...
>
I haven't seen anything by David, but there's an existing comment in
tcp_input.c at the place where SYN data will be added later:
/* Now we have several options: In theory there is
* nothing else in the frame. KA9Q has an option to
* send data with the syn, BSD accepts data with the
* syn up to the [to be] advertised window and
* Solaris 2.1 gives you a protocol error. For now
* we just ignore it, that fits the spec precisely
* and avoids incompatibilities. It would be nice in
* future to drop through and process the data.
As you know, there's an old Linux t/tcp patch series out there to add SYN
data. I'm also working with my KA9Q and BSD sources.
^ permalink raw reply
* Re: [PATCH] net: factorize rt_do_flush for batch device unregistering
From: Octavian Purdila @ 2009-11-16 22:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <4B01C507.9050908@gmail.com>
On Monday 16 November 2009 23:32:55 you wrote:
> > @@ -5374,6 +5395,9 @@ EXPORT_SYMBOL(unregister_netdevice_queue);
> > * unregister_netdevice_many - unregister many devices
> > * @head: list of devices
> > *
> > + * WARNING: This function modifies the list. It may change the order of
> > the + * elements in the list. However, you can assume it does not add or
> > delete + * elements to/from the list.
>
> Sorry I dont understand this comment
>
The list passed to unregister_netdevice_many(), as the "head" parameter, may
be altered, e.g. order may change between the elements.
That is because we temporarily move the items from the list to the
rt_flush_list for the flush. When we add the items back they may not be added in
the same place.
Perhaps the confusion comes from the fact that I did not specified which list?
(i.e. head)
> > @@ -937,7 +937,10 @@ static int fib_netdev_event(struct notifier_block
> > *this, unsigned long event, vo struct in_device *in_dev =
> > __in_dev_get_rtnl(dev);
> >
> > if (event == NETDEV_UNREGISTER) {
> > - fib_disable_ip(dev, 2);
> > + /* if this event is part of a batch then don't flush the cache
> > + * now; we will receive another event at the end of the batch */
> > + int rt_flush = list_empty(&dev->unreg_list) ? 0 : -1;
>
> hmm... a bit ugly...
>
Would it be better if I would add a dev_is_batch_unregister() instead?
Or add a new device flag to explicitly signal the batch unregister?
> > + fib_disable_ip(dev, 2, rt_flush);
> > return NOTIFY_DONE;
> > }
> >
> > @@ -955,7 +958,7 @@ static int fib_netdev_event(struct notifier_block
> > *this, unsigned long event, vo rt_cache_flush(dev_net(dev), -1);
> > break;
> > case NETDEV_DOWN:
> > - fib_disable_ip(dev, 0);
> > + fib_disable_ip(dev, 0, 0);
> > break;
> > case NETDEV_CHANGEMTU:
> > case NETDEV_CHANGE:
>
> Are you sure you want to overload NETDEV_UNREGISTER ?
>
> Maybe it would be cleaner to add a new value, NETDEV_UNREGISTER_PERNET or
> something for the final loop...
>
Hmm, I think that will allow us to get rid of the ugly test: never flush the
cache for NETDEV_UNREGISTER, only flush it for NETDEV_UNREGISTER_PERNET.
We just need to make sure to add NETDEV_UNREGISTER_PERNET in other places
where NETDEV_UNREGISTER is called.
I'll try this in the next patch. Thanks for reviewing.
^ permalink raw reply
* netlink: remove subscriptions check on notifier
From: Johannes Berg @ 2009-11-16 22:05 UTC (permalink / raw)
To: netdev; +Cc: Patrick McHardy
The netlink URELEASE notifier doesn't notify for
sockets that have been used to receive multicast
but it should be called for such sockets as well
since they might _also_ be used for sending and
not solely for receiving multicast. We will need
that for nl80211 (generic netlink sockets) in the
future.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: Patrick McHardy <kaber@trash.net>
---
net/netlink/af_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- wireless-testing.orig/net/netlink/af_netlink.c 2009-11-16 23:02:13.000000000 +0100
+++ wireless-testing/net/netlink/af_netlink.c 2009-11-16 23:02:23.000000000 +0100
@@ -497,7 +497,7 @@ static int netlink_release(struct socket
skb_queue_purge(&sk->sk_write_queue);
- if (nlk->pid && !nlk->subscriptions) {
+ if (nlk->pid) {
struct netlink_notify n = {
.net = sock_net(sk),
.protocol = sk->sk_protocol,
^ permalink raw reply
* Re: [PATCH net-2.6 V2] tg3: Fix phylib locking strategy
From: Felix Radensky @ 2009-11-16 20:53 UTC (permalink / raw)
To: Matt Carlson; +Cc: netdev, Michael Chan, andy
In-Reply-To: <1254801692.18507@xw6200>
Hi, Matt
Matt Carlson wrote:
> O.K. Here is the latest version. Felix, can you verify your problem
> is solved with this patch?
>
> ---
>
> Felix Radensky noted that chip resets were generating stack trace dumps.
> This is because the driver is attempting to acquire the mdio bus mutex
> while holding the tp->lock spinlock. The fix is to change the code such
> that every phy access takes the tp->lock spinlock instead.
>
>
Sorry for a very long delay, I didn't have access to hardware until today.
Your patch fixes initial problem I've reported, but I have a different
problem now, maybe it's related. It is 100% reproducible by the following
sequence
modprobe tg3
ifconfig eth2 up
ifconfig eth2 down
ifconfig eth2 up
As a result, link is not recognized after third invocation of ifconfig,
although network cable is plugged in. Below are relevant kernel messages.
Reloading tg3 driver fixes the problem.
tg3.c:v3.102 (September 1, 2009)
tg3 0002:05:00.0: PME# disabled
tg3 mdio bus: probed
eth2: Tigon3 [partno(BCM57760) rev 57780001] (PCI Express) MAC address
00:10:18:00:00:00
eth2: attached PHY driver [Broadcom BCM57780] (mii_bus:phy_addr=500:01)
eth2: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
eth2: dma_rwctrl[76180000] dma_mask[64-bit]
tg3 0002:05:00.0: PME# disabled
tg3: eth2: Link is down.
tg3: eth2: Link is up at 1000 Mbps, full duplex.
tg3: eth2: Flow control is on for TX and on for RX.
tg3: eth2: Link is down.
tg3 0002:05:00.0: PME# disabled
tg3: eth2: Link is down.
Thanks.
Felix.
^ permalink raw reply
* PATCH net-next-2.6] linkwatch: linkwatch_forget_dev() to speedup device dismantle
From: Eric Dumazet @ 2009-11-16 21:50 UTC (permalink / raw)
To: David S. Miller, Herbert Xu, Stephen Hemminger; +Cc: Linux Netdev List
In-Reply-To: <4B019381.2010509@gmail.com>
time ip link del eth3.103 ; time ip link del eth3.104 ; time ip link del eth3.105
real 0m0.266s
user 0m0.000s
sys 0m0.001s
real 0m0.770s
user 0m0.000s
sys 0m0.000s
real 0m1.022s
user 0m0.000s
sys 0m0.000s
One problem of current schem in vlan dismantle phase is the
holding of device done by following chain :
vlan_dev_stop() ->
netif_carrier_off(dev) ->
linkwatch_fire_event(dev) ->
dev_hold() ...
And __linkwatch_run_queue() runs up to one second later...
A generic fix to this problem is to add a linkwatch_forget_dev() method
to unlink the device from the list of watched devices.
dev->link_watch_next becomes dev->link_watch_list (and use a bit more memory),
to be able to unlink device in O(1).
After patch :
time ip link del eth3.103 ; time ip link del eth3.104 ; time ip link del eth3.105
real 0m0.024s
user 0m0.000s
sys 0m0.000s
real 0m0.032s
user 0m0.000s
sys 0m0.001s
real 0m0.033s
user 0m0.000s
sys 0m0.000s
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/netdevice.h | 3 +-
net/core/dev.c | 3 ++
net/core/link_watch.c | 42 ++++++++++++++++++++++++------------
3 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7043f85..4e25730 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -896,7 +896,7 @@ struct net_device {
/* device index hash chain */
struct hlist_node index_hlist;
- struct net_device *link_watch_next;
+ struct list_head link_watch_list;
/* register/unregister state machine */
enum { NETREG_UNINITIALIZED=0,
@@ -1600,6 +1600,7 @@ static inline void dev_hold(struct net_device *dev)
*/
extern void linkwatch_fire_event(struct net_device *dev);
+extern void linkwatch_forget_dev(struct net_device *dev);
/**
* netif_carrier_ok - test if carrier present
diff --git a/net/core/dev.c b/net/core/dev.c
index 4b24d79..649de02 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5066,6 +5066,8 @@ static void netdev_wait_allrefs(struct net_device *dev)
{
unsigned long rebroadcast_time, warning_time;
+ linkwatch_forget_dev(dev);
+
rebroadcast_time = warning_time = jiffies;
while (atomic_read(&dev->refcnt) != 0) {
if (time_after(jiffies, rebroadcast_time + 1 * HZ)) {
@@ -5280,6 +5282,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
INIT_LIST_HEAD(&dev->napi_list);
INIT_LIST_HEAD(&dev->unreg_list);
+ INIT_LIST_HEAD(&dev->link_watch_list);
dev->priv_flags = IFF_XMIT_DST_RELEASE;
setup(dev);
strcpy(dev->name, name);
diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index bf8f7af..05fe273 100644
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -35,7 +35,7 @@ static unsigned long linkwatch_nextevent;
static void linkwatch_event(struct work_struct *dummy);
static DECLARE_DELAYED_WORK(linkwatch_work, linkwatch_event);
-static struct net_device *lweventlist;
+static LIST_HEAD(lweventlist);
static DEFINE_SPINLOCK(lweventlist_lock);
static unsigned char default_operstate(const struct net_device *dev)
@@ -89,8 +89,10 @@ static void linkwatch_add_event(struct net_device *dev)
unsigned long flags;
spin_lock_irqsave(&lweventlist_lock, flags);
- dev->link_watch_next = lweventlist;
- lweventlist = dev;
+ if (list_empty(&dev->link_watch_list)) {
+ list_add_tail(&dev->link_watch_list, &lweventlist);
+ dev_hold(dev);
+ }
spin_unlock_irqrestore(&lweventlist_lock, flags);
}
@@ -135,7 +137,8 @@ static void linkwatch_schedule_work(int urgent)
static void __linkwatch_run_queue(int urgent_only)
{
- struct net_device *next;
+ struct net_device *dev;
+ LIST_HEAD(wrk);
/*
* Limit the number of linkwatch events to one
@@ -153,19 +156,18 @@ static void __linkwatch_run_queue(int urgent_only)
clear_bit(LW_URGENT, &linkwatch_flags);
spin_lock_irq(&lweventlist_lock);
- next = lweventlist;
- lweventlist = NULL;
- spin_unlock_irq(&lweventlist_lock);
+ list_splice_init(&lweventlist, &wrk);
- while (next) {
- struct net_device *dev = next;
+ while (!list_empty(&wrk)) {
- next = dev->link_watch_next;
+ dev = list_first_entry(&wrk, struct net_device, link_watch_list);
+ list_del_init(&dev->link_watch_list);
if (urgent_only && !linkwatch_urgent_event(dev)) {
- linkwatch_add_event(dev);
+ list_add_tail(&dev->link_watch_list, &lweventlist);
continue;
}
+ spin_unlock_irq(&lweventlist_lock);
/*
* Make sure the above read is complete since it can be
@@ -189,10 +191,24 @@ static void __linkwatch_run_queue(int urgent_only)
}
dev_put(dev);
+ spin_lock_irq(&lweventlist_lock);
}
- if (lweventlist)
+ if (!list_empty(&lweventlist))
linkwatch_schedule_work(0);
+ spin_unlock_irq(&lweventlist_lock);
+}
+
+void linkwatch_forget_dev(struct net_device *dev)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&lweventlist_lock, flags);
+ if (!list_empty(&dev->link_watch_list)) {
+ list_del_init(&dev->link_watch_list);
+ dev_put(dev);
+ }
+ spin_unlock_irqrestore(&lweventlist_lock, flags);
}
@@ -216,8 +232,6 @@ void linkwatch_fire_event(struct net_device *dev)
bool urgent = linkwatch_urgent_event(dev);
if (!test_and_set_bit(__LINK_STATE_LINKWATCH_PENDING, &dev->state)) {
- dev_hold(dev);
-
linkwatch_add_event(dev);
} else if (!urgent)
return;
^ permalink raw reply related
* Re: [net-next-2.6 PATCH v6 6/7 RFC] TCPCT part 1f: Initiator Cookie => Responder
From: William Allen Simpson @ 2009-11-16 21:35 UTC (permalink / raw)
To: Linux Kernel Network Developers; +Cc: Eric Dumazet, Joe Perches
In-Reply-To: <4AFD8E9D.9050602@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 262 bytes --]
William Allen Simpson wrote:
> Moving the initialization (and destruction) to part 1d makes this about as
> short and easy to analyze as possible. Any more technical observations?
>
Removed 8-bit test, the result of related comments in part 1c.
Seeking Acks.
[-- Attachment #2: TCPCT+1f6++.patch --]
[-- Type: text/plain, Size: 9477 bytes --]
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index e59fa5a..34b750e 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -370,15 +370,45 @@ static inline int tcp_urg_mode(const struct tcp_sock *tp)
#define OPTION_TS (1 << 1)
#define OPTION_MD5 (1 << 2)
#define OPTION_WSCALE (1 << 3)
+#define OPTION_COOKIE_EXTENSION (1 << 4)
struct tcp_out_options {
u8 options; /* bit field of OPTION_* */
u8 ws; /* window scale, 0 to disable */
u8 num_sack_blocks; /* number of SACK blocks to include */
+ u8 hash_size; /* bytes in hash_location */
u16 mss; /* 0 to disable */
__u32 tsval, tsecr; /* need to include OPTION_TS */
+ __u8 *hash_location; /* temporary pointer, overloaded */
};
+/* The sysctl int routines are generic, so check consistency here.
+ */
+static u8 tcp_cookie_size_check(u8 desired)
+{
+ if (desired > 0) {
+ /* previously specified */
+ return desired;
+ }
+ if (sysctl_tcp_cookie_size <= 0) {
+ /* no default specified */
+ return 0;
+ }
+ if (sysctl_tcp_cookie_size < TCP_COOKIE_MIN) {
+ /* value too small, increase to minimum */
+ return TCP_COOKIE_MIN;
+ }
+ if (sysctl_tcp_cookie_size > TCP_COOKIE_MAX) {
+ /* value too large, decrease to maximum */
+ return TCP_COOKIE_MAX;
+ }
+ if (0x1 & sysctl_tcp_cookie_size) {
+ /* 8-bit multiple, illegal, fix it */
+ return (u8)(sysctl_tcp_cookie_size + 0x1);
+ }
+ return (u8)sysctl_tcp_cookie_size;
+}
+
/* Write previously computed TCP options to the packet.
*
* Beware: Something in the Internet is very sensitive to the ordering of
@@ -393,17 +423,34 @@ struct tcp_out_options {
* (but it may well be that other scenarios fail similarly).
*/
static void tcp_options_write(__be32 *ptr, struct tcp_sock *tp,
- const struct tcp_out_options *opts,
- __u8 **md5_hash) {
- if (unlikely(OPTION_MD5 & opts->options)) {
- *ptr++ = htonl((TCPOPT_NOP << 24) |
- (TCPOPT_NOP << 16) |
- (TCPOPT_MD5SIG << 8) |
- TCPOLEN_MD5SIG);
- *md5_hash = (__u8 *)ptr;
+ struct tcp_out_options *opts)
+{
+ u8 options = opts->options; /* mungable copy */
+
+ /* Having both authentication and cookies for security is redundant,
+ * and there's certainly not enough room. Instead, the cookie-less
+ * extension variant is proposed.
+ *
+ * Consider the pessimal case with authentication. The options
+ * could look like:
+ * COOKIE|MD5(20) + MSS(4) + SACK|TS(12) + WSCALE(4) == 40
+ */
+ if (unlikely(OPTION_MD5 & options)) {
+ if (unlikely(OPTION_COOKIE_EXTENSION & options)) {
+ *ptr++ = htonl((TCPOPT_COOKIE << 24) |
+ (TCPOLEN_COOKIE_BASE << 16) |
+ (TCPOPT_MD5SIG << 8) |
+ TCPOLEN_MD5SIG);
+ } else {
+ *ptr++ = htonl((TCPOPT_NOP << 24) |
+ (TCPOPT_NOP << 16) |
+ (TCPOPT_MD5SIG << 8) |
+ TCPOLEN_MD5SIG);
+ }
+ options &= ~OPTION_COOKIE_EXTENSION;
+ /* overload cookie hash location */
+ opts->hash_location = (__u8 *)ptr;
ptr += 4;
- } else {
- *md5_hash = NULL;
}
if (unlikely(opts->mss)) {
@@ -412,12 +459,13 @@ static void tcp_options_write(__be32 *ptr, struct tcp_sock *tp,
opts->mss);
}
- if (likely(OPTION_TS & opts->options)) {
- if (unlikely(OPTION_SACK_ADVERTISE & opts->options)) {
+ if (likely(OPTION_TS & options)) {
+ if (unlikely(OPTION_SACK_ADVERTISE & options)) {
*ptr++ = htonl((TCPOPT_SACK_PERM << 24) |
(TCPOLEN_SACK_PERM << 16) |
(TCPOPT_TIMESTAMP << 8) |
TCPOLEN_TIMESTAMP);
+ options &= ~OPTION_SACK_ADVERTISE;
} else {
*ptr++ = htonl((TCPOPT_NOP << 24) |
(TCPOPT_NOP << 16) |
@@ -428,15 +476,52 @@ static void tcp_options_write(__be32 *ptr, struct tcp_sock *tp,
*ptr++ = htonl(opts->tsecr);
}
- if (unlikely(OPTION_SACK_ADVERTISE & opts->options &&
- !(OPTION_TS & opts->options))) {
+ /* Specification requires after timestamp, so do it now.
+ *
+ * Consider the pessimal case without authentication. The options
+ * could look like:
+ * MSS(4) + SACK|TS(12) + COOKIE(20) + WSCALE(4) == 40
+ */
+ if (unlikely(OPTION_COOKIE_EXTENSION & options)) {
+ __u8 *cookie_copy = opts->hash_location;
+ u8 cookie_size = opts->hash_size;
+
+ /* 8-bit multiple handled in tcp_cookie_size_check() above,
+ * and elsewhere.
+ */
+ if (0x2 & cookie_size) {
+ __u8 *p = (__u8 *)ptr;
+
+ /* 16-bit multiple */
+ *p++ = TCPOPT_COOKIE;
+ *p++ = TCPOLEN_COOKIE_BASE + cookie_size;
+ *p++ = *cookie_copy++;
+ *p++ = *cookie_copy++;
+ ptr++;
+ cookie_size -= 2;
+ } else {
+ /* 32-bit multiple */
+ *ptr++ = htonl(((TCPOPT_NOP << 24) |
+ (TCPOPT_NOP << 16) |
+ (TCPOPT_COOKIE << 8) |
+ TCPOLEN_COOKIE_BASE) +
+ cookie_size);
+ }
+
+ if (cookie_size > 0) {
+ memcpy(ptr, cookie_copy, cookie_size);
+ ptr += (cookie_size / 4);
+ }
+ }
+
+ if (unlikely(OPTION_SACK_ADVERTISE & options)) {
*ptr++ = htonl((TCPOPT_NOP << 24) |
(TCPOPT_NOP << 16) |
(TCPOPT_SACK_PERM << 8) |
TCPOLEN_SACK_PERM);
}
- if (unlikely(OPTION_WSCALE & opts->options)) {
+ if (unlikely(OPTION_WSCALE & options)) {
*ptr++ = htonl((TCPOPT_NOP << 24) |
(TCPOPT_WINDOW << 16) |
(TCPOLEN_WINDOW << 8) |
@@ -471,8 +556,12 @@ static unsigned tcp_syn_options(struct sock *sk, struct sk_buff *skb,
struct tcp_out_options *opts,
struct tcp_md5sig_key **md5) {
struct tcp_sock *tp = tcp_sk(sk);
- unsigned size = 0;
+ struct tcp_cookie_values *cvp = tp->cookie_values;
struct dst_entry *dst = __sk_dst_get(sk);
+ unsigned size = 0;
+ u8 cookie_size = (!tp->rx_opt.cookie_out_never && cvp != NULL)
+ ? tcp_cookie_size_check(cvp->cookie_desired)
+ : 0;
#ifdef CONFIG_TCP_MD5SIG
*md5 = tp->af_specific->md5_lookup(sk, sk);
@@ -517,6 +606,53 @@ static unsigned tcp_syn_options(struct sock *sk, struct sk_buff *skb,
size += TCPOLEN_SACKPERM_ALIGNED;
}
+ /* Note that timestamps are required by the specification.
+ *
+ * Odd numbers of bytes are prohibited by the specification, ensuring
+ * that the cookie is 16-bit aligned, and the resulting cookie pair is
+ * 32-bit aligned.
+ */
+ if (*md5 == NULL
+ && (OPTION_TS & opts->options)
+ && cookie_size > 0) {
+ int need = TCPOLEN_COOKIE_BASE + cookie_size;
+ int remaining = MAX_TCP_OPTION_SPACE - size;
+
+ if (0x2 & need) {
+ /* 32-bit multiple */
+ need += 2; /* NOPs */
+
+ if (need > remaining) {
+ /* try shrinking cookie to fit */
+ cookie_size -= 2;
+ need -= 4;
+ }
+ }
+ while (need > remaining && TCP_COOKIE_MIN <= cookie_size) {
+ cookie_size -= 4;
+ need -= 4;
+ }
+ if (TCP_COOKIE_MIN <= cookie_size) {
+ opts->options |= OPTION_COOKIE_EXTENSION;
+ opts->hash_location = (__u8 *)&cvp->cookie_pair[0];
+ opts->hash_size = cookie_size;
+
+ /* Remember for future incarnations. */
+ cvp->cookie_desired = cookie_size;
+
+ if (cvp->cookie_desired != cvp->cookie_pair_size) {
+ /* Currently use random bytes as a nonce,
+ * assuming these are completely unpredictable
+ * by hostile users of the same system.
+ */
+ get_random_bytes(&cvp->cookie_pair[0],
+ cookie_size);
+ cvp->cookie_pair_size = cookie_size;
+ }
+
+ size += need;
+ }
+ }
return size;
}
@@ -632,7 +768,6 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,
struct tcp_out_options opts;
unsigned tcp_options_size, tcp_header_size;
struct tcp_md5sig_key *md5;
- __u8 *md5_hash_location;
struct tcphdr *th;
int err;
@@ -703,7 +838,7 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,
}
}
- tcp_options_write((__be32 *)(th + 1), tp, &opts, &md5_hash_location);
+ tcp_options_write((__be32 *)(th + 1), tp, &opts);
if (likely((tcb->flags & TCPCB_FLAG_SYN) == 0))
TCP_ECN_send(sk, skb, tcp_header_size);
@@ -711,7 +846,7 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,
/* Calculate the MD5 hash, as we have all we need now */
if (md5) {
sk->sk_route_caps &= ~NETIF_F_GSO_MASK;
- tp->af_specific->calc_md5_hash(md5_hash_location,
+ tp->af_specific->calc_md5_hash(opts.hash_location,
md5, sk, NULL, skb);
}
#endif
@@ -2235,14 +2370,13 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
struct request_sock *req,
struct request_values *rvp)
{
+ struct tcp_out_options opts;
struct inet_request_sock *ireq = inet_rsk(req);
struct tcp_sock *tp = tcp_sk(sk);
struct tcphdr *th;
- int tcp_header_size;
- struct tcp_out_options opts;
struct sk_buff *skb;
struct tcp_md5sig_key *md5;
- __u8 *md5_hash_location;
+ int tcp_header_size;
int mss;
skb = sock_wmalloc(sk, MAX_TCP_HEADER + 15, 1, GFP_ATOMIC);
@@ -2303,14 +2437,14 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
/* RFC1323: The window in SYN & SYN/ACK segments is never scaled. */
th->window = htons(min(req->rcv_wnd, 65535U));
- tcp_options_write((__be32 *)(th + 1), tp, &opts, &md5_hash_location);
+ tcp_options_write((__be32 *)(th + 1), tp, &opts);
th->doff = (tcp_header_size >> 2);
TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTSEGS);
#ifdef CONFIG_TCP_MD5SIG
/* Okay, we have all we need - do the md5 hash if needed */
if (md5) {
- tcp_rsk(req)->af_specific->calc_md5_hash(md5_hash_location,
+ tcp_rsk(req)->af_specific->calc_md5_hash(opts.hash_location,
md5, NULL, req, skb);
}
#endif
^ permalink raw reply related
* Re: [PATCH] net: factorize rt_do_flush for batch device unregistering
From: Eric Dumazet @ 2009-11-16 21:32 UTC (permalink / raw)
To: Octavian Purdila; +Cc: netdev
In-Reply-To: <200911162308.59730.opurdila@ixiacom.com>
Octavian Purdila a écrit :
> Tests performed with per device sysctl/sysfs entries disabled:
>
> $ insmod /lib/modules/dummy.ko numdummies=8000
> $ time rmmod dummy
>
> Without the patch: With the patch:
> real 0m 3.65s real 0m 0.27s
> user 0m 0.00s user 0m 0.00s
> sys 0m 3.42s sys 0m 0.24s
>
> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> ---
> net/core/dev.c | 28 ++++++++++++++++++++++++++--
> net/ipv4/fib_frontend.c | 13 ++++++++-----
> 2 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 4b24d79..b0a14f0 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4720,7 +4720,8 @@ static void net_set_todo(struct net_device *dev)
>
> static void rollback_registered_many(struct list_head *head)
> {
> - struct net_device *dev;
> + struct net_device *dev, *aux, *fdev;
> + LIST_HEAD(rt_flush_list);
>
> BUG_ON(dev_boot_phase);
> ASSERT_RTNL();
> @@ -4778,8 +4779,28 @@ static void rollback_registered_many(struct list_head *head)
>
> synchronize_net();
>
> - list_for_each_entry(dev, head, unreg_list)
> + /* flush route cache by resending one NETDEV_UNREGISTER per namespace */
> + list_for_each_entry_safe(dev, aux, head, unreg_list) {
> + int needs_flush = 1;
> + list_for_each_entry(fdev, &rt_flush_list, unreg_list) {
> + if (dev_net(dev) == dev_net(fdev)) {
> + needs_flush = 0;
> + dev_put(dev);
> + break;
> + }
> + }
> + if (needs_flush) {
> + list_del(&dev->unreg_list);
> + list_add(&dev->unreg_list, &rt_flush_list);
list_move ...
> + }
> + }
> +
> + list_for_each_entry_safe(dev, aux, &rt_flush_list, unreg_list) {
> + list_del_init(&dev->unreg_list);
> + call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
> + list_add(&dev->unreg_list, head);
> dev_put(dev);
> + }
> }
>
> static void rollback_registered(struct net_device *dev)
> @@ -5374,6 +5395,9 @@ EXPORT_SYMBOL(unregister_netdevice_queue);
> * unregister_netdevice_many - unregister many devices
> * @head: list of devices
> *
> + * WARNING: This function modifies the list. It may change the order of the
> + * elements in the list. However, you can assume it does not add or delete
> + * elements to/from the list.
Sorry I dont understand this comment
> */
> void unregister_netdevice_many(struct list_head *head)
> {
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index 816e218..1972760 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -895,11 +895,11 @@ static void nl_fib_lookup_exit(struct net *net)
> net->ipv4.fibnl = NULL;
> }
>
> -static void fib_disable_ip(struct net_device *dev, int force)
> +static void fib_disable_ip(struct net_device *dev, int force, int delay)
> {
> if (fib_sync_down_dev(dev, force))
> fib_flush(dev_net(dev));
> - rt_cache_flush(dev_net(dev), 0);
> + rt_cache_flush(dev_net(dev), delay);
> arp_ifdown(dev);
> }
>
> @@ -922,7 +922,7 @@ static int fib_inetaddr_event(struct notifier_block *this, unsigned long event,
> /* Last address was deleted from this interface.
> Disable IP.
> */
> - fib_disable_ip(dev, 1);
> + fib_disable_ip(dev, 1, 0);
> } else {
> rt_cache_flush(dev_net(dev), -1);
> }
> @@ -937,7 +937,10 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo
> struct in_device *in_dev = __in_dev_get_rtnl(dev);
>
> if (event == NETDEV_UNREGISTER) {
> - fib_disable_ip(dev, 2);
> + /* if this event is part of a batch then don't flush the cache
> + * now; we will receive another event at the end of the batch */
> + int rt_flush = list_empty(&dev->unreg_list) ? 0 : -1;
hmm... a bit ugly...
> + fib_disable_ip(dev, 2, rt_flush);
> return NOTIFY_DONE;
> }
>
> @@ -955,7 +958,7 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo
> rt_cache_flush(dev_net(dev), -1);
> break;
> case NETDEV_DOWN:
> - fib_disable_ip(dev, 0);
> + fib_disable_ip(dev, 0, 0);
> break;
> case NETDEV_CHANGEMTU:
> case NETDEV_CHANGE:
Are you sure you want to overload NETDEV_UNREGISTER ?
Maybe it would be cleaner to add a new value, NETDEV_UNREGISTER_PERNET or something
for the final loop...
^ permalink raw reply
* [PATCH] net: factorize rt_do_flush for batch device unregistering
From: Octavian Purdila @ 2009-11-16 21:08 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet
Tests performed with per device sysctl/sysfs entries disabled:
$ insmod /lib/modules/dummy.ko numdummies=8000
$ time rmmod dummy
Without the patch: With the patch:
real 0m 3.65s real 0m 0.27s
user 0m 0.00s user 0m 0.00s
sys 0m 3.42s sys 0m 0.24s
Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
net/core/dev.c | 28 ++++++++++++++++++++++++++--
net/ipv4/fib_frontend.c | 13 ++++++++-----
2 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 4b24d79..b0a14f0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4720,7 +4720,8 @@ static void net_set_todo(struct net_device *dev)
static void rollback_registered_many(struct list_head *head)
{
- struct net_device *dev;
+ struct net_device *dev, *aux, *fdev;
+ LIST_HEAD(rt_flush_list);
BUG_ON(dev_boot_phase);
ASSERT_RTNL();
@@ -4778,8 +4779,28 @@ static void rollback_registered_many(struct list_head *head)
synchronize_net();
- list_for_each_entry(dev, head, unreg_list)
+ /* flush route cache by resending one NETDEV_UNREGISTER per namespace */
+ list_for_each_entry_safe(dev, aux, head, unreg_list) {
+ int needs_flush = 1;
+ list_for_each_entry(fdev, &rt_flush_list, unreg_list) {
+ if (dev_net(dev) == dev_net(fdev)) {
+ needs_flush = 0;
+ dev_put(dev);
+ break;
+ }
+ }
+ if (needs_flush) {
+ list_del(&dev->unreg_list);
+ list_add(&dev->unreg_list, &rt_flush_list);
+ }
+ }
+
+ list_for_each_entry_safe(dev, aux, &rt_flush_list, unreg_list) {
+ list_del_init(&dev->unreg_list);
+ call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
+ list_add(&dev->unreg_list, head);
dev_put(dev);
+ }
}
static void rollback_registered(struct net_device *dev)
@@ -5374,6 +5395,9 @@ EXPORT_SYMBOL(unregister_netdevice_queue);
* unregister_netdevice_many - unregister many devices
* @head: list of devices
*
+ * WARNING: This function modifies the list. It may change the order of the
+ * elements in the list. However, you can assume it does not add or delete
+ * elements to/from the list.
*/
void unregister_netdevice_many(struct list_head *head)
{
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 816e218..1972760 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -895,11 +895,11 @@ static void nl_fib_lookup_exit(struct net *net)
net->ipv4.fibnl = NULL;
}
-static void fib_disable_ip(struct net_device *dev, int force)
+static void fib_disable_ip(struct net_device *dev, int force, int delay)
{
if (fib_sync_down_dev(dev, force))
fib_flush(dev_net(dev));
- rt_cache_flush(dev_net(dev), 0);
+ rt_cache_flush(dev_net(dev), delay);
arp_ifdown(dev);
}
@@ -922,7 +922,7 @@ static int fib_inetaddr_event(struct notifier_block *this, unsigned long event,
/* Last address was deleted from this interface.
Disable IP.
*/
- fib_disable_ip(dev, 1);
+ fib_disable_ip(dev, 1, 0);
} else {
rt_cache_flush(dev_net(dev), -1);
}
@@ -937,7 +937,10 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo
struct in_device *in_dev = __in_dev_get_rtnl(dev);
if (event == NETDEV_UNREGISTER) {
- fib_disable_ip(dev, 2);
+ /* if this event is part of a batch then don't flush the cache
+ * now; we will receive another event at the end of the batch */
+ int rt_flush = list_empty(&dev->unreg_list) ? 0 : -1;
+ fib_disable_ip(dev, 2, rt_flush);
return NOTIFY_DONE;
}
@@ -955,7 +958,7 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo
rt_cache_flush(dev_net(dev), -1);
break;
case NETDEV_DOWN:
- fib_disable_ip(dev, 0);
+ fib_disable_ip(dev, 0, 0);
break;
case NETDEV_CHANGEMTU:
case NETDEV_CHANGE:
--
1.5.6.5
^ permalink raw reply related
* Re: [net-next-2.6 PATCH v6 4/7 RFC] TCPCT part 1d: define TCP cookie option, extend existing struct's
From: Eric Dumazet @ 2009-11-16 21:08 UTC (permalink / raw)
To: William Allen Simpson; +Cc: Linux Kernel Network Developers, Joe Perches
In-Reply-To: <4B01BB33.7080702@gmail.com>
William Allen Simpson a écrit :
> William Allen Simpson wrote:
>> Here's my revised attempt (untested). Any technical corrections?
>>
> Limited multiple (=) assignments to only 2 per line for readability
> (that in existing code have 4 per line and run well beyond 80 characters,
> triggering a warning in scripts/checkpatch.pl).
>
> Still no technical corrections. Seeking Acks.
>
I am a bit uneasy on this patch, since apparently you have infrastructure
to send DATA payload on SYN, but I thought it was an optional part of your 'RFC'
and as such, being implemented later ?
I remember a previous remark from David that our skb queues would not contain
DATA on SYN packets...
^ permalink raw reply
* Re: [Bugme-new] [Bug 14450] New: [2.6.31] Network interfaces are dead with 2.6.31 (iwlagn and sky2)
From: Bjorn Helgaas @ 2009-11-16 21:06 UTC (permalink / raw)
To: Andrew Morton
Cc: orion.linbug, bugzilla-daemon, bugme-daemon, netdev, linux-acpi,
Rafael J. Wysocki, linux-wireless
In-Reply-To: <200911040947.28054.bjorn.helgaas@hp.com>
On Wednesday 04 November 2009 09:47:26 am Bjorn Helgaas wrote:
> On Tuesday 03 November 2009 04:21:46 pm Andrew Morton wrote:
> >
> > (switched to email. Please respond via emailed reply-to-all, not via the
> > bugzilla web interface).
>
> > > http://bugzilla.kernel.org/show_bug.cgi?id=14450
>
> This looks like a duplicate of http://bugzilla.kernel.org/show_bug.cgi?id=13940
>
> There's a patch in that bug that is upstream already:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=15b812f1d0a5ca8f5efe7f5882f468af10682ca8
>
> Please try booting 2.6.31 with "pci=use_crs" and report the result.
>
> If possible, please also try booting an upstream kernel containing
> the patch above (without "pci=use_crs") and report that result, too.
Ping! A.G., have you had a chance to try this out?
^ permalink raw reply
* Re: [net-next-2.6 PATCH v6 4/7 RFC] TCPCT part 1d: define TCP cookie option, extend existing struct's
From: William Allen Simpson @ 2009-11-16 20:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Kernel Network Developers, Joe Perches
In-Reply-To: <4AFD8421.6090003@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 338 bytes --]
William Allen Simpson wrote:
> Here's my revised attempt (untested). Any technical corrections?
>
Limited multiple (=) assignments to only 2 per line for readability
(that in existing code have 4 per line and run well beyond 80 characters,
triggering a warning in scripts/checkpatch.pl).
Still no technical corrections. Seeking Acks.
[-- Attachment #2: TCPCT+1d6++.patch --]
[-- Type: text/plain, Size: 10948 bytes --]
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index eaa3113..7fee8a4 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -247,31 +247,38 @@ struct tcp_options_received {
sack_ok : 4, /* SACK seen on SYN packet */
snd_wscale : 4, /* Window scaling received from sender */
rcv_wscale : 4; /* Window scaling to send to receiver */
-/* SACKs data */
+ u8 cookie_plus:6, /* bytes in authenticator/cookie option */
+ cookie_out_never:1,
+ cookie_in_always:1;
u8 num_sacks; /* Number of SACK blocks */
- u16 user_mss; /* mss requested by user in ioctl */
+ u16 user_mss; /* mss requested by user in ioctl */
u16 mss_clamp; /* Maximal mss, negotiated at connection setup */
};
static inline void tcp_clear_options(struct tcp_options_received *rx_opt)
{
- rx_opt->tstamp_ok = rx_opt->sack_ok = rx_opt->wscale_ok = rx_opt->snd_wscale = 0;
+ rx_opt->tstamp_ok = rx_opt->sack_ok = 0;
+ rx_opt->wscale_ok = rx_opt->snd_wscale = 0;
+ rx_opt->cookie_plus = 0;
}
/* This is the max number of SACKS that we'll generate and process. It's safe
- * to increse this, although since:
+ * to increase this, although since:
* size = TCPOLEN_SACK_BASE_ALIGNED (4) + n * TCPOLEN_SACK_PERBLOCK (8)
* only four options will fit in a standard TCP header */
#define TCP_NUM_SACKS 4
+struct tcp_cookie_values;
+struct tcp_request_sock_ops;
+
struct tcp_request_sock {
struct inet_request_sock req;
#ifdef CONFIG_TCP_MD5SIG
/* Only used by TCP MD5 Signature so far. */
const struct tcp_request_sock_ops *af_specific;
#endif
- u32 rcv_isn;
- u32 snt_isn;
+ u32 rcv_isn;
+ u32 snt_isn;
};
static inline struct tcp_request_sock *tcp_rsk(const struct request_sock *req)
@@ -441,6 +448,12 @@ struct tcp_sock {
/* TCP MD5 Signature Option information */
struct tcp_md5sig_info *md5sig_info;
#endif
+
+ /* When the cookie options are generated and exchanged, then this
+ * object holds a reference to them (cookie_values->kref). Also
+ * contains related tcp_cookie_transactions fields.
+ */
+ struct tcp_cookie_values *cookie_values;
};
static inline struct tcp_sock *tcp_sk(const struct sock *sk)
@@ -459,6 +472,10 @@ struct tcp_timewait_sock {
u16 tw_md5_keylen;
u8 tw_md5_key[TCP_MD5SIG_MAXKEYLEN];
#endif
+ /* Few sockets in timewait have cookies; in that case, then this
+ * object holds a reference to them (tw_cookie_values->kref).
+ */
+ struct tcp_cookie_values *tw_cookie_values;
};
static inline struct tcp_timewait_sock *tcp_twsk(const struct sock *sk)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 738b65f..f9abd9b 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -30,6 +30,7 @@
#include <linux/dmaengine.h>
#include <linux/crypto.h>
#include <linux/cryptohash.h>
+#include <linux/kref.h>
#include <net/inet_connection_sock.h>
#include <net/inet_timewait_sock.h>
@@ -164,6 +165,7 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
#define TCPOPT_SACK 5 /* SACK Block */
#define TCPOPT_TIMESTAMP 8 /* Better RTT estimations/PAWS */
#define TCPOPT_MD5SIG 19 /* MD5 Signature (RFC2385) */
+#define TCPOPT_COOKIE 253 /* Cookie extension (experimental) */
/*
* TCP option lengths
@@ -174,6 +176,10 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
#define TCPOLEN_SACK_PERM 2
#define TCPOLEN_TIMESTAMP 10
#define TCPOLEN_MD5SIG 18
+#define TCPOLEN_COOKIE_BASE 2 /* Cookie-less header extension */
+#define TCPOLEN_COOKIE_PAIR 3 /* Cookie pair header extension */
+#define TCPOLEN_COOKIE_MIN (TCPOLEN_COOKIE_BASE+TCP_COOKIE_MIN)
+#define TCPOLEN_COOKIE_MAX (TCPOLEN_COOKIE_BASE+TCP_COOKIE_MAX)
/* But this is what stacks really send out. */
#define TCPOLEN_TSTAMP_ALIGNED 12
@@ -1482,6 +1488,83 @@ struct tcp_request_sock_ops {
extern int tcp_cookie_generator(u32 *bakery);
+/**
+ * struct tcp_cookie_values - each socket needs extra space for the
+ * cookies, together with (optional) space for any SYN data.
+ *
+ * A tcp_sock contains a pointer to the current value, and this is
+ * cloned to the tcp_timewait_sock.
+ *
+ * @cookie_pair: variable data from the option exchange.
+ *
+ * @cookie_desired: user specified tcpct_cookie_desired. Zero
+ * indicates default (sysctl_tcp_cookie_size).
+ * After cookie sent, remembers size of cookie.
+ * Range 0, TCP_COOKIE_MIN to TCP_COOKIE_MAX.
+ *
+ * @s_data_desired: user specified tcpct_s_data_desired. When the
+ * constant payload is specified (@s_data_constant),
+ * holds its length instead.
+ * Range 0 to TCP_MSS_DESIRED.
+ *
+ * @s_data_payload: constant data that is to be included in the
+ * payload of SYN or SYNACK segments when the
+ * cookie option is present.
+ */
+struct tcp_cookie_values {
+ struct kref kref;
+ u8 cookie_pair[TCP_COOKIE_PAIR_SIZE];
+ u8 cookie_pair_size;
+ u8 cookie_desired;
+ u16 s_data_desired:11,
+ s_data_constant:1,
+ s_data_in:1,
+ s_data_out:1,
+ s_data_unused:2;
+ u8 s_data_payload[0];
+};
+
+static inline void tcp_cookie_values_release(struct kref *kref)
+{
+ kfree(container_of(kref, struct tcp_cookie_values, kref));
+}
+
+/* The length of constant payload data. Note that s_data_desired is
+ * overloaded, depending on s_data_constant: either the length of constant
+ * data (returned here) or the limit on variable data.
+ */
+static inline int tcp_s_data_size(const struct tcp_sock *tp)
+{
+ return (tp->cookie_values != NULL && tp->cookie_values->s_data_constant)
+ ? tp->cookie_values->s_data_desired
+ : 0;
+}
+
+/**
+ * struct tcp_extend_values - tcp_ipv?.c to tcp_output.c workspace.
+ *
+ * As tcp_request_sock has already been extended in other places, the
+ * only remaining method is to pass stack values along as function
+ * parameters. These parameters are not needed after sending SYNACK.
+ *
+ * @cookie_bakery: cryptographic secret and message workspace.
+ *
+ * @cookie_plus: bytes in authenticator/cookie option, copied from
+ * struct tcp_options_received (above).
+ */
+struct tcp_extend_values {
+ struct request_values rv;
+ u32 cookie_bakery[COOKIE_WORKSPACE_WORDS];
+ u8 cookie_plus:6,
+ cookie_out_never:1,
+ cookie_in_always:1;
+};
+
+static inline struct tcp_extend_values *tcp_xv(struct request_values *rvp)
+{
+ return (struct tcp_extend_values *)rvp;
+}
+
extern void tcp_v4_init(void);
extern void tcp_init(void);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 397ab8f..2bb7864 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1834,6 +1834,19 @@ static int tcp_v4_init_sock(struct sock *sk)
tp->af_specific = &tcp_sock_ipv4_specific;
#endif
+ /* TCP Cookie Transactions */
+ if (sysctl_tcp_cookie_size > 0) {
+ /* Default, cookies without s_data_payload. */
+ tp->cookie_values =
+ kzalloc(sizeof(*tp->cookie_values),
+ sk->sk_allocation);
+ if (tp->cookie_values != NULL)
+ kref_init(&tp->cookie_values->kref);
+ }
+ /* Presumed zeroed, in order of appearance:
+ * cookie_in_always, cookie_out_never,
+ * s_data_constant, s_data_in, s_data_out
+ */
sk->sk_sndbuf = sysctl_tcp_wmem[1];
sk->sk_rcvbuf = sysctl_tcp_rmem[1];
@@ -1887,6 +1900,13 @@ void tcp_v4_destroy_sock(struct sock *sk)
sk->sk_sndmsg_page = NULL;
}
+ /* TCP Cookie Transactions */
+ if (tp->cookie_values != NULL) {
+ kref_put(&tp->cookie_values->kref,
+ tcp_cookie_values_release);
+ tp->cookie_values = NULL;
+ }
+
percpu_counter_dec(&tcp_sockets_allocated);
}
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 7a42990..53ef6d8 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -389,14 +389,43 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
const struct inet_request_sock *ireq = inet_rsk(req);
struct tcp_request_sock *treq = tcp_rsk(req);
struct inet_connection_sock *newicsk = inet_csk(newsk);
- struct tcp_sock *newtp;
+ struct tcp_sock *newtp = tcp_sk(newsk);
+ struct tcp_sock *oldtp = tcp_sk(sk);
+ struct tcp_cookie_values *oldcvp = oldtp->cookie_values;
+
+ /* TCP Cookie Transactions require space for the cookie pair,
+ * as it differs for each connection. There is no need to
+ * copy any s_data_payload stored at the original socket.
+ * Failure will prevent resuming the connection.
+ *
+ * Presumed copied, in order of appearance:
+ * cookie_in_always, cookie_out_never
+ */
+ if (oldcvp != NULL) {
+ struct tcp_cookie_values *newcvp =
+ kzalloc(sizeof(*newtp->cookie_values),
+ GFP_ATOMIC);
+
+ if (newcvp != NULL) {
+ kref_init(&newcvp->kref);
+ newcvp->cookie_desired =
+ oldcvp->cookie_desired;
+ newtp->cookie_values = newcvp;
+ } else {
+ /* Not Yet Implemented */
+ newtp->cookie_values = NULL;
+ }
+ }
/* Now setup tcp_sock */
- newtp = tcp_sk(newsk);
newtp->pred_flags = 0;
- newtp->rcv_wup = newtp->copied_seq = newtp->rcv_nxt = treq->rcv_isn + 1;
- newtp->snd_sml = newtp->snd_una = newtp->snd_nxt = treq->snt_isn + 1;
- newtp->snd_up = treq->snt_isn + 1;
+
+ newtp->rcv_wup = newtp->copied_seq =
+ newtp->rcv_nxt = treq->rcv_isn + 1;
+
+ newtp->snd_sml = newtp->snd_una =
+ newtp->snd_nxt = newtp->snd_up =
+ treq->snt_isn + 1 + tcp_s_data_size(oldtp);
tcp_prequeue_init(newtp);
@@ -429,8 +458,8 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
tcp_set_ca_state(newsk, TCP_CA_Open);
tcp_init_xmit_timers(newsk);
skb_queue_head_init(&newtp->out_of_order_queue);
- newtp->write_seq = treq->snt_isn + 1;
- newtp->pushed_seq = newtp->write_seq;
+ newtp->write_seq = newtp->pushed_seq =
+ treq->snt_isn + 1 + tcp_s_data_size(oldtp);
newtp->rx_opt.saw_tstamp = 0;
@@ -596,7 +625,8 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
* Invalid ACK: reset will be sent by listening socket
*/
if ((flg & TCP_FLAG_ACK) &&
- (TCP_SKB_CB(skb)->ack_seq != tcp_rsk(req)->snt_isn + 1))
+ (TCP_SKB_CB(skb)->ack_seq !=
+ tcp_rsk(req)->snt_isn + 1 + tcp_s_data_size(tcp_sk(sk))))
return sk;
/* Also, it would be not so bad idea to check rcv_tsecr, which
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 3e327bc..973096a 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1865,6 +1865,19 @@ static int tcp_v6_init_sock(struct sock *sk)
tp->af_specific = &tcp_sock_ipv6_specific;
#endif
+ /* TCP Cookie Transactions */
+ if (sysctl_tcp_cookie_size > 0) {
+ /* Default, cookies without s_data_payload. */
+ tp->cookie_values =
+ kzalloc(sizeof(*tp->cookie_values),
+ sk->sk_allocation);
+ if (tp->cookie_values != NULL)
+ kref_init(&tp->cookie_values->kref);
+ }
+ /* Presumed zeroed, in order of appearance:
+ * cookie_in_always, cookie_out_never,
+ * s_data_constant, s_data_in, s_data_out
+ */
sk->sk_sndbuf = sysctl_tcp_wmem[1];
sk->sk_rcvbuf = sysctl_tcp_rmem[1];
^ permalink raw reply related
* Re: sparse vs. skbuff.h
From: Eric Dumazet @ 2009-11-16 20:42 UTC (permalink / raw)
To: Vegard Nossum; +Cc: Johannes Berg, netdev
In-Reply-To: <19f34abd0911161223s3ead190aka62bf4295a3f935@mail.gmail.com>
Vegard Nossum a écrit :
> 2009/11/16 Eric Dumazet <eric.dumazet@gmail.com>:
>> Johannes Berg a écrit :
>>> On Mon, 2009-11-16 at 20:21 +0100, Johannes Berg wrote:
>>>> commit 14d18a81b5171d4433e41129619c75748b4f4d26
>>>> Author: Eric Dumazet <eric.dumazet@gmail.com>
>>>> Date: Thu Oct 29 00:10:37 2009 +0000
>>>>
>>>> net: fix kmemcheck annotations
>>>>
>>>>
>>>> broke sparse endian checks on everything that includes skbuff.h because
>>>> the first and only (because it's an error) thing sparse now reports is
>>>> this:
>>>>
>>>> include/linux/skbuff.h:357:41: error: invalid bitfield specifier for type restricted __be16.
>>> Simply changing from
>>> __be16 protocol:16;
>>> to
>>> __be16 protocol;
>>>
>>> but leaving it inside the kmemcheck annotation seems to do the right
>>> thing. Except of course that kmemcheck will not properly check it now.
>>> Maybe those annotations should simply be made to have no impact on
>>> struct padding instead?
>>>
>> Hmm, I have really no idea of what is the right way to fix this stuff.
>>
>> Last time I did adding a non bitfield element inside the begin/end annotations,
>> I was flamed, because a bitfield is a bitfield, not a char/short
>>
>> http://www.spinics.net/lists/netdev/msg108803.html
>>
>> Now sparse is complaining... What will be the next story ?
>
> If by "I was flamed" you are referring to my reply:
>
> http://www.spinics.net/lists/netdev/msg108825.html
>
> then I am really sorry, because I had no intentions to insult you. In
> fact, I am grateful that you are finding bugs and telling me about
> them But I should also be allowed to disagree with a patch if I truly
> believe it is the wrong thing to do.
Oh well, maybe flamed is not the right word Vegard, and I did not feel
being insulted at all ! Sorry if the tone of my mail was misleading.
>
> For the issue in question: If the variable is turned into a
> non-bitfield (as Johannes suggested), it would be fine, because now
> GCC won't emit masking operations (AND, OR) when initializing it, but
> a regular MOV. Also, the struct annotations do not by themselves do
> anything, but they are used by kmemcheck_annotate_bitfield().
>
> In other words, I think the right thing to do is to turn it into a
> non-bitfield and move it _outside_ the bitfield annotation. Johannes,
> can you make the patch and let us have a look? In the meantime I will
> submit the patch that fixes the extraneous field padding in
> KMEMCHECK=n kernels.
>
Thanks
^ permalink raw reply
* Re: [net-next-2.6 PATCH v6 3/7 RFC] TCPCT part 1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS
From: William Allen Simpson @ 2009-11-16 20:40 UTC (permalink / raw)
To: Joe Perches; +Cc: Linux Kernel Network Developers
In-Reply-To: <4AFED025.1060202@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 102 bytes --]
Improved Documentation to match existing code:
Odd values are interpreted as the next even value.
[-- Attachment #2: TCPCT+1c6+.patch --]
[-- Type: text/plain, Size: 5128 bytes --]
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index a0e134d..820dd4b 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -164,6 +164,14 @@ tcp_congestion_control - STRING
additional choices may be available based on kernel configuration.
Default is set as part of kernel configuration.
+tcp_cookie_size - INTEGER
+ Default size of TCP Cookie Transactions (TCPCT) option, that may be
+ overridden on a per socket basis by the TCPCT socket option.
+ Values greater than the maximum (16) are interpreted as the maximum.
+ Values greater than zero and less than the minimum (8) are interpreted
+ as the minimum. Odd values are interpreted as the next even value.
+ Default: 0 (off).
+
tcp_dsack - BOOLEAN
Allows TCP to send "duplicate" SACKs.
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 32d7d77..eaa3113 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -102,7 +102,9 @@ enum {
#define TCP_QUICKACK 12 /* Block/reenable quick acks */
#define TCP_CONGESTION 13 /* Congestion control algorithm */
#define TCP_MD5SIG 14 /* TCP MD5 Signature (RFC2385) */
+#define TCP_COOKIE_TRANSACTIONS 15 /* TCP Cookie Transactions */
+/* for TCP_INFO socket option */
#define TCPI_OPT_TIMESTAMPS 1
#define TCPI_OPT_SACK 2
#define TCPI_OPT_WSCALE 4
@@ -174,6 +176,30 @@ struct tcp_md5sig {
__u8 tcpm_key[TCP_MD5SIG_MAXKEYLEN]; /* key (binary) */
};
+/* for TCP_COOKIE_TRANSACTIONS (TCPCT) socket option */
+#define TCP_COOKIE_MIN 8 /* 64-bits */
+#define TCP_COOKIE_MAX 16 /* 128-bits */
+#define TCP_COOKIE_PAIR_SIZE (2*TCP_COOKIE_MAX)
+
+/* Flags for both getsockopt and setsockopt */
+#define TCP_COOKIE_IN_ALWAYS (1 << 0) /* Discard SYN without cookie */
+#define TCP_COOKIE_OUT_NEVER (1 << 1) /* Prohibit outgoing cookies,
+ * supercedes everything. */
+
+/* Flags for getsockopt */
+#define TCP_S_DATA_IN (1 << 2) /* Was data received? */
+#define TCP_S_DATA_OUT (1 << 3) /* Was data sent? */
+
+/* TCP_COOKIE_TRANSACTIONS data */
+struct tcp_cookie_transactions {
+ __u16 tcpct_flags; /* see above */
+ __u8 __tcpct_pad1; /* zero */
+ __u8 tcpct_cookie_desired; /* bytes */
+ __u16 tcpct_s_data_desired; /* bytes of variable data */
+ __u16 tcpct_used; /* bytes in value */
+ __u8 tcpct_value[TCP_MSS_DEFAULT];
+};
+
#ifdef __KERNEL__
#include <linux/skbuff.h>
@@ -227,6 +253,11 @@ struct tcp_options_received {
u16 mss_clamp; /* Maximal mss, negotiated at connection setup */
};
+static inline void tcp_clear_options(struct tcp_options_received *rx_opt)
+{
+ rx_opt->tstamp_ok = rx_opt->sack_ok = rx_opt->wscale_ok = rx_opt->snd_wscale = 0;
+}
+
/* This is the max number of SACKS that we'll generate and process. It's safe
* to increse this, although since:
* size = TCPOLEN_SACK_BASE_ALIGNED (4) + n * TCPOLEN_SACK_PERBLOCK (8)
@@ -435,6 +466,6 @@ static inline struct tcp_timewait_sock *tcp_twsk(const struct sock *sk)
return (struct tcp_timewait_sock *)sk;
}
-#endif
+#endif /* __KERNEL__ */
#endif /* _LINUX_TCP_H */
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 4a99a8e..738b65f 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -234,6 +234,7 @@ extern int sysctl_tcp_base_mss;
extern int sysctl_tcp_workaround_signed_windows;
extern int sysctl_tcp_slow_start_after_idle;
extern int sysctl_tcp_max_ssthresh;
+extern int sysctl_tcp_cookie_size;
extern atomic_t tcp_memory_allocated;
extern struct percpu_counter tcp_sockets_allocated;
@@ -340,11 +341,6 @@ static inline void tcp_dec_quickack_mode(struct sock *sk,
extern void tcp_enter_quickack_mode(struct sock *sk);
-static inline void tcp_clear_options(struct tcp_options_received *rx_opt)
-{
- rx_opt->tstamp_ok = rx_opt->sack_ok = rx_opt->wscale_ok = rx_opt->snd_wscale = 0;
-}
-
#define TCP_ECN_OK 1
#define TCP_ECN_QUEUE_CWR 2
#define TCP_ECN_DEMAND_CWR 4
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 2dcf04d..3422c54 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -714,6 +714,14 @@ static struct ctl_table ipv4_table[] = {
},
{
.ctl_name = CTL_UNNUMBERED,
+ .procname = "tcp_cookie_size",
+ .data = &sysctl_tcp_cookie_size,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec
+ },
+ {
+ .ctl_name = CTL_UNNUMBERED,
.procname = "udp_mem",
.data = &sysctl_udp_mem,
.maxlen = sizeof(sysctl_udp_mem),
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 1151cb8..e59fa5a 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -59,6 +59,14 @@ int sysctl_tcp_base_mss __read_mostly = 512;
/* By default, RFC2861 behavior. */
int sysctl_tcp_slow_start_after_idle __read_mostly = 1;
+#ifdef CONFIG_SYSCTL
+/* By default, let the user enable it. */
+int sysctl_tcp_cookie_size __read_mostly = 0;
+#else
+int sysctl_tcp_cookie_size __read_mostly = TCP_COOKIE_MAX;
+#endif
+
+
/* Account for new data that has been sent to the network. */
static void tcp_event_new_data_sent(struct sock *sk, struct sk_buff *skb)
{
^ permalink raw reply related
* Re: Those gpio-mdio patches...
From: Lennart Sorensen @ 2009-11-16 20:25 UTC (permalink / raw)
To: Lennart Sorensen; +Cc: David Miller, netdev
In-Reply-To: <20091116182145.GB15157@caffeine.csclub.uwaterloo.ca>
On Mon, Nov 16, 2009 at 01:21:45PM -0500, Lennart Sorensen wrote:
> On Fri, Nov 13, 2009 at 01:59:08PM -0800, David Miller wrote:
> > Please don't fix the issue that way. If the routine is returning a
> > true or false value, make it return a 'bool' instead of an 'int'.
> > That way we don't have to "accept something other than 0 and 1"
>
> Hmm. Well I just tried to fix what was already there. gpio-lib does
> NOT return 0 or 1, it returns 0 or 1 shifted by the gpio position in a
> gpio register, but soemthing has to convert that to a boolean value.
> Making it return a bool doesn't change the fact it needs the '!!' applied
> to make it a bool.
>
> I can look at changing the mdio-gpio to using bool instead of integer
> where necesary.
So it looks like this would involve changing mdio-bitbang to using bool
rather than int in a couple of places. This would affect:
./include/linux/mdio-bitbang.h
./arch/powerpc/platforms/82xx/ep8248e.c
./drivers/net/fs_enet/mii-bitbang.c
./drivers/net/phy/mdio-bitbang.c
./drivers/net/phy/mdio-gpio.c
./drivers/net/sh_eth.c
I could write up a patch for that if it makes sense.
I figure it would mean changing:
void (*set_mdc)(struct mdiobb_ctrl *ctrl, int level);
void (*set_mdio_dir)(struct mdiobb_ctrl *ctrl, int output);
void (*set_mdio_data)(struct mdiobb_ctrl *ctrl, int value);
int (*get_mdio_data)(struct mdiobb_ctrl *ctrl);
into:
void (*set_mdc)(struct mdiobb_ctrl *ctrl, bool level);
void (*set_mdio_dir)(struct mdiobb_ctrl *ctrl, bool output);
void (*set_mdio_data)(struct mdiobb_ctrl *ctrl, bool value);
bool (*get_mdio_data)(struct mdiobb_ctrl *ctrl);
Does that make sense?
Of course the only place that breaks right now is mdiobb_get_num which
assumes the value is 0 or 1. Everywhere else appears to be fine with
0 and non-0. Maybe putting the fix just in there or in mdiobb_get_bit
makes more sense, since then it doesn't matter what the users of
mdio-bitbang do, it will always work.
--
Len Sorensen
^ permalink raw reply
* Re: sparse vs. skbuff.h
From: Vegard Nossum @ 2009-11-16 20:23 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Johannes Berg, netdev
In-Reply-To: <4B01AD5F.7020402@gmail.com>
2009/11/16 Eric Dumazet <eric.dumazet@gmail.com>:
> Johannes Berg a écrit :
>> On Mon, 2009-11-16 at 20:21 +0100, Johannes Berg wrote:
>>> commit 14d18a81b5171d4433e41129619c75748b4f4d26
>>> Author: Eric Dumazet <eric.dumazet@gmail.com>
>>> Date: Thu Oct 29 00:10:37 2009 +0000
>>>
>>> net: fix kmemcheck annotations
>>>
>>>
>>> broke sparse endian checks on everything that includes skbuff.h because
>>> the first and only (because it's an error) thing sparse now reports is
>>> this:
>>>
>>> include/linux/skbuff.h:357:41: error: invalid bitfield specifier for type restricted __be16.
>>
>> Simply changing from
>> __be16 protocol:16;
>> to
>> __be16 protocol;
>>
>> but leaving it inside the kmemcheck annotation seems to do the right
>> thing. Except of course that kmemcheck will not properly check it now.
>> Maybe those annotations should simply be made to have no impact on
>> struct padding instead?
>>
>
> Hmm, I have really no idea of what is the right way to fix this stuff.
>
> Last time I did adding a non bitfield element inside the begin/end annotations,
> I was flamed, because a bitfield is a bitfield, not a char/short
>
> http://www.spinics.net/lists/netdev/msg108803.html
>
> Now sparse is complaining... What will be the next story ?
If by "I was flamed" you are referring to my reply:
http://www.spinics.net/lists/netdev/msg108825.html
then I am really sorry, because I had no intentions to insult you. In
fact, I am grateful that you are finding bugs and telling me about
them But I should also be allowed to disagree with a patch if I truly
believe it is the wrong thing to do.
For the issue in question: If the variable is turned into a
non-bitfield (as Johannes suggested), it would be fine, because now
GCC won't emit masking operations (AND, OR) when initializing it, but
a regular MOV. Also, the struct annotations do not by themselves do
anything, but they are used by kmemcheck_annotate_bitfield().
In other words, I think the right thing to do is to turn it into a
non-bitfield and move it _outside_ the bitfield annotation. Johannes,
can you make the patch and let us have a look? In the meantime I will
submit the patch that fixes the extraneous field padding in
KMEMCHECK=n kernels.
Vegard
^ permalink raw reply
* Re: [RFC PATCH] net: add dataref destructor to sk_buff
From: Gregory Haskins @ 2009-11-16 20:18 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Herbert Xu, Gregory Haskins, Michael S. Tsirkin, alacrityvm-devel,
linux-kernel, netdev
In-Reply-To: <20091116115931.6266f9c9@nehalam>
[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]
Stephen Hemminger wrote:
> On Sat, 14 Nov 2009 00:27:46 -0500
> Gregory Haskins <gregory.haskins@gmail.com> wrote:
>
>> Stephen Hemminger wrote:
>
>>> People have tried doing copy-less send by page flipping, but the overhead of the IPI to
>>> invalidate the TLB exceeded the overhead of the copy. There was an Intel paper on this in
>>> at Linux Symposium (Ottawa) several years ago.
>> I think you are confusing copy-less tx with copy-less rx. You can try
>> to do copy-less rx with page flipping, which has the IPI/TLB thrashing
>> properties you mention, and I agree is problematic. We are talking
>> about copy-less tx here, however, and therefore no page-flipping is
>> involved. Rather, we are just posting SG lists of pages directly to the
>> NIC (assuming the nic supports HIGH_DMA, etc). You do not need to flip
>> the page, or invalidate the TLB (and thus IPI the other cores) to do
>> this to my knowledge.
>>
>
> If you want to do copy-less tx for all applications, you have to
> do COW to handle the trivial case of :
>
> while (cc = read(infd, buffer, sizeof buffer)) {
> send(outsock, buffer, cc);
> }
>
>
You certainly _could_ implement this as a COW I suppose, but that would
be insane. If someone did do this, you are right: you need TLB
invalidation.
However, if I were going to actually propose the changeover of the
system calls to use zero-copy (note that I am not), it would be based on
the concept in this patch. That is: the send() would block until the
NIC completes the DMA and the shinfo block is freed. Alternate
implementations would be AIO based, where the shinfo destructor
signifies the generation of the completion event.
FWIW: The latter is conceptually similar to how this is being used in
AlacrityVM.
HTH
Kind Regards,
-Greg
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]
^ permalink raw reply
* Re: Doubt in implementations of mean loss interval at sender side
From: Ivo Calado @ 2009-11-16 20:09 UTC (permalink / raw)
To: Gerrit Renker, dccp, netdev
In-Reply-To: <cb00fa210911161207n5f255a16w1b750701c1bd177c@mail.gmail.com>
On Mon, Nov 9, 2009 at 4:09 AM, Gerrit Renker <gerrit@erg.abdn.ac.uk> wrote:
> | > To sum up, here is whay I think is minimally required to satisfy the union
> | > of RFC 4340, 4342, 4828, 5348, and 5622:
> | >
> | > struct tfrc_tx_packet_info {
> | > u64 seqno:48,
> | > is_ect0:1,
> | > is_data_packet:1,
> | > is_in_loss_interval:1;
> | > u32 send_time;
> | > u32 rtt_estimate;
> | > struct tfrc_tx_packet_info *next; /* FIFO */
> | > };
> | >
> | > That would be a per-packet storage cost of about 16 bytes, plus the pointer
> | > (8 bytes on 64-bit architectures). One could avoid the pointer by defining a
> | > u64 base_seqno;
> | > and then
> | > struct tfrc_tx_packet_info[some constant here];
> | > and then index the array relative to the base_seqno.
> | >
> |
> | Yes, I believe that struct is enough too. But how long would be necessary
> | the struct array to be?
> |
> The problem is the same as with Ack Vectors - the array (or list) can grow
> arbitrarily large. You made a good reply, since all the questions are
> inter-related. The first two I see here are
>
> 1) the choice of data structure (array or list)
> 2) the design of a garbage-collector
>
> This includes your point from above, about the maximum size. To draw the
> analogy to Ack Vectors, at the moment they use a fixed size. On certain
> mediums (WiFi) there exist situations where even that fixed limit is
> reached, causing an overflow with Ack Vectors that have reached a size
> of 2 * 253 = 506 bytes.
>
> Looking after old data of sent packets is similar, the main difference I
> see that at some stage "unused" old entries need to be collected, to avoid
> the overflow problem which occurs when using a fixed-size structure.
>
> I find that 'Acknowledgments of Acknowledgments' is a bad idea, since it
> means implementing reliable delivery over unreliable transport; on the
> one hand DCCP is designed to be unreliable, but here suddenly is a break
> with that design decision.
>
> So point (2) will probably mean coming up with some guessed heuristics
> that will work for most scenarios, but may fail in others.
>
>
> This is why I am not a big fan of the sender-based solution: to solve (2) and
> your question from above requires a lot of testing of the garbage-collection
> and book-keeping algorithms, rather than on the actual congestion control.
>
> One can spend a lot of time going over these issues, but of what use is the
> most ingenious data structure if the overall protocol behavior does not
> deliver a useful performance to users of the protocol?
Yes, a sender-based implementation seems really complicated, mainly in
principle.
>
> | > IIb) Further remarks
> | > --------------------
> | > At first sight it would seem that storing the RTT also solves the problem
> | > of inaccurate RTTs used at the receiver. Unfortunately, this is not the
> | > case. X_recv is sampled over intervals of varying length which may or may
> | > not equal the RTT. To factor out the effect of window counters, the sender
> | > would need to store the packet size as well and would need to use rather
> | > complicated computations - an ugly workaround.
> |
> | I didn't understand how the packet size would help and what
> | computations are needed.
> |
> The above still refers to the earlier posting about letting the sender
> supply the RTT estimate R_i in packet `i' as defined in RFC 5348, 3.2.1.
>
> Though the same section later suggests that a coarse-grained timestamp
> is sufficient, in practice the inaccuracy of the RTT means inaccurate
> X_recv, and as a consequence sub-optimal protocol performance.
>
> The problem is that the algorithm from RFC 4342, 8.1 assumes that the
> rate of change of the window counter also relates to the change of
> packet spacing (the difference between the T_i packe arrivel times).
>
> Howver, especially when using high-speed (1/10 Gbit) networking, this
> assumption often does not hold in practice. Packets are sent by the network
> card in bunches, or intervening switches/routers cause a compression of
> packet inter-arrival times. Hence it is perfectly possible that a bundle of
> packets with different Window Counter CCVal values arrive at virtually
> the same time. For instance, on a 100Mbs ethernet I have seen spikes of
> X_recv of up to 2-3 Gbits/sec. Several orders of magnitude from the
> real packet speed (not to mention the unrealistic value).
>
> So the question above was asking whether there is a way for the sender
> to "compute away" the inaccuracies reported by the receiver. Your reply
> confirms my doubts that doing this is probably not possible.
>
> To clarify, I was asking whether it would be possible for the sender to
> perform step (2) of RFC 5348, 6.2; to compensate for the fact that the
> receiver does not have a reliable RTT estimate.
I understand now the issue, thanks. Isn't better to just send the RTT estimate
to the sender, as the RFC says?
>
> For example, when receiving feedback for packet `i', it would iterate
> through the list/array, going back over as many packets as are covered
> by the send time T_i of packet `i' minus the RTT estimate R_i at that
> time, sum their packet sizes, and from that value recompute X_recv.
>
> This is a bit complicated if the garbage-collector has already purged
> older entries, so part of the garbage collector would probably have
> to watch over acknowledged packets. I add this as item
> 3) validate X_recv
> to the above running list of book-keeping items done at the sender.
>
>
> | > One thing I stumbled across while reading your code was the fact that RFC 4342
> | > leaves it open as to how many Loss Intervals to send: on the one hand it follows
> | > the suggestion of RFC 5348 to use 1+NINTERVAL=9, but on the other hand it does
> | > not restrict the number of loss intervals. Also RFC 5622 does not limit the
> | > number of Loss Intervals / Data Dropped options.
> | >
> | > If receiving n > 9 Loss Intervals, what does the sender do with the n-9 older
> | > intervals? There must be some mechanism to stop these options from growing
> | > beyond bounds, so it needs to store also which loss intervals have been
> | > acknowledged, introducing the "Acknowledgment of Acknowledgments"
> | > problem.
> |
> | In RFC 4342 section 8.6 it says that the limit of loss interval data
> | to send is 28, and RFC 5622 8.7 says 84 for dropped packets option.
> | But I don't see why to send so many data in these options.
> | Yes, the most recent 9 loss intervals are required to be reported,
> | except if the sender acknowledged previous sent loss intervals, so in
> | that case only one is required, the open interval.
> | And we can avoid the "Acknowledgment of Acknowledgments" if we always send
> | the required 9 loss intervals, I think.
> |
> | > A second point is how to compute the loss event rate when n > 9. It seems
> | > that this would mean grinding through all loss intervals using a window
> | > of 9. If that is the case, the per-packet-computation costs become very
> | > expensive.
> |
> | RFC 4342 section 8.6 suggests that only 9 loss intervals are required
> | anyway. And I believe that's enough for the computation of current
> | mean loss interval. What do you think?
> |
> Yes, absolutely, I am completely in favour of this very sensible suggestion.
>
> If people really must experiment with such outdated data, that could be
> done in truly experimental patches. Especially since RFC 5348 normatively
> recommends a value of n = 8 in section 5.4. And we are saving further
> headaches about the book-keeping/garbage collection of old data.
>
> | > II) Computational part of the implementation
> | > --------------------------------------------
> | > If only Loss Intervals alone are used, only these need to be verified
> | > before being used to alter the sender behaviour.
> | >
> | > But when one or more other DCCP options also appear, the verification is
> | > * intra: make sure each received option is in itself consistent,
> | > * inter: make sure options are mutually consistent.
> | >
> | > The second has a combinatorial effect, i.e. n! verifications for n options.
> | >
> <snip>
> |
> | Yes, there's a combinatorial problem in checking the options for consistence.
> | But, what if we find out that some option doesn't match against others?
> | What action would be taken?
> I add this as
> 4) define policy for dealing with options that are not mutually consistent
>
> | First, what can cause the receiver to send inconsistent options?
> | A bad implementation only?
> Yes I think that a bad implementation (whether on purpose or not) would be
> the main cause, since header options are protected even if partial
> checksums are used (RFC 4340, 9.2).
>
> But there is also the benign case mentioned at the end of RFC 4342, 9.2,
> where a receiver collapses multiple losses into a single loss event, i.e.
> 5) validate received Loss Intervals and regroup the receiver-based
> information if necessary, without interpreting this as attempted
> receiver misbehaviour.
>
> | Accordingly to ecn nonce echo sum algorithm, if a receiver is found to be
> | lying about loss or to be bad implemented, the sender adjusts the send rate
> | as if loss were perceived.
> | Can we do the same in this situation? If so, can we skip checking options
> | between them and only check ecn nonce sum?
> This is difficult since Ack Vectors and Loss Intervals use different
> definitions of ECN Nonce sum (last paragraph in RFC 4342, 9.1), i.e. we have
> 6) separate algorithms to compute Ack Vector/Loss Intervals ECN Nonce sum.
>
> With regard to (5) above, your suggestion gives
> 7) validate options, on mismatch other than (5) only validate ECN nonce.
>
> | If some option is wrong it show more loss (or any worse situation for the
> | receiver) or conceals loss. In the first case, I don't believe we need to care,
> | and in the second, the ecn nonce sum can reveal the bad acting of the receiver.
> Yes you are right, we need not worry if a receiver reports a higher loss rate
> than the verification done by the sender (which recomputes the data that the
> receiver already has computed) calculates.
>
> But for the second case, there is no guarantee to catch a misbehaving
> receiver, only a 50% chance at the end of many computations.
Isn't it 50% chance at each ecn verified? So, at the end we'll end up with 100%?
>
> RFC 4342, 9 suggests one way of verifying Loss Intervals / Ack Vectors:
> 5) occasionally do not send a packet, or send packet out of order.
>
> This increases complexity of book-keeping, the sender needs to keep track
> which of the sent packets was a fake send/drop. It also requires an algorithm
> to iterate over the sender data structures in order to find out whether the
> reasoning of the receiver is sane. I have doubts whether this can be done
> without sacrificing the performance of the in-kernel sender side.
>
I have doubts either. This seems to be too complicated and not much useful.
>
> | > III) Closing remarks in favour of receiver-based implementation
> | > ---------------------------------------------------------------
> | > Finally, both RFC 4342 and RFC 5622 do not explicitly discard the
> | > possibility of using a receiver-based implementation. Quoting
> | > RFC 4342, 3.2: "If it prefers, the sender can also use a loss event
> | > rate calculated and reported by the receiver."
> | > Furthermore, the revised TFRC specification points out in section 7
> | > the advantages that a receiver-based implementation has:
> | > * it does not mandate reliable delivery of packet loss data;
> | > * it is robust against the loss of feedback packets;
> | > * better suited for scalable server design.
> | >
> | > Quite likely, if the server does not have to store and validate a mass
> | > of data, it is also less prone to be toppled by DoS attacks.
> |
> | You're right. But what the RFC's says about it is almost exactly the
> | opposite, isn't? What can we do about it? I like the receiver-based design,
> | but I believe that loss intervals are interesting, mostly because of
> | receiver behavior verification.
> |
> While writing the above reply, I was amazed to see how much of the computation
> that has already been done at the receiver needs to be done again at the sender,
> ust in order to be able to verify the data.
>
> To me this seems very inefficient.
>
> Moreover, the biggest danger I see here is spending a lot of time with the
> details of sender book-keeping and verification, just to then see that the
> performance of CCID-3/4 in practice turns out to be below the standards
> acceptable to even modest users.
>
> I think it is clearly better to prefer the simplest possible implementation
> in such cases, to better debug live protocol performance.
>
> In particular, since CCID-4 is designed to be an experimental protocol
> (i.e. if it works, RFC 5622 may mature into a Proposed Standard, if not,
> it might be superseded by a different specification).
>
> And I think that testing the actual user performance has the highest priority.
Yes, we can work more with a simpler implementation at the receiver
side and focus
on performance and test, and features too. After, we have a stable
version and good enough in performance terms,
we can continue improving the sender side.
>
> The literature on the subject is almost exclusively done on experiences in
> ns-2 userland. Almost no Internet experiments at all have been done with DCCP.
>
> This is because the IPPROTO = 33 identifier needs to be entered
> especially into a firewall, which opens holes that firewall
> administrators don't like to open (unless the firewall is based on
> a recent Linux kernel, opening all ports for IP protocol identifier
> 33 is the only way of allowing DCCP traffic in/out of a firewall).
>
> In addition, most people use NAT at home, putting another obstacle
> on experiments. The result is then that tests are done in a lab testbed
> or in virtualisation - emulated networks.
>
> To conclude, I still think that the simpler, receiver-based implementation
> gives a better start. A 'perfect' receiver implementation is also a good
> reference point to start protocol evaluation: if the performance is bad
> despite getting things right at the receiver, then other parts of the
> protocol need investigation/improvement.
I agree. It's a risk to work on the sender at the moment, implementing
these features,
algorithms and ending with a CCID that doesn't match the expected performance.
Can you list the pending tasks in both code and tests to be done?
Cheers,
Ivo
--
Ivo Augusto Andrade Rocha Calado
MSc. Candidate
Embedded Systems and Pervasive Computing Lab - http://embedded.ufcg.edu.br
Systems and Computing Department - http://www.dsc.ufcg.edu.br
Electrical Engineering and Informatics Center - http://www.ceei.ufcg.edu.br
Federal University of Campina Grande - http://www.ufcg.edu.br
PGP: 0x03422935
Putt's Law:
Technology is dominated by two types of people:
Those who understand what they do not manage.
Those who manage what they do not understand.
^ permalink raw reply
* Re: [net-next-2.6 PATCH v6 2/7 RFC] TCPCT part 1b: generate Responder Cookie
From: William Allen Simpson @ 2009-11-16 20:06 UTC (permalink / raw)
To: Eric Dumazet
Cc: Joe Perches, Linux Kernel Network Developers, Paul E. McKenney
In-Reply-To: <4B01710F.70800@gmail.com>
Eric Dumazet wrote:
> William Allen Simpson a écrit :
>> Do I hear an Ack?
>>
> Fine by me William
>
> But you forgot to re-include full commit message and Signed-off-by:
>
> Yes it seems cumbersome, but David is handling ~50 patches per day,
> we shall give him as _perfect_ patches as possible.
>
Agreed, I understood that I'd need to send out the _entire_ patch series
without the RFC designation before our leader would consider it.
Right now, I'm just collecting as many comments, improvements, and Acks as
possible before fetch, rebase, compiling, testing, and re-posting the
whole megillah again with "no fuzz" patches.
Hopefully, splitting this into more sub-parts (as you requested) enables
reviewing, but of course it's an exponential increase in preparation and
test time. So, I'm trying to get some comments on every sub-part.
^ permalink raw reply
* Re: [RFC PATCH] net: add dataref destructor to sk_buff
From: Stephen Hemminger @ 2009-11-16 19:59 UTC (permalink / raw)
To: Gregory Haskins
Cc: Herbert Xu, Gregory Haskins, Michael S. Tsirkin, alacrityvm-devel,
linux-kernel, netdev
In-Reply-To: <4AFE3FD2.6030403@gmail.com>
On Sat, 14 Nov 2009 00:27:46 -0500
Gregory Haskins <gregory.haskins@gmail.com> wrote:
> Stephen Hemminger wrote:
> > On Fri, 13 Nov 2009 21:27:57 -0500
> > Gregory Haskins <gregory.haskins@gmail.com> wrote:
> >
> >> Herbert Xu wrote:
> >>> On Fri, Nov 13, 2009 at 08:33:35PM -0500, Gregory Haskins wrote:
> >>>> Well, not with respect to the overall protocol, of course not. But with
> >>>> respect to the buffer in question, it _has_ to be. Or am I missing
> >>>> something?
> >>> sendfile() has never guaranteed that the kernel is finished with
> >>> the underlying pages when it returns.
> >>>
> >>> Cheers,
> >> Clearly there must be _some_ mechanism to synchronize (e.g.
> >> flush/barrier) though, right? Otherwise, that interface would seem to
> >> be quite prone to races and would likely be unusable. So what does
> >> said flush use to know when the buffer is free?
> >
> > No all the interfaces require a copy.
>
> I'm sorry, but I do not think that is correct. As others have pointed
> out, that would not appear to be true for at least sendfile.
Correct.
>
> > Actually, sendfile makes no guarantee about synchronization
> > because the receiver of said file could be arbitrarily slow, and any attempt at locking down
> > current contents of file is a denial of service exposure.
>
> I think you are inverting the problem space. It is fully expected that
> changing the "file", or the pages that represent the file before the
> packet is queued would constitute the modification of the stream on the
> wire.
>
> I am more thinking about the applications of mmap+sendfile to implement
> a zero-copy interface. As David mentions in another mail, it seems that
> there is no sync mechanism available, so this would not appear to be a
> viable use case today, unfortunately.
yes, if you do mmap/sendfile then there is no synchronization, and the stack
can hold onto your data for an arbitrary time. The file and mapping's can
be closed but that risks tying up all of memory.
> >
> > People have tried doing copy-less send by page flipping, but the overhead of the IPI to
> > invalidate the TLB exceeded the overhead of the copy. There was an Intel paper on this in
> > at Linux Symposium (Ottawa) several years ago.
>
> I think you are confusing copy-less tx with copy-less rx. You can try
> to do copy-less rx with page flipping, which has the IPI/TLB thrashing
> properties you mention, and I agree is problematic. We are talking
> about copy-less tx here, however, and therefore no page-flipping is
> involved. Rather, we are just posting SG lists of pages directly to the
> NIC (assuming the nic supports HIGH_DMA, etc). You do not need to flip
> the page, or invalidate the TLB (and thus IPI the other cores) to do
> this to my knowledge.
>
If you want to do copy-less tx for all applications, you have to
do COW to handle the trivial case of :
while (cc = read(infd, buffer, sizeof buffer)) {
send(outsock, buffer, cc);
}
--
^ permalink raw reply
* Re: [RFC] dev->refcnt long term holder
From: Eric Dumazet @ 2009-11-16 19:54 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, Herbert Xu, Linux Netdev List
In-Reply-To: <20091116110207.08b60c81@nehalam>
Stephen Hemminger a écrit :
>
> Or both..
>
Well well well :)
I was hoping a fast path, but anyway, a linkwatch_forget_dev(dev) is probably better,
(in case "ip link del" closely follows an "ip link add / up")
I'll post something when tested.
Thanks
^ permalink raw reply
* Re: sparse vs. skbuff.h
From: Eric Dumazet @ 2009-11-16 19:51 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev, Vegard Nossum
In-Reply-To: <1258399627.32159.41.camel@johannes.local>
Johannes Berg a écrit :
> On Mon, 2009-11-16 at 20:21 +0100, Johannes Berg wrote:
>> commit 14d18a81b5171d4433e41129619c75748b4f4d26
>> Author: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Thu Oct 29 00:10:37 2009 +0000
>>
>> net: fix kmemcheck annotations
>>
>>
>> broke sparse endian checks on everything that includes skbuff.h because
>> the first and only (because it's an error) thing sparse now reports is
>> this:
>>
>> include/linux/skbuff.h:357:41: error: invalid bitfield specifier for type restricted __be16.
>
> Simply changing from
> __be16 protocol:16;
> to
> __be16 protocol;
>
> but leaving it inside the kmemcheck annotation seems to do the right
> thing. Except of course that kmemcheck will not properly check it now.
> Maybe those annotations should simply be made to have no impact on
> struct padding instead?
>
Hmm, I have really no idea of what is the right way to fix this stuff.
Last time I did adding a non bitfield element inside the begin/end annotations,
I was flamed, because a bitfield is a bitfield, not a char/short
http://www.spinics.net/lists/netdev/msg108803.html
Now sparse is complaining... What will be the next story ?
^ permalink raw reply
* Re: sparse vs. skbuff.h
From: Johannes Berg @ 2009-11-16 19:27 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, Vegard Nossum
In-Reply-To: <1258399271.32159.39.camel@johannes.local>
[-- Attachment #1: Type: text/plain, Size: 825 bytes --]
On Mon, 2009-11-16 at 20:21 +0100, Johannes Berg wrote:
> commit 14d18a81b5171d4433e41129619c75748b4f4d26
> Author: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu Oct 29 00:10:37 2009 +0000
>
> net: fix kmemcheck annotations
>
>
> broke sparse endian checks on everything that includes skbuff.h because
> the first and only (because it's an error) thing sparse now reports is
> this:
>
> include/linux/skbuff.h:357:41: error: invalid bitfield specifier for type restricted __be16.
Simply changing from
__be16 protocol:16;
to
__be16 protocol;
but leaving it inside the kmemcheck annotation seems to do the right
thing. Except of course that kmemcheck will not properly check it now.
Maybe those annotations should simply be made to have no impact on
struct padding instead?
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* sparse vs. skbuff.h
From: Johannes Berg @ 2009-11-16 19:21 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet
[-- Attachment #1: Type: text/plain, Size: 438 bytes --]
commit 14d18a81b5171d4433e41129619c75748b4f4d26
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu Oct 29 00:10:37 2009 +0000
net: fix kmemcheck annotations
broke sparse endian checks on everything that includes skbuff.h because
the first and only (because it's an error) thing sparse now reports is
this:
include/linux/skbuff.h:357:41: error: invalid bitfield specifier for type restricted __be16.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [RFC] dev->refcnt long term holder
From: Stephen Hemminger @ 2009-11-16 19:02 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Herbert Xu, Linux Netdev List
In-Reply-To: <4B019381.2010509@gmail.com>
On Mon, 16 Nov 2009 19:01:37 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> time ip link del eth3.103 ; time ip link del eth3.104 ; time ip link del eth3.105
>
> real 0m0.266s
> user 0m0.000s
> sys 0m0.001s
>
> real 0m0.770s
> user 0m0.000s
> sys 0m0.000s
>
> real 0m1.022s
> user 0m0.000s
> sys 0m0.000s
>
>
> One problem of current schem in vlan dismantle phase is the
> holding of device done by following chain :
>
> vlan_dev_stop() ->
> netif_carrier_off(dev) ->
> linkwatch_fire_event(dev) ->
> dev_hold() ...
>
> And __linkwatch_run_queue() run up to one second later...
>
> Is following patch one way to avoid the problem, or should
> we add a new linkwatch_forgetpro_device(dev) method to immediately
> release the device reference (and unlink device from the list) ?
> (This would probably need a doubly linked list instead of single link list)
Or both..
--
^ 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