* Re: [Drbd-dev] [PATCH 2/2 v2] netlink: kill eff_cap from struct netlink_skb_parms
From: Patrick McHardy @ 2011-03-08 14:50 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: Chris Wright, David Miller, linux-fbdev, netdev,
linux-security-module, dm-devel, drbd-dev
In-Reply-To: <20110304012956.GA13573@ioremap.net>
Am 04.03.2011 02:29, schrieb Evgeniy Polyakov:
> Hi.
>
> On Thu, Mar 03, 2011 at 11:37:46PM +0100, Lars Ellenberg (lars.ellenberg@linbit.com) wrote:
>> If so, then this change introduces the possibility for normal users to
>> send privileged commands to connector based subsystems, even if they
>> may not be able to bind() to suitable sockets to receive any replies.
>>
>> Am I missing something?
>
> Yup, connector is very async at that place, but I wonder why the hell I
> ever made that decision. I believe we can replace it with pure sync call
> of the registered connector callback, since netlink is synchronous and
> no one has any problem with it.
>
Are you going to do this or do you want me to take care of it?
^ permalink raw reply
* Re: [PATCH] Make CUBIC Hystart more robust to RTT variations
From: Injong Rhee @ 2011-03-08 15:26 UTC (permalink / raw)
To: Lucas Nussbaum; +Cc: WANG Cong, netdev
In-Reply-To: <20110308111011.GA27967@xanadu.blop.info>
Thanks for updating CUBIC hystart. You might want to test the
cases with more background traffic and verify whether this
threshold is too conservative.
On 3/8/11 6:10 AM, Lucas Nussbaum wrote:
> CUBIC Hystart uses two heuristics to exit slow start earlier, before
> losses start to occur. Unfortunately, it tends to exit slow start far
> too early, causing poor performance since convergence to the optimal
> cwnd is then very slow. This was reported in
> http://permalink.gmane.org/gmane.linux.network/188169 and
> https://partner-bugzilla.redhat.com/show_bug.cgi?id=616985
>
> I am using an experimental testbed (http://www.grid5000.fr/) with two
> machines connected using Gigabit ethernet to a dedicated 10-Gb backbone.
> RTT between both machines is 11.3ms. Using TCP CUBIC without Hystart,
> cwnd grows to ~2200. With Hystart enabled, CUBIC exits slow start with
> cwnd lower than 100, and often lower than 20, which leads to the poor
> performance that I reported.
>
> After instrumenting TCP CUBIC, I found out that the segment-to-ack RTT
> tends to vary quite a lot even when the network is not congested, due to
> several factors including the fact that TCP sends packet in burst (so
> the packets are queued locally before being sent, increasing their RTT),
> and delayed ACKs on the destination host.
>
> The patch below increases the thresholds used by the two Hystart
> heuristics. First, the length of an ACK train needs to reach 2*minRTT.
> Second, the max RTT of a group of packets also needs to reach 2*minRTT.
> In my setup, this causes Hystart to exit slow start when cwnd is in the
> 1900-2000 range using the ACK train heuristics, and sometimes to exit in
> the 700-900 range using the delay increase heuristic, dramatically
> improving performance.
>
> I could provide access to my testbed if someone wants to do further
> experiments.
>
> Signed-off-by: Lucas Nussbaum<lucas.nussbaum@loria.fr>
^ permalink raw reply
* Re: [PATCH 2/3] [RFC] Changes for MQ virtio-net
From: Krishna Kumar2 @ 2011-03-08 15:32 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: anthony, arnd, avi, davem, eric.dumazet, horms, kvm, netdev,
rusty
In-Reply-To: <20110302100600.GB31309@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> wrote on 03/02/2011 03:36:00 PM:
Sorry for the delayed response, I have been sick the last few days.
I am responding to both your posts here.
> > Both virtio-net and vhost need some check to make sure very
> > high values are not passed by userspace. Is this not required?
>
> Whatever we stick in the header is effectively part of
> host/gues interface. Are you sure we'll never want
> more than 16 VQs? This value does not seem that high.
OK, so even constants cannot change? Given that, should I remove all
checks and use kcalloc?
> > OK, so virtio_net_config has num_queue_pairs, and this gets converted to
> > numtxqs in virtnet_info?
>
> Or put num_queue_pairs in virtnet_info too.
For virtnet_info, having numtxqs is easier since all code that loops needs
only 'numtxq'.
> > Also, vhost has some
> > code that processes tx first before rx (e.g. vhost_net_stop/flush),
>
> No idea why did I do it this way. I don't think it matters.
>
> > so this approach seemed helpful.
> > I am OK either way, what do you
> > suggest?
>
> We get less code generated but also less flexibility.
> I am not sure, I'll play around with code, for now
> let's keep it as is.
OK.
> > Yes, it is a waste to have these vectors for tx ints. I initially
> > thought of adding a flag to virtio_device to pass to vp_find_vqs,
> > but it won't work, so a new API is needed. I can work with you on
> > this in the background if you like.
>
> OK. For starters, how about we change find_vqs to get a structure? Then
> we can easily add flags that tell us that some interrupts are rare.
Yes. OK to work on this outside this patch series, I guess?
> > vq's are matched between qemu, virtio-net and vhost. Isn't some check
> > required that userspace has not passed a bad value?
>
>
> For virtio, I'm not too concerned: qemu can already easily
> crash the guest :)
> For vhost yes, but I'm concerned that even with 16 VQs we are
> drinking a lot of resources already. I would be happier
> if we had a file descriptor per VQs pair in some way.
> The the amount of memory userspace can use up is
> limited by the # of file descriptors.
I will start working on this approach this week and see how it goes.
> > OK, so define free_unused_bufs() as:
> >
> > static void free_unused_bufs(struct virtnet_info *vi, struct virtqueue
> > *svq,
> > struct virtqueue *rvq)
> > {
> > /* Use svq and rvq with the remaining code unchanged */
> > }
>
> Not sure I understand. I am just suggesting
> adding symmetrical functions like init/cleanup
> alloc/free etc instead of adding stuff in random
> functions that just happens to be called at the right time.
OK, I will clean up this part in the next revision.
> > I was not sure what is the best way - a sysctl parameter? Or should the
> > maximum depend on number of host cpus? But that results in too many
> > threads, e.g. if I have 16 cpus and 16 txqs.
>
> I guess the question is, wouldn't # of threads == # of vqs work best?
> If we process stuff on a single CPU, let's make it pass through
> a single VQ.
> And to do this, we could simply open multiple vhost fds without
> changing vhost at all.
>
> Would this work well?
>
> > > > - enum vhost_net_poll_state tx_poll_state;
> > > > + enum vhost_net_poll_state *tx_poll_state;
> > >
> > > another array?
> >
> > Yes... I am also allocating twice the space than what is required
> > to make it's usage simple.
>
> Where's the allocation? Couldn't find it.
vhost_setup_vqs(net.c) allocates it based on nvqs, though numtxqs is
enough.
Thanks,
- KK
^ permalink raw reply
* Re: [PATCH 2/3] [RFC] Changes for MQ virtio-net
From: Michael S. Tsirkin @ 2011-03-08 15:41 UTC (permalink / raw)
To: Krishna Kumar2
Cc: anthony, arnd, avi, davem, eric.dumazet, horms, kvm, netdev,
rusty
In-Reply-To: <OFCDD0BC16.2EA548B2-ON6525784D.00551582-6525784D.00553911@in.ibm.com>
On Tue, Mar 08, 2011 at 09:02:38PM +0530, Krishna Kumar2 wrote:
> "Michael S. Tsirkin" <mst@redhat.com> wrote on 03/02/2011 03:36:00 PM:
>
> Sorry for the delayed response, I have been sick the last few days.
> I am responding to both your posts here.
>
> > > Both virtio-net and vhost need some check to make sure very
> > > high values are not passed by userspace. Is this not required?
> >
> > Whatever we stick in the header is effectively part of
> > host/gues interface. Are you sure we'll never want
> > more than 16 VQs? This value does not seem that high.
>
> OK, so even constants cannot change?
Parse error :).
> Given that, should I remove all
> checks and use kcalloc?
I think that it's not a bad idea to avoid crashing if
hardware (in our case, host) misbehaves.
But as long as the code is prepared to handle any # of vqs,
I don't see a point of limiting that arbitrarily,
and if the code to handle hardware errors is complex
it'll have bugs itself.
> > > OK, so virtio_net_config has num_queue_pairs, and this gets converted to
> > > numtxqs in virtnet_info?
> >
> > Or put num_queue_pairs in virtnet_info too.
>
> For virtnet_info, having numtxqs is easier since all code that loops needs
> only 'numtxq'.
It seemed to me that the code used numtxqs for # of rx qs as well
sometimes.
> > > Also, vhost has some
> > > code that processes tx first before rx (e.g. vhost_net_stop/flush),
> >
> > No idea why did I do it this way. I don't think it matters.
> >
> > > so this approach seemed helpful.
> > > I am OK either way, what do you
> > > suggest?
> >
> > We get less code generated but also less flexibility.
> > I am not sure, I'll play around with code, for now
> > let's keep it as is.
>
> OK.
>
> > > Yes, it is a waste to have these vectors for tx ints. I initially
> > > thought of adding a flag to virtio_device to pass to vp_find_vqs,
> > > but it won't work, so a new API is needed. I can work with you on
> > > this in the background if you like.
> >
> > OK. For starters, how about we change find_vqs to get a structure? Then
> > we can easily add flags that tell us that some interrupts are rare.
>
> Yes. OK to work on this outside this patch series, I guess?
We could work on this in parallel. Changing APIs is not a problem,
I'm a bit concerned that this might affect the host/guest ABI as well.
> > > vq's are matched between qemu, virtio-net and vhost. Isn't some check
> > > required that userspace has not passed a bad value?
> >
> >
> > For virtio, I'm not too concerned: qemu can already easily
> > crash the guest :)
> > For vhost yes, but I'm concerned that even with 16 VQs we are
> > drinking a lot of resources already. I would be happier
> > if we had a file descriptor per VQs pair in some way.
> > The the amount of memory userspace can use up is
> > limited by the # of file descriptors.
>
> I will start working on this approach this week and see how it goes.
Also, could you post your current version of the qemu code pls?
It's useful for testing and to see the whole picture.
> > > OK, so define free_unused_bufs() as:
> > >
> > > static void free_unused_bufs(struct virtnet_info *vi, struct virtqueue
> > > *svq,
> > > struct virtqueue *rvq)
> > > {
> > > /* Use svq and rvq with the remaining code unchanged */
> > > }
> >
> > Not sure I understand. I am just suggesting
> > adding symmetrical functions like init/cleanup
> > alloc/free etc instead of adding stuff in random
> > functions that just happens to be called at the right time.
>
> OK, I will clean up this part in the next revision.
>
> > > I was not sure what is the best way - a sysctl parameter? Or should the
> > > maximum depend on number of host cpus? But that results in too many
> > > threads, e.g. if I have 16 cpus and 16 txqs.
> >
> > I guess the question is, wouldn't # of threads == # of vqs work best?
> > If we process stuff on a single CPU, let's make it pass through
> > a single VQ.
> > And to do this, we could simply open multiple vhost fds without
> > changing vhost at all.
> >
> > Would this work well?
> >
> > > > > - enum vhost_net_poll_state tx_poll_state;
> > > > > + enum vhost_net_poll_state *tx_poll_state;
> > > >
> > > > another array?
> > >
> > > Yes... I am also allocating twice the space than what is required
> > > to make it's usage simple.
> >
> > Where's the allocation? Couldn't find it.
>
> vhost_setup_vqs(net.c) allocates it based on nvqs, though numtxqs is
> enough.
>
> Thanks,
>
> - KK
^ permalink raw reply
* Re: [PATCH] vhost: Cleanup vhost.c and net.c
From: Michael S. Tsirkin @ 2011-03-08 17:23 UTC (permalink / raw)
To: Krishna Kumar; +Cc: netdev
In-Reply-To: <20110301113637.4130.38439.sendpatchset@krkumar2.in.ibm.com>
On Tue, Mar 01, 2011 at 05:06:37PM +0530, Krishna Kumar wrote:
> Minor cleanup of vhost.c and net.c to match coding style.
>
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
Applied, thanks!
> ---
> drivers/vhost/net.c | 19 +++++++++-----
> drivers/vhost/vhost.c | 53 +++++++++++++++++++++++++++-------------
> 2 files changed, 49 insertions(+), 23 deletions(-)
>
> diff -ruNp org/drivers/vhost/net.c new/drivers/vhost/net.c
> --- org/drivers/vhost/net.c 2011-03-01 13:33:25.000000000 +0530
> +++ new/drivers/vhost/net.c 2011-03-01 13:36:44.000000000 +0530
> @@ -60,6 +60,7 @@ static int move_iovec_hdr(struct iovec *
> {
> int seg = 0;
> size_t size;
> +
> while (len && seg < iov_count) {
> size = min(from->iov_len, len);
> to->iov_base = from->iov_base;
> @@ -79,6 +80,7 @@ static void copy_iovec_hdr(const struct
> {
> int seg = 0;
> size_t size;
> +
> while (len && seg < iovcount) {
> size = min(from->iov_len, len);
> to->iov_base = from->iov_base;
> @@ -296,17 +298,16 @@ static void handle_rx_big(struct vhost_n
> .msg_iov = vq->iov,
> .msg_flags = MSG_DONTWAIT,
> };
> -
> struct virtio_net_hdr hdr = {
> .flags = 0,
> .gso_type = VIRTIO_NET_HDR_GSO_NONE
> };
> -
> size_t len, total_len = 0;
> int err;
> size_t hdr_size;
> /* TODO: check that we are running from vhost_worker? */
> struct socket *sock = rcu_dereference_check(vq->private_data, 1);
> +
> if (!sock || skb_queue_empty(&sock->sk->sk_receive_queue))
> return;
>
> @@ -405,18 +406,17 @@ static void handle_rx_mergeable(struct v
> .msg_iov = vq->iov,
> .msg_flags = MSG_DONTWAIT,
> };
> -
> struct virtio_net_hdr_mrg_rxbuf hdr = {
> .hdr.flags = 0,
> .hdr.gso_type = VIRTIO_NET_HDR_GSO_NONE
> };
> -
> size_t total_len = 0;
> int err, headcount;
> size_t vhost_hlen, sock_hlen;
> size_t vhost_len, sock_len;
> /* TODO: check that we are running from vhost_worker? */
> struct socket *sock = rcu_dereference_check(vq->private_data, 1);
> +
> if (!sock || skb_queue_empty(&sock->sk->sk_receive_queue))
> return;
>
> @@ -654,6 +654,7 @@ static struct socket *get_raw_socket(int
> } uaddr;
> int uaddr_len = sizeof uaddr, r;
> struct socket *sock = sockfd_lookup(fd, &r);
> +
> if (!sock)
> return ERR_PTR(-ENOTSOCK);
>
> @@ -682,6 +683,7 @@ static struct socket *get_tap_socket(int
> {
> struct file *file = fget(fd);
> struct socket *sock;
> +
> if (!file)
> return ERR_PTR(-EBADF);
> sock = tun_get_socket(file);
> @@ -696,6 +698,7 @@ static struct socket *get_tap_socket(int
> static struct socket *get_socket(int fd)
> {
> struct socket *sock;
> +
> /* special case to disable backend */
> if (fd == -1)
> return NULL;
> @@ -741,9 +744,9 @@ static long vhost_net_set_backend(struct
> oldsock = rcu_dereference_protected(vq->private_data,
> lockdep_is_held(&vq->mutex));
> if (sock != oldsock) {
> - vhost_net_disable_vq(n, vq);
> - rcu_assign_pointer(vq->private_data, sock);
> - vhost_net_enable_vq(n, vq);
> + vhost_net_disable_vq(n, vq);
> + rcu_assign_pointer(vq->private_data, sock);
> + vhost_net_enable_vq(n, vq);
> }
>
> mutex_unlock(&vq->mutex);
> @@ -768,6 +771,7 @@ static long vhost_net_reset_owner(struct
> struct socket *tx_sock = NULL;
> struct socket *rx_sock = NULL;
> long err;
> +
> mutex_lock(&n->dev.mutex);
> err = vhost_dev_check_owner(&n->dev);
> if (err)
> @@ -829,6 +833,7 @@ static long vhost_net_ioctl(struct file
> struct vhost_vring_file backend;
> u64 features;
> int r;
> +
> switch (ioctl) {
> case VHOST_NET_SET_BACKEND:
> if (copy_from_user(&backend, argp, sizeof backend))
> diff -ruNp org/drivers/vhost/vhost.c new/drivers/vhost/vhost.c
> --- org/drivers/vhost/vhost.c 2011-01-19 20:01:29.000000000 +0530
> +++ new/drivers/vhost/vhost.c 2011-03-01 13:36:58.000000000 +0530
> @@ -41,8 +41,8 @@ static void vhost_poll_func(struct file
> poll_table *pt)
> {
> struct vhost_poll *poll;
> - poll = container_of(pt, struct vhost_poll, table);
>
> + poll = container_of(pt, struct vhost_poll, table);
> poll->wqh = wqh;
> add_wait_queue(wqh, &poll->wait);
> }
> @@ -85,6 +85,7 @@ void vhost_poll_init(struct vhost_poll *
> void vhost_poll_start(struct vhost_poll *poll, struct file *file)
> {
> unsigned long mask;
> +
> mask = file->f_op->poll(file, &poll->table);
> if (mask)
> vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
> @@ -101,6 +102,7 @@ static bool vhost_work_seq_done(struct v
> unsigned seq)
> {
> int left;
> +
> spin_lock_irq(&dev->work_lock);
> left = seq - work->done_seq;
> spin_unlock_irq(&dev->work_lock);
> @@ -222,6 +224,7 @@ static int vhost_worker(void *data)
> static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
> {
> int i;
> +
> for (i = 0; i < dev->nvqs; ++i) {
> dev->vqs[i].indirect = kmalloc(sizeof *dev->vqs[i].indirect *
> UIO_MAXIOV, GFP_KERNEL);
> @@ -235,6 +238,7 @@ static long vhost_dev_alloc_iovecs(struc
> goto err_nomem;
> }
> return 0;
> +
> err_nomem:
> for (; i >= 0; --i) {
> kfree(dev->vqs[i].indirect);
> @@ -247,6 +251,7 @@ err_nomem:
> static void vhost_dev_free_iovecs(struct vhost_dev *dev)
> {
> int i;
> +
> for (i = 0; i < dev->nvqs; ++i) {
> kfree(dev->vqs[i].indirect);
> dev->vqs[i].indirect = NULL;
> @@ -296,26 +301,28 @@ long vhost_dev_check_owner(struct vhost_
> }
>
> struct vhost_attach_cgroups_struct {
> - struct vhost_work work;
> - struct task_struct *owner;
> - int ret;
> + struct vhost_work work;
> + struct task_struct *owner;
> + int ret;
> };
>
> static void vhost_attach_cgroups_work(struct vhost_work *work)
> {
> - struct vhost_attach_cgroups_struct *s;
> - s = container_of(work, struct vhost_attach_cgroups_struct, work);
> - s->ret = cgroup_attach_task_all(s->owner, current);
> + struct vhost_attach_cgroups_struct *s;
> +
> + s = container_of(work, struct vhost_attach_cgroups_struct, work);
> + s->ret = cgroup_attach_task_all(s->owner, current);
> }
>
> static int vhost_attach_cgroups(struct vhost_dev *dev)
> {
> - struct vhost_attach_cgroups_struct attach;
> - attach.owner = current;
> - vhost_work_init(&attach.work, vhost_attach_cgroups_work);
> - vhost_work_queue(dev, &attach.work);
> - vhost_work_flush(dev, &attach.work);
> - return attach.ret;
> + struct vhost_attach_cgroups_struct attach;
> +
> + attach.owner = current;
> + vhost_work_init(&attach.work, vhost_attach_cgroups_work);
> + vhost_work_queue(dev, &attach.work);
> + vhost_work_flush(dev, &attach.work);
> + return attach.ret;
> }
>
> /* Caller should have device mutex */
> @@ -323,11 +330,13 @@ static long vhost_dev_set_owner(struct v
> {
> struct task_struct *worker;
> int err;
> +
> /* Is there an owner already? */
> if (dev->mm) {
> err = -EBUSY;
> goto err_mm;
> }
> +
> /* No owner, become one */
> dev->mm = get_task_mm(current);
> worker = kthread_create(vhost_worker, dev, "vhost-%d", current->pid);
> @@ -380,6 +389,7 @@ long vhost_dev_reset_owner(struct vhost_
> void vhost_dev_cleanup(struct vhost_dev *dev)
> {
> int i;
> +
> for (i = 0; i < dev->nvqs; ++i) {
> if (dev->vqs[i].kick && dev->vqs[i].handle_kick) {
> vhost_poll_stop(&dev->vqs[i].poll);
> @@ -421,6 +431,7 @@ void vhost_dev_cleanup(struct vhost_dev
> static int log_access_ok(void __user *log_base, u64 addr, unsigned long sz)
> {
> u64 a = addr / VHOST_PAGE_SIZE / 8;
> +
> /* Make sure 64 bit math will not overflow. */
> if (a > ULONG_MAX - (unsigned long)log_base ||
> a + (unsigned long)log_base > ULONG_MAX)
> @@ -461,6 +472,7 @@ static int memory_access_ok(struct vhost
> int log_all)
> {
> int i;
> +
> for (i = 0; i < d->nvqs; ++i) {
> int ok;
> mutex_lock(&d->vqs[i].mutex);
> @@ -527,6 +539,7 @@ static long vhost_set_memory(struct vhos
> {
> struct vhost_memory mem, *newmem, *oldmem;
> unsigned long size = offsetof(struct vhost_memory, regions);
> +
> if (copy_from_user(&mem, m, size))
> return -EFAULT;
> if (mem.padding)
> @@ -544,7 +557,8 @@ static long vhost_set_memory(struct vhos
> return -EFAULT;
> }
>
> - if (!memory_access_ok(d, newmem, vhost_has_feature(d, VHOST_F_LOG_ALL))) {
> + if (!memory_access_ok(d, newmem,
> + vhost_has_feature(d, VHOST_F_LOG_ALL))) {
> kfree(newmem);
> return -EFAULT;
> }
> @@ -560,6 +574,7 @@ static int init_used(struct vhost_virtqu
> struct vring_used __user *used)
> {
> int r = put_user(vq->used_flags, &used->flags);
> +
> if (r)
> return r;
> return get_user(vq->last_used_idx, &used->idx);
> @@ -849,6 +864,7 @@ static const struct vhost_memory_region
> {
> struct vhost_memory_region *reg;
> int i;
> +
> /* linear search is not brilliant, but we really have on the order of 6
> * regions in practice */
> for (i = 0; i < mem->nregions; ++i) {
> @@ -871,6 +887,7 @@ static int set_bit_to_user(int nr, void
> void *base;
> int bit = nr + (log % PAGE_SIZE) * 8;
> int r;
> +
> r = get_user_pages_fast(log, 1, 1, &page);
> if (r < 0)
> return r;
> @@ -888,6 +905,7 @@ static int log_write(void __user *log_ba
> {
> u64 write_page = write_address / VHOST_PAGE_SIZE;
> int r;
> +
> if (!write_length)
> return 0;
> write_length += write_address % VHOST_PAGE_SIZE;
> @@ -1037,8 +1055,8 @@ static int get_indirect(struct vhost_dev
> i, count);
> return -EINVAL;
> }
> - if (unlikely(memcpy_fromiovec((unsigned char *)&desc, vq->indirect,
> - sizeof desc))) {
> + if (unlikely(memcpy_fromiovec((unsigned char *)&desc,
> + vq->indirect, sizeof desc))) {
> vq_err(vq, "Failed indirect descriptor: idx %d, %zx\n",
> i, (size_t)indirect->addr + i * sizeof desc);
> return -EINVAL;
> @@ -1317,6 +1335,7 @@ int vhost_add_used_n(struct vhost_virtqu
> void vhost_signal(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> {
> __u16 flags;
> +
> /* Flush out used index updates. This is paired
> * with the barrier that the Guest executes when enabling
> * interrupts. */
> @@ -1361,6 +1380,7 @@ bool vhost_enable_notify(struct vhost_vi
> {
> u16 avail_idx;
> int r;
> +
> if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
> return false;
> vq->used_flags &= ~VRING_USED_F_NO_NOTIFY;
> @@ -1387,6 +1407,7 @@ bool vhost_enable_notify(struct vhost_vi
> void vhost_disable_notify(struct vhost_virtqueue *vq)
> {
> int r;
> +
> if (vq->used_flags & VRING_USED_F_NO_NOTIFY)
> return;
> vq->used_flags |= VRING_USED_F_NO_NOTIFY;
^ permalink raw reply
* Re: [PATCH 1/4] slub: automatically reserve bytes at the end of slab
From: Christoph Lameter @ 2011-03-08 18:24 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Ingo Molnar, Paul E. McKenney, Pekka Enberg, Eric Dumazet,
David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
In-Reply-To: <4D6CA84B.9080606@cn.fujitsu.com>
On Tue, 1 Mar 2011, Lai Jiangshan wrote:
> So we add a field "reserved" to struct kmem_cache, when a slab
> is allocated, kmem_cache->reserved bytes are automatically reserved
> at the end of the slab for slab's metadata.
The reserved field should be exported via sysfs. Please add the code to
show it in /sys/kernel/slab/<name>
^ permalink raw reply
* Re: [PATCH] Make CUBIC Hystart more robust to RTT variations
From: Lucas Nussbaum @ 2011-03-08 18:14 UTC (permalink / raw)
To: Sangtae Ha; +Cc: WANG Cong, Injong Rhee, Netdev
In-Reply-To: <AANLkTimdpEKHfVKw+bm6OnymcnUrauU+jGOPeLzy3Q0o@mail.gmail.com>
On 08/03/11 at 11:43 -0500, Sangtae Ha wrote:
> Hi Lucas,
>
> The current packet-train threshold and the delay threshold have been tested
> with the bandwidth ranging from 10M to 400M, the RTT from 10ms to 320ms, and
> the buffer size from 10% BDP to 200% BDP and they were set conservatively to
> make it work over the network with very small buffer sizes. I will recreate
> your setup and check whether the current thresholds are too conservative and
> will come up with the patch.
I'm surprised. It's possible that a seemingly unrelated change broke it,
but it was already broken for me on 2.6.32.
I can provide access to the testbed if you want to run tests on it.
--
| Lucas Nussbaum MCF Université Nancy 2 |
| lucas.nussbaum@loria.fr LORIA / AlGorille |
| http://www.loria.fr/~lnussbau/ +33 3 54 95 86 19 |
^ permalink raw reply
* Re: [PATCH 2/4] slub,rcu: don't assume the size of struct rcu_head
From: Christoph Lameter @ 2011-03-08 18:25 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Ingo Molnar, Paul E. McKenney, Pekka Enberg, Eric Dumazet,
David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
In-Reply-To: <4D6CA852.3060303@cn.fujitsu.com>
On Tue, 1 Mar 2011, Lai Jiangshan wrote:
> - * RCU free overloads the RCU head over the LRU
> - */
> - struct rcu_head *head = (void *)&page->lru;
> + struct rcu_head *head;
> +
> + if (need_reserve_slab_rcu) {
> + int order = compound_order(page);
> + int offset = (PAGE_SIZE << order) - s->reserved;
> +
> + BUG_ON(s->reserved != sizeof(*head));
VM_BUG_ON is sufficient here I think.
Otherwise
Acked-by: Christoph Lameter <cl@linux.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 3/4] slab,rcu: don't assume the size of struct rcu_head
From: Christoph Lameter @ 2011-03-08 18:27 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Ingo Molnar, Paul E. McKenney, Pekka Enberg, Eric Dumazet,
David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
In-Reply-To: <4D6CA85B.5030107@cn.fujitsu.com>
Acked-by: Christoph Lameter <cl@linux.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [Drbd-dev] [PATCH 2/2 v2] netlink: kill eff_cap from struct netlink_skb_parms
From: Evgeniy Polyakov @ 2011-03-08 18:32 UTC (permalink / raw)
To: Patrick McHardy
Cc: Chris Wright, David Miller, linux-fbdev, netdev,
linux-security-module, dm-devel, drbd-dev
In-Reply-To: <4D764247.9080509@trash.net>
Hi Patrick.
On Tue, Mar 08, 2011 at 03:50:47PM +0100, Patrick McHardy (kaber@trash.net) wrote:
> > Yup, connector is very async at that place, but I wonder why the hell I
> > ever made that decision. I believe we can replace it with pure sync call
> > of the registered connector callback, since netlink is synchronous and
> > no one has any problem with it.
>
> Are you going to do this or do you want me to take care of it?
I will return back at the end of the week and will take care of this
problem. I will not mind if you complete it first though :)
--
Evgeniy Polyakov
^ permalink raw reply
* Re: [PATCH] ipv4: Cache source address in nexthop entries.
From: David Miller @ 2011-03-08 18:38 UTC (permalink / raw)
To: ja; +Cc: netdev
In-Reply-To: <alpine.LFD.2.00.1103081030090.1529@ja.ssi.bg>
From: Julian Anastasov <ja@ssi.bg>
Date: Tue, 8 Mar 2011 11:57:38 +0200 (EET)
> It means, even if there are addresses on the
> concerned device it does not mean the routes on this device
> are required to use prefsrc from this device. We must
> restrict the scope according to the provided for the
> route: cfg->fc_scope.
Simple to fix, I'll remember the fc_scope value in the nexthop
and use that in the address selection call.
> As the addresses have nothing to do with the link
> state, I don't think it is correct to call fib_update_nh_saddrs
> for DEV events.
fib_update_nh_saddrs() gets called for "DEV" events since those
are what are emitted when addresses are addded and removed.
^ permalink raw reply
* Re: [PATCH] tcp_cubic: enable TCP timestamps
From: Stephen Hemminger @ 2011-03-08 18:42 UTC (permalink / raw)
To: Lucas Nussbaum; +Cc: netdev, Sangtae Ha
In-Reply-To: <20110308080926.GA22641@xanadu.blop.info>
On Tue, 8 Mar 2011 09:09:26 +0100
Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:
> The Hystart slow start algorithm requires precise RTT delay measurements
> to decide when to leave slow start. However, currently, CUBIC doesn't
> enable TCP timestamps. This can cause Hystart to mis-estimate the RTT,
> and to leave slow start too early, generating bad performance since
> convergence to the optimal cwnd is slower.
>
> Timestamps are already used by TCP Illinois, LP, Vegas, Veno and Yeah.
>
> Signed-off-by: Lucas Nussbaum <lucas.nussbaum@loria.fr>
Just to explain what RTT_STAMP does. It causes the tcp receive code
to compute the rtt using high resolution clocks rather than just
jiffies. It requires access to ktime_get_real which means accessing
clock source. This is cheap for TSC, a little expensive for HPET but
expensive for PIT. I worry that enabling it may hurt regular users
on old desktops. But without it enabling RTT_STAMP, packets that
get acked in less than a jiffie (1 - 10 ms) will
Also I should have used ktime_get rather than ktime_get_real
because real time is altered by NTP and other actions.
^ permalink raw reply
* Re: ANNOUNCE: debloat-testing kernel git tree
From: Rick Jones @ 2011-03-08 18:48 UTC (permalink / raw)
To: Pavel Machek
Cc: Tianji Li, Dave T?ht, sedat.dilek-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
bloat-devel-JXvr2/1DY2fm6VMwtOF2vx4hnT+Y9+D1,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20110308065819.GA1357-+ZI9xUNit7I@public.gmane.org>
On Tue, 2011-03-08 at 07:58 +0100, Pavel Machek wrote:
> Bandwidth varies greatly with time. On cellphone, you can go from
> 5KB/sec GPRS to 300KB/sec HSDPA and back...
How quickly/often and does that trigger an event in the host that would
allow adjustment of the queue?
rick jones
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3] tcp: ioctl type SIOCOUTQNSD returns amount of data not sent
From: David Miller @ 2011-03-08 18:48 UTC (permalink / raw)
To: sledz
Cc: netdev, alan, linux-kernel, kuznet, pekkas, jmorris, yoshfuji,
kaber, m.schuknecht
In-Reply-To: <4D7619A2.4090207@dresearch.de>
From: Steffen Sledz <sledz@dresearch.de>
Date: Tue, 08 Mar 2011 12:57:22 +0100
> Anything else to do?
Be patient.
^ permalink raw reply
* Re: [rds-devel] [PATCH] rds: prevent BUG_ON triggering on congestion map updates
From: David Miller @ 2011-03-08 18:52 UTC (permalink / raw)
To: chris.mason; +Cc: nhorman, netdev, rds-devel, venkat.x.venkatsubra
In-Reply-To: <1299530363-sup-8922@think>
From: Chris Mason <chris.mason@oracle.com>
Date: Mon, 07 Mar 2011 15:41:04 -0500
> Excerpts from David Miller's message of 2011-03-07 15:27:53 -0500:
>> From: Neil Horman <nhorman@tuxdriver.com>
>> Date: Wed, 2 Mar 2011 11:28:22 -0500
>>
>> > Recently had this bug halt reported to me:
>>
>> Well, does anyone on the RDS team care about this bug fix at all?
>>
>> Stuff like this should not sit for nearly a week without any reply
>> whatsoever.
>>
>
> The patch looks good to me, but I'm surprised we haven't seen it here.
> Venkat please take a look (link to the patch below) Has it only been seen on ppc?
>
> http://permalink.gmane.org/gmane.linux.network/187933
Can I get an ACK or something, _please_?
^ permalink raw reply
* Re: [PATCH] tcp_cubic: enable TCP timestamps
From: David Miller @ 2011-03-08 18:55 UTC (permalink / raw)
To: shemminger; +Cc: lucas.nussbaum, netdev, sha2
In-Reply-To: <20110308104211.3567526f@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 8 Mar 2011 10:42:11 -0800
> On Tue, 8 Mar 2011 09:09:26 +0100
> Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:
>
>> The Hystart slow start algorithm requires precise RTT delay measurements
>> to decide when to leave slow start. However, currently, CUBIC doesn't
>> enable TCP timestamps. This can cause Hystart to mis-estimate the RTT,
>> and to leave slow start too early, generating bad performance since
>> convergence to the optimal cwnd is slower.
>>
>> Timestamps are already used by TCP Illinois, LP, Vegas, Veno and Yeah.
>>
>> Signed-off-by: Lucas Nussbaum <lucas.nussbaum@loria.fr>
>
> Just to explain what RTT_STAMP does. It causes the tcp receive code
> to compute the rtt using high resolution clocks rather than just
> jiffies. It requires access to ktime_get_real which means accessing
> clock source. This is cheap for TSC, a little expensive for HPET but
> expensive for PIT. I worry that enabling it may hurt regular users
> on old desktops. But without it enabling RTT_STAMP, packets that
> get acked in less than a jiffie (1 - 10 ms) will
>
> Also I should have used ktime_get rather than ktime_get_real
> because real time is altered by NTP and other actions.
This also means that whatever testing was originally done on Hystart
is dependent upon whatever value CONFIG_HZ had in the test kernel.
^ permalink raw reply
* Re: [Drbd-dev] [PATCH 2/2 v2] netlink: kill eff_cap from struct netlink_skb_parms
From: Patrick McHardy @ 2011-03-08 18:54 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: Chris Wright, David Miller, linux-fbdev, netdev,
linux-security-module, dm-devel, drbd-dev
In-Reply-To: <20110308183246.GA17412@ioremap.net>
Am 08.03.2011 19:32, schrieb Evgeniy Polyakov:
> Hi Patrick.
>
> On Tue, Mar 08, 2011 at 03:50:47PM +0100, Patrick McHardy (kaber@trash.net) wrote:
>>> Yup, connector is very async at that place, but I wonder why the hell I
>>> ever made that decision. I believe we can replace it with pure sync call
>>> of the registered connector callback, since netlink is synchronous and
>>> no one has any problem with it.
>>
>> Are you going to do this or do you want me to take care of it?
>
> I will return back at the end of the week and will take care of this
> problem. I will not mind if you complete it first though :)
Thanks Evgeniy, I'll give it a shot.
^ permalink raw reply
* [PATCH] cnic: fix double initalization of bnx2 based cards
From: Neil Horman @ 2011-03-08 18:56 UTC (permalink / raw)
To: netdev
Cc: Neil Horman, David S. Miller, Michael Chan, Dmitry Kravkov,
Eddie Wai, Eilon Greenstein
bnx2 cards can work with the cnic driver, but when the cnic driver detects a
bnx2 card, is_cnic_dev erroneously calls the initalization routines for both
bnx2 and bnx2x (the former being a regex subset of the later). This causes
initalization of bnx2 to unilaterally fail in the cnic driver, which, while not
catastrophic, is definately not expected. Fix this by choosing either the bnx2
or bnx2x initalization path, not both
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: David S. Miller <davem@davemloft.net>
CC: Michael Chan <mchan@broadcom.com>
CC: Dmitry Kravkov <dmitry@broadcom.com>
CC: Eddie Wai <waie@broadcom.com>
CC: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/cnic.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 271a1f0..18b59ad 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -5292,7 +5292,7 @@ static struct cnic_dev *is_cnic_dev(struct net_device *dev)
if (!strcmp(drvinfo.driver, "bnx2"))
cdev = init_bnx2_cnic(dev);
- if (!strcmp(drvinfo.driver, "bnx2x"))
+ else if (!strcmp(drvinfo.driver, "bnx2x"))
cdev = init_bnx2x_cnic(dev);
if (cdev) {
write_lock(&cnic_dev_lock);
--
1.7.4
^ permalink raw reply related
* Re: [PATCH] cnic: fix double initalization of bnx2 based cards
From: Ben Hutchings @ 2011-03-08 18:59 UTC (permalink / raw)
To: Neil Horman
Cc: netdev, David S. Miller, Michael Chan, Dmitry Kravkov, Eddie Wai,
Eilon Greenstein
In-Reply-To: <1299610588-22356-1-git-send-email-nhorman@tuxdriver.com>
On Tue, 2011-03-08 at 13:56 -0500, Neil Horman wrote:
> bnx2 cards can work with the cnic driver, but when the cnic driver detects a
> bnx2 card, is_cnic_dev erroneously calls the initalization routines for both
> bnx2 and bnx2x (the former being a regex subset of the later).
Since when does strcmp() do a regex match?
And why is a function named 'is_cnic_dev' doing initialisation, anyway?
Ben.
> This causes
> initalization of bnx2 to unilaterally fail in the cnic driver, which, while not
> catastrophic, is definately not expected. Fix this by choosing either the bnx2
> or bnx2x initalization path, not both
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: David S. Miller <davem@davemloft.net>
> CC: Michael Chan <mchan@broadcom.com>
> CC: Dmitry Kravkov <dmitry@broadcom.com>
> CC: Eddie Wai <waie@broadcom.com>
> CC: Eilon Greenstein <eilong@broadcom.com>
> ---
> drivers/net/cnic.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
> index 271a1f0..18b59ad 100644
> --- a/drivers/net/cnic.c
> +++ b/drivers/net/cnic.c
> @@ -5292,7 +5292,7 @@ static struct cnic_dev *is_cnic_dev(struct net_device *dev)
>
> if (!strcmp(drvinfo.driver, "bnx2"))
> cdev = init_bnx2_cnic(dev);
> - if (!strcmp(drvinfo.driver, "bnx2x"))
> + else if (!strcmp(drvinfo.driver, "bnx2x"))
> cdev = init_bnx2x_cnic(dev);
> if (cdev) {
> write_lock(&cnic_dev_lock);
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [rds-devel] [PATCH] rds: prevent BUG_ON triggering on congestion map updates
From: Chris Mason @ 2011-03-08 18:58 UTC (permalink / raw)
To: David Miller; +Cc: nhorman, netdev, rds-devel, venkat.x.venkatsubra
In-Reply-To: <20110308.105223.71117584.davem@davemloft.net>
Excerpts from David Miller's message of 2011-03-08 13:52:23 -0500:
> From: Chris Mason <chris.mason@oracle.com>
> Date: Mon, 07 Mar 2011 15:41:04 -0500
>
> > Excerpts from David Miller's message of 2011-03-07 15:27:53 -0500:
> >> From: Neil Horman <nhorman@tuxdriver.com>
> >> Date: Wed, 2 Mar 2011 11:28:22 -0500
> >>
> >> > Recently had this bug halt reported to me:
> >>
> >> Well, does anyone on the RDS team care about this bug fix at all?
> >>
> >> Stuff like this should not sit for nearly a week without any reply
> >> whatsoever.
> >>
> >
> > The patch looks good to me, but I'm surprised we haven't seen it here.
> > Venkat please take a look (link to the patch below) Has it only been seen on ppc?
> >
> > http://permalink.gmane.org/gmane.linux.network/187933
>
> Can I get an ACK or something, _please_?
Sorry, wasn't clear:
Acked-by: Chris Mason <chris.mason@oracle.com>
-chris
^ permalink raw reply
* Re: [PATCH] ipv4: Cache source address in nexthop entries.
From: David Miller @ 2011-03-08 19:03 UTC (permalink / raw)
To: ja; +Cc: netdev
In-Reply-To: <20110308.103801.193702247.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Tue, 08 Mar 2011 10:38:01 -0800 (PST)
> From: Julian Anastasov <ja@ssi.bg>
> Date: Tue, 8 Mar 2011 11:57:38 +0200 (EET)
>
>> It means, even if there are addresses on the
>> concerned device it does not mean the routes on this device
>> are required to use prefsrc from this device. We must
>> restrict the scope according to the provided for the
>> route: cfg->fc_scope.
>
> Simple to fix, I'll remember the fc_scope value in the nexthop
> and use that in the address selection call.
I'll push the following into net-next-2.6, thanks Julian:
--------------------
ipv4: Fix scope value used in route src-address caching.
We have to use cfg->fc_scope not the final nh_scope value.
Reported-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/net/ip_fib.h | 1 +
net/ipv4/fib_semantics.c | 5 +++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 0e14083..3f6c943 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -51,6 +51,7 @@ struct fib_nh {
struct fib_info *nh_parent;
unsigned nh_flags;
unsigned char nh_scope;
+ unsigned char nh_cfg_scope;
#ifdef CONFIG_IP_ROUTE_MULTIPATH
int nh_weight;
int nh_power;
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 952c737..d73d758 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -854,9 +854,10 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
}
change_nexthops(fi) {
+ nexthop_nh->nh_cfg_scope = cfg->fc_scope;
nexthop_nh->nh_saddr = inet_select_addr(nexthop_nh->nh_dev,
nexthop_nh->nh_gw,
- nexthop_nh->nh_scope);
+ nexthop_nh->nh_cfg_scope);
} endfor_nexthops(fi)
link_it:
@@ -1141,7 +1142,7 @@ void fib_update_nh_saddrs(struct net_device *dev)
continue;
nh->nh_saddr = inet_select_addr(nh->nh_dev,
nh->nh_gw,
- nh->nh_scope);
+ nh->nh_cfg_scope);
}
}
--
1.7.4.1
^ permalink raw reply related
* Re: [net-next-2.6 00/14][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2011-03-08 19:11 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips
In-Reply-To: <1299551108-23663-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 7 Mar 2011 18:24:54 -0800
> The following series contains fixes and cleanups for e1000e and ixgbe, as
> well as support for FCoE DDP in target mode for ixgbe. In igb, added
> additional stats for OS2BMC feature.
>
> The following are changes since commit 541ac7c9b30ee2ff84ad87f27e0bc069e143afb5:
> bonding: COW before overwriting the destination MAC address
>
> and are available in the git repository at:
> master.kernel.org:/pub/scm/linux/kernel/git/jkirsher/net-next-2.6 master
Pulled.
Please address the feedback given on the lack of docbook descriptions for
all of those fcoe device ops.
^ permalink raw reply
* Re: [PATCH] cnic: fix double initalization of bnx2 based cards
From: Michael Chan @ 2011-03-08 19:07 UTC (permalink / raw)
To: Ben Hutchings
Cc: Neil Horman, netdev@vger.kernel.org, David S. Miller,
Dmitry Kravkov, Eddie Wai, Eilon Greenstein
In-Reply-To: <1299610768.4957.1.camel@bwh-desktop>
On Tue, 2011-03-08 at 10:59 -0800, Ben Hutchings wrote:
> On Tue, 2011-03-08 at 13:56 -0500, Neil Horman wrote:
> > bnx2 cards can work with the cnic driver, but when the cnic driver detects a
> > bnx2 card, is_cnic_dev erroneously calls the initalization routines for both
> > bnx2 and bnx2x (the former being a regex subset of the later).
>
> Since when does strcmp() do a regex match?
Yeah, strcmp() does NULL-terminated string compare, right?
>
> And why is a function named 'is_cnic_dev' doing initialisation, anyway?
Just data structure init if a supported device is detected.
>
> Ben.
>
> > This causes
> > initalization of bnx2 to unilaterally fail in the cnic driver, which, while not
> > catastrophic, is definately not expected. Fix this by choosing either the bnx2
> > or bnx2x initalization path, not both
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: David S. Miller <davem@davemloft.net>
> > CC: Michael Chan <mchan@broadcom.com>
> > CC: Dmitry Kravkov <dmitry@broadcom.com>
> > CC: Eddie Wai <waie@broadcom.com>
> > CC: Eilon Greenstein <eilong@broadcom.com>
> > ---
> > drivers/net/cnic.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
> > index 271a1f0..18b59ad 100644
> > --- a/drivers/net/cnic.c
> > +++ b/drivers/net/cnic.c
> > @@ -5292,7 +5292,7 @@ static struct cnic_dev *is_cnic_dev(struct net_device *dev)
> >
> > if (!strcmp(drvinfo.driver, "bnx2"))
> > cdev = init_bnx2_cnic(dev);
> > - if (!strcmp(drvinfo.driver, "bnx2x"))
> > + else if (!strcmp(drvinfo.driver, "bnx2x"))
> > cdev = init_bnx2x_cnic(dev);
> > if (cdev) {
> > write_lock(&cnic_dev_lock);
>
^ permalink raw reply
* Re: [PATCH] tcp_cubic: enable TCP timestamps
From: Sangtae Ha @ 2011-03-08 19:15 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, lucas.nussbaum, netdev
In-Reply-To: <20110308.105503.104060436.davem@davemloft.net>
Yes. I remember that CONFIG_HZ was 1000 at that time and the value of
CONFIG_HZ could affect the algorithm.
I don't think HyStart needs this extra RTT_STAMP since we only need a
rough delay estimate.
Let me check HyStart with the latest git and with different CONFIG_HZ values.
Sangtae
On Tue, Mar 8, 2011 at 1:55 PM, David Miller <davem@davemloft.net> wrote:
>
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Tue, 8 Mar 2011 10:42:11 -0800
>
> > On Tue, 8 Mar 2011 09:09:26 +0100
> > Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:
> >
> >> The Hystart slow start algorithm requires precise RTT delay measurements
> >> to decide when to leave slow start. However, currently, CUBIC doesn't
> >> enable TCP timestamps. This can cause Hystart to mis-estimate the RTT,
> >> and to leave slow start too early, generating bad performance since
> >> convergence to the optimal cwnd is slower.
> >>
> >> Timestamps are already used by TCP Illinois, LP, Vegas, Veno and Yeah.
> >>
> >> Signed-off-by: Lucas Nussbaum <lucas.nussbaum@loria.fr>
> >
> > Just to explain what RTT_STAMP does. It causes the tcp receive code
> > to compute the rtt using high resolution clocks rather than just
> > jiffies. It requires access to ktime_get_real which means accessing
> > clock source. This is cheap for TSC, a little expensive for HPET but
> > expensive for PIT. I worry that enabling it may hurt regular users
> > on old desktops. But without it enabling RTT_STAMP, packets that
> > get acked in less than a jiffie (1 - 10 ms) will
> >
> > Also I should have used ktime_get rather than ktime_get_real
> > because real time is altered by NTP and other actions.
>
> This also means that whatever testing was originally done on Hystart
> is dependent upon whatever value CONFIG_HZ had in the test kernel.
^ permalink raw reply
* Re: [PATCH] tcp_cubic: enable TCP timestamps
From: Stephen Hemminger @ 2011-03-08 19:17 UTC (permalink / raw)
To: David Miller; +Cc: lucas.nussbaum, netdev, sha2
In-Reply-To: <20110308.105503.104060436.davem@davemloft.net>
On Tue, 08 Mar 2011 10:55:03 -0800 (PST)
David Miller <davem@davemloft.net> wrote:
Complete the sentence...
> on old desktops.
But without it enabling RTT_STAMP, packets that
get acked in less than a jiffie (1 - 10 ms) will
be processed with no rtt value (-1); this causes hystart
never to be invoked on local connections.
Maybe the following would be better, it passes 0 as RTT
when not using RTT_STAMP for quick packets.
It turns out the Cubic ends up converting rtt_us back
to jiffies anyway, so RTT_STAMP is not really needed.
----
Subject: tcp: fix RTT for quick packets in congestion control
In the congestion control interface, the callback for each ACK
includes an estimated round trip time in microseconds.
Some algorithms need high resolution (Vegas style) but most only
need jiffie resolution. If RTT is not accurate (like a retransmission)
-1 is used as a flag value.
When doing coarse resolution if RTT is less than a a jiffie
then 0 should be returned rather than no estimate. Otherwise algorithms
that expect good ack's to trigger slow start (like CUBIC Hystart)
will be confused.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/ipv4/tcp_input.c 2011-03-08 11:11:26.093183654 -0800
+++ b/net/ipv4/tcp_input.c 2011-03-08 11:11:46.641404939 -0800
@@ -3350,7 +3350,7 @@ static int tcp_clean_rtx_queue(struct so
net_invalid_timestamp()))
rtt_us = ktime_us_delta(ktime_get_real(),
last_ackt);
- else if (ca_seq_rtt > 0)
+ else if (ca_seq_rtt >= 0)
rtt_us = jiffies_to_usecs(ca_seq_rtt);
}
^ 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