* Re: [PATCH] bgmac: don't update slot on skb alloc/dma mapping error
From: David Miller @ 2013-10-29 19:39 UTC (permalink / raw)
To: nlhintz; +Cc: zajec5, netdev
In-Reply-To: <BLU0-SMTP200803D333FF3A2F15AB82DAC090@phx.gbl>
From: Nathan Hintz <nlhintz@hotmail.com>
Date: Tue, 29 Oct 2013 01:22:55 -0700
> On Tue, 29 Oct 2013 07:52:58 +0100
> Rafał Miłecki <zajec5@gmail.com> wrote:
>
>> 2013/10/29 Nathan Hintz <nlhintz@hotmail.com>:
>> > Don't update the slot in "bgmac_dma_rx_skb_for_slot" unless both the
>> > skb alloc and dma mapping are successful; and free the newly allocated
>> > skb if a dma mapping error occurs. This will prevent an skb leak upon
>> > returning when an error occurs.
>>
>> In case of bgmac_dma_rx_skb_for_slot failure we're giving up anyway
>> (and freeing everything), but with your patch code is simpler to
>> understand, so I'm OK with that.
>>
>> Acked-by: Rafał Miłecki <zajec5@gmail.com>
>>
>
> I might be misunderstanding; but it in the case of failure, it appeared to me
> that the currently received packet was dropped and the old skb would continue
> to be assigned to the slot and would be used to receive future packets (this
> would continue until bgmac_dma_rx_skb_for_slot was successful).
That's exactly the wanted, and most desirable, behavior for any network
driver.
If you can't allocate a new SKB, reuse the old one, because the worst
thing you can do is prioritize packet reception over making sure the
device doesn't end up with no RX slots to DMA into.
^ permalink raw reply
* Re: [PATCH net-next] ipv4: introduce new IP_MTU_DISCOVER mode IP_PMTUDISC_INTERFACE
From: David Miller @ 2013-10-29 19:38 UTC (permalink / raw)
To: fweimer; +Cc: hannes, netdev
In-Reply-To: <526F6583.4000506@redhat.com>
From: Florian Weimer <fweimer@redhat.com>
Date: Tue, 29 Oct 2013 08:36:35 +0100
> You could make the path MTU dependent on the protocol (which would
> even be the correct solution from a technical point of view) and use
> validation for TCP and UDP, but that's a fairly invasive change for
> such relatively minor functionality.
The code pretty much already does this, we cannot even find the
exact route without being able to look up the socket right now.
We need the full flow info.
There is a backup path, but you could make the new (non-default)
behavior be to never trust that path for protocols like UDP and
TCP.
I don't buy any of your arguments against my suggestions so far.
^ permalink raw reply
* Re: [Bug 64011] New: Link-local addresses are used to go outside instead of assigned IPv6 routable address
From: Hannes Frederic Sowa @ 2013-10-29 19:36 UTC (permalink / raw)
To: luzemario; +Cc: netdev
In-Reply-To: <bug-64011-16487@https.bugzilla.kernel.org/>
Hi!
On Tue, Oct 29, 2013 at 01:36:37PM +0000, bugzilla-daemon@bugzilla.kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=64011
>
> Bug ID: 64011
> Summary: Link-local addresses are used to go outside instead of
> assigned IPv6 routable address
> Product: Networking
> Version: 2.5
> Kernel Version: 3.11.6-200 and earlier
> Hardware: All
> OS: Linux
> Tree: Mainline
> Status: NEW
> Severity: high
> Priority: P1
> Component: IPV6
> Assignee: yoshfuji@linux-ipv6.org
> Reporter: luzemario@gmail.com
> Regression: No
>
> Created attachment 112651
> --> https://bugzilla.kernel.org/attachment.cgi?id=112651&action=edit
> pfSense screenshot of blocked link-local requests to outside
>
> Sometimes kernel tries to estabilish a IPv6 connection to external global
> routable addresses using [FE80::] link-local addresses, instead of using the
> learned DHCPv6 global-unicast address.
>
> In the attached screenshot, the machine got an IPv6 address in the
> [2001:1291:200::] range, and was correctly configured.
>
> Since link-local should not traverse network segments, I am getting lots of
> events in my firewall from several kernel versions of different distros (see
> attachment).
>
> At the time when the kernel attempts to use the link-local address, there is a
> small delay (around 1s) to open IPv6 pages. It appears kernel tries to reach a
> remote machine using link-local address. When it fails, the kernel uses the
> assigned global routable address.
>
> This issue was seen on several major distros, as Fedora, Ubuntu, Mageia, etc.
> yet on the distro's later updates.
Could you provide ip -6 a l, ip -6 r l, ip -6 n l and a copy of /proc/net/ipv6_route
(please check that the lines are not broken up)? The configuration of the
router advertisment daemon on your gateway would be interesting, too.
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH net-next] 3c515: Fix warning when building
From: Ben Hutchings @ 2013-10-29 19:31 UTC (permalink / raw)
To: David Miller, ricec2; +Cc: netdev
In-Reply-To: <20131029.000316.767189982282205506.davem@davemloft.net>
On Tue, 2013-10-29 at 00:03 -0400, David Miller wrote:
> From: Colin Rice <ricec2@rpi.edu>
> Date: Fri, 25 Oct 2013 21:59:46 +0000
>
> > - outl((int) (skb->data), ioaddr + Wn7_MasterAddr);
> > + outl((size_t) (skb->data), ioaddr + Wn7_MasterAddr);
>
> outl() takes an "int" not a "size_t". You're avoiding the warning,
> but on 64-bit, for example, you're silently allowing the compiler
> to accept chopping off the top 32-bits of the 64-bit pointer address
> off.
>
> The warning is valid, this code won't work in certain environments,
> and killing the warning is just papering over the problem such that
> it will never get addressed properly.
>
> I'm not applying patches like this, sorry.
The driver is writing a virtual address as the DMA address. The
hardware is an ISA device and will presumably ignore the top 8 bits,
thus doing the virt-to-bus conversion for us. Very efficient - so long
as this runs on a PC with no more than 16 MB RAM.
For other chips with a DMA descriptor ring, the driver uses
isa_virt_to_bus() which is less efficient but no more correct.
Maybe the driver should just be deleted, as it doesn't look like it's
actually usable any more.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
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: [PATCH net-next] virtio_net: migrate mergeable rx buffers to page frag allocators
From: Michael S. Tsirkin @ 2013-10-29 19:17 UTC (permalink / raw)
To: Michael Dalton
Cc: netdev, Eric Dumazet, lf-virt, Daniel Borkmann, David S. Miller
In-Reply-To: <CANJ5vP+=wCrzJ3Fdg0_5ZTTT-V-fw2AXUG=EgwsUcxXO=UpMgw@mail.gmail.com>
On Tue, Oct 29, 2013 at 12:05:27PM -0700, Michael Dalton wrote:
> Agreed Eric, the buffer size should be increased so that we can accommodate a
> MTU-sized packet + mergeable virtio net header in a single buffer. I will send
> a patch to fix shortly cleaning up the #define headers as Rusty indicated and
> increasing the buffer size slightly by VirtioNet header size bytes per Eric.
>
> Jason, I'll followup with you directly - I'd like to know your exact workload
> (single steam or multi-stream netperf?), VM configuration, etc, and also see if
> the nit that Erichas pointed out affects your results. It is also
> worth noting that
> we may want to tune the queue sizes for your benchmarks, e.g, by reducing
> buffer size from 4KB to MTU-sized but keeping queue length constant, we're
> implicitly decreasing the number of bytes stored in the VirtioQueue for the
> VirtioNet device, so increasing the queue size may help.
>
> Best,
>
> Mike
Well we have 256 descriptors per queue, each descriptor is 16 bytes
already and they have to be physically contigious.
I don't think we can easily increase the queue size much more without
risking memory allocation failures on busy systems.
I guess one approach is to do something like:
if (queue size > 1024)
use small buffers
else
use 4K buffers.
That would reduce the risk of regressions for existing users.
--
MST
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: migrate mergeable rx buffers to page frag allocators
From: Michael S. Tsirkin @ 2013-10-29 19:12 UTC (permalink / raw)
To: David Miller
Cc: ffusco, mwdalton, eric.dumazet, netdev, dborkman, virtualization,
edumazet
In-Reply-To: <20131028.235721.919284648105056096.davem@davemloft.net>
On Mon, Oct 28, 2013 at 11:57:21PM -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 28 Oct 2013 16:19:49 -0700
>
> > On Mon, 2013-10-28 at 15:44 -0700, Michael Dalton wrote:
> >> The virtio_net driver's mergeable receive buffer allocator
> >> uses 4KB packet buffers. For MTU-sized traffic, SKB truesize
> >> is > 4KB but only ~1500 bytes of the buffer is used to store
> >> packet data, reducing the effective TCP window size
> >> substantially. This patch addresses the performance concerns
> >> with mergeable receive buffers by allocating MTU-sized packet
> >> buffers using page frag allocators. If more than MAX_SKB_FRAGS
> >> buffers are needed, the SKB frag_list is used.
> >>
> >> Signed-off-by: Michael Dalton <mwdalton@google.com>
> >> ---
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> >
> > Daniel & Francesco, this should address the performance problem you
> > tried to address with ("tcp: rcvbuf autotuning improvements")
> >
> > ( http://www.spinics.net/lists/netdev/msg252642.html )
>
> Applied, thanks everyone.
Hmm that was very quick. It *should* fix a bug, great.
But how about we wait for some Tested-by reports that it actually does?
--
MST
^ permalink raw reply
* [patch] libertas: potential oops in debugfs
From: Dan Carpenter @ 2013-10-29 19:06 UTC (permalink / raw)
To: John W. Linville
Cc: libertas-dev, linux-wireless, netdev, security, Nico Golde,
Fabian Yamaguchi
In-Reply-To: <20131025144452.GA28451@ngolde.de>
If we do a zero size write then it will oops. This can only be
triggered by root.
Reported-by: Nico Golde <nico@ngolde.de>
Reported-by: Fabian Yamaguchi <fabs@goesec.de>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/net/wireless/libertas/debugfs.c b/drivers/net/wireless/libertas/debugfs.c
index 668dd27..a148f14 100644
--- a/drivers/net/wireless/libertas/debugfs.c
+++ b/drivers/net/wireless/libertas/debugfs.c
@@ -913,6 +913,9 @@ static ssize_t lbs_debugfs_write(struct file *f, const char __user *buf,
char *p2;
struct debug_data *d = f->private_data;
+ if (cnt == 0)
+ return 0;
+
pdata = kmalloc(cnt, GFP_KERNEL);
if (pdata == NULL)
return 0;
^ permalink raw reply related
* Re: [PATCH net-next] virtio_net: migrate mergeable rx buffers to page frag allocators
From: Michael S. Tsirkin @ 2013-10-29 19:05 UTC (permalink / raw)
To: Eric Northup
Cc: Michael Dalton, netdev, Eric Dumazet, lf-virt, Daniel Borkmann,
David S. Miller
In-Reply-To: <CAG7+5M1VFSZciaZf+Y_yzm=aq1Z4fZ-QgACN2srnC9twOWhVWw@mail.gmail.com>
On Tue, Oct 29, 2013 at 06:44:40PM +0000, Eric Northup wrote:
> On Mon, Oct 28, 2013 at 10:44 PM, Michael Dalton <mwdalton@google.com> wrote:
> > The virtio_net driver's mergeable receive buffer allocator
> > uses 4KB packet buffers. For MTU-sized traffic, SKB truesize
> > is > 4KB but only ~1500 bytes of the buffer is used to store
> > packet data, reducing the effective TCP window size
> > substantially. This patch addresses the performance concerns
> > with mergeable receive buffers by allocating MTU-sized packet
> > buffers using page frag allocators. If more than MAX_SKB_FRAGS
> > buffers are needed, the SKB frag_list is used.
>
> This looks really good, just one nit below -- should we add MTU-sized
> packets, or add MTU+sizeof(virtnet_hdr)-sized packets?
>
> Reviewed-by: Eric Northup <digitaleric@google.com>
Yea, except it increases the typical number of packets per TSO
packet by a factor of 2.5, so it's almost sure to hurt performance
for some workloads.
Maybe the right thing to do is to pass a mix of small and big
buffers to the device. Device can pick the right size depending
on the size of the packet it has.
What to do for existing devices? I'm not sure. tcp rcv buf is at
least tunable. I guess we could choose the buffer size depending on
default rcv buf. And/or maybe a module parameter ...
> >
> > Signed-off-by: Michael Dalton <mwdalton@google.com>
> > ---
> > drivers/net/virtio_net.c | 164 ++++++++++++++++++++++++++++++-----------------
> > 1 file changed, 106 insertions(+), 58 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 9fbdfcd..113ee93 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -124,6 +124,11 @@ struct virtnet_info {
> > /* Lock for config space updates */
> > struct mutex config_lock;
> >
> > + /* Page_frag for GFP_KERNEL packet buffer allocation when we run
> > + * low on memory.
> > + */
> > + struct page_frag alloc_frag;
> > +
> > /* Does the affinity hint is set for virtqueues? */
> > bool affinity_hint_set;
> >
> > @@ -217,33 +222,18 @@ static void skb_xmit_done(struct virtqueue *vq)
> > netif_wake_subqueue(vi->dev, vq2txq(vq));
> > }
> >
> > -static void set_skb_frag(struct sk_buff *skb, struct page *page,
> > - unsigned int offset, unsigned int *len)
> > -{
> > - int size = min((unsigned)PAGE_SIZE - offset, *len);
> > - int i = skb_shinfo(skb)->nr_frags;
> > -
> > - __skb_fill_page_desc(skb, i, page, offset, size);
> > -
> > - skb->data_len += size;
> > - skb->len += size;
> > - skb->truesize += PAGE_SIZE;
> > - skb_shinfo(skb)->nr_frags++;
> > - skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
> > - *len -= size;
> > -}
> > -
> > /* Called from bottom half context */
> > static struct sk_buff *page_to_skb(struct receive_queue *rq,
> > - struct page *page, unsigned int len)
> > + struct page *page, unsigned int offset,
> > + unsigned int len, unsigned int truesize)
> > {
> > struct virtnet_info *vi = rq->vq->vdev->priv;
> > struct sk_buff *skb;
> > struct skb_vnet_hdr *hdr;
> > - unsigned int copy, hdr_len, offset;
> > + unsigned int copy, hdr_len, hdr_padded_len;
> > char *p;
> >
> > - p = page_address(page);
> > + p = page_address(page) + offset;
> >
> > /* copy small packet so we can reuse these pages for small data */
> > skb = netdev_alloc_skb_ip_align(vi->dev, GOOD_COPY_LEN);
> > @@ -254,16 +244,17 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
> >
> > if (vi->mergeable_rx_bufs) {
> > hdr_len = sizeof hdr->mhdr;
> > - offset = hdr_len;
> > + hdr_padded_len = sizeof hdr->mhdr;
> > } else {
> > hdr_len = sizeof hdr->hdr;
> > - offset = sizeof(struct padded_vnet_hdr);
> > + hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > }
> >
> > memcpy(hdr, p, hdr_len);
> >
> > len -= hdr_len;
> > - p += offset;
> > + offset += hdr_padded_len;
> > + p += hdr_padded_len;
> >
> > copy = len;
> > if (copy > skb_tailroom(skb))
> > @@ -273,6 +264,14 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
> > len -= copy;
> > offset += copy;
> >
> > + if (vi->mergeable_rx_bufs) {
> > + if (len)
> > + skb_add_rx_frag(skb, 0, page, offset, len, truesize);
> > + else
> > + put_page(page);
> > + return skb;
> > + }
> > +
> > /*
> > * Verify that we can indeed put this data into a skb.
> > * This is here to handle cases when the device erroneously
> > @@ -284,9 +283,12 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
> > dev_kfree_skb(skb);
> > return NULL;
> > }
> > -
> > + BUG_ON(offset >= PAGE_SIZE);
> > while (len) {
> > - set_skb_frag(skb, page, offset, &len);
> > + unsigned int frag_size = min((unsigned)PAGE_SIZE - offset, len);
> > + skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page, offset,
> > + frag_size, truesize);
> > + len -= frag_size;
> > page = (struct page *)page->private;
> > offset = 0;
> > }
> > @@ -297,33 +299,52 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
> > return skb;
> > }
> >
> > -static int receive_mergeable(struct receive_queue *rq, struct sk_buff *skb)
> > +static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
> > {
> > - struct skb_vnet_hdr *hdr = skb_vnet_hdr(skb);
> > + struct skb_vnet_hdr *hdr = skb_vnet_hdr(head_skb);
> > + struct sk_buff *curr_skb = head_skb;
> > + char *buf;
> > struct page *page;
> > - int num_buf, i, len;
> > + int num_buf, len;
> >
> > num_buf = hdr->mhdr.num_buffers;
> > while (--num_buf) {
> > - i = skb_shinfo(skb)->nr_frags;
> > - if (i >= MAX_SKB_FRAGS) {
> > - pr_debug("%s: packet too long\n", skb->dev->name);
> > - skb->dev->stats.rx_length_errors++;
> > - return -EINVAL;
> > - }
> > - page = virtqueue_get_buf(rq->vq, &len);
> > - if (!page) {
> > + int num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
> > + buf = virtqueue_get_buf(rq->vq, &len);
> > + if (unlikely(!buf)) {
> > pr_debug("%s: rx error: %d buffers missing\n",
> > - skb->dev->name, hdr->mhdr.num_buffers);
> > - skb->dev->stats.rx_length_errors++;
> > + head_skb->dev->name, hdr->mhdr.num_buffers);
> > + head_skb->dev->stats.rx_length_errors++;
> > return -EINVAL;
> > }
> > -
> > - if (len > PAGE_SIZE)
> > - len = PAGE_SIZE;
> > -
> > - set_skb_frag(skb, page, 0, &len);
> > -
> > + if (unlikely(len > MAX_PACKET_LEN)) {
> > + pr_debug("%s: rx error: merge buffer too long\n",
> > + head_skb->dev->name);
> > + len = MAX_PACKET_LEN;
> > + }
> > + if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) {
> > + struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC);
> > + if (unlikely(!nskb)) {
> > + head_skb->dev->stats.rx_dropped++;
> > + return -ENOMEM;
> > + }
> > + if (curr_skb == head_skb)
> > + skb_shinfo(curr_skb)->frag_list = nskb;
> > + else
> > + curr_skb->next = nskb;
> > + curr_skb = nskb;
> > + head_skb->truesize += nskb->truesize;
> > + num_skb_frags = 0;
> > + }
> > + if (curr_skb != head_skb) {
> > + head_skb->data_len += len;
> > + head_skb->len += len;
> > + head_skb->truesize += MAX_PACKET_LEN;
> > + }
> > + page = virt_to_head_page(buf);
> > + skb_add_rx_frag(curr_skb, num_skb_frags, page,
> > + buf - (char *)page_address(page), len,
> > + MAX_PACKET_LEN);
> > --rq->num;
> > }
> > return 0;
> > @@ -341,8 +362,10 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> > if (unlikely(len < sizeof(struct virtio_net_hdr) + ETH_HLEN)) {
> > pr_debug("%s: short packet %i\n", dev->name, len);
> > dev->stats.rx_length_errors++;
> > - if (vi->mergeable_rx_bufs || vi->big_packets)
> > + if (vi->big_packets)
> > give_pages(rq, buf);
> > + else if (vi->mergeable_rx_bufs)
> > + put_page(virt_to_head_page(buf));
> > else
> > dev_kfree_skb(buf);
> > return;
> > @@ -352,19 +375,28 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> > skb = buf;
> > len -= sizeof(struct virtio_net_hdr);
> > skb_trim(skb, len);
> > + } else if (vi->mergeable_rx_bufs) {
> > + struct page *page = virt_to_head_page(buf);
> > + skb = page_to_skb(rq, page,
> > + (char *)buf - (char *)page_address(page),
> > + len, MAX_PACKET_LEN);
> > + if (unlikely(!skb)) {
> > + dev->stats.rx_dropped++;
> > + put_page(page);
> > + return;
> > + }
> > + if (receive_mergeable(rq, skb)) {
> > + dev_kfree_skb(skb);
> > + return;
> > + }
> > } else {
> > page = buf;
> > - skb = page_to_skb(rq, page, len);
> > + skb = page_to_skb(rq, page, 0, len, PAGE_SIZE);
> > if (unlikely(!skb)) {
> > dev->stats.rx_dropped++;
> > give_pages(rq, page);
> > return;
> > }
> > - if (vi->mergeable_rx_bufs)
> > - if (receive_mergeable(rq, skb)) {
> > - dev_kfree_skb(skb);
> > - return;
> > - }
> > }
> >
> > hdr = skb_vnet_hdr(skb);
> > @@ -501,18 +533,28 @@ static int add_recvbuf_big(struct receive_queue *rq, gfp_t gfp)
> >
> > static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
> > {
> > - struct page *page;
> > + struct virtnet_info *vi = rq->vq->vdev->priv;
> > + char *buf = NULL;
> > int err;
> >
> > - page = get_a_page(rq, gfp);
> > - if (!page)
> > + if (gfp & __GFP_WAIT) {
>
> Shouldn't this be MAX_PACKET_LEN + sizeof(virtio_net_hdr_mrg_rxbuf) ?
>
> That way, we can receive an MTU-sized packet while only consuming a
> single queue entry.
>
> > + if (skb_page_frag_refill(MAX_PACKET_LEN, &vi->alloc_frag,
> > + gfp)) {
> > + buf = (char *)page_address(vi->alloc_frag.page) +
> > + vi->alloc_frag.offset;
> > + get_page(vi->alloc_frag.page);
> > + vi->alloc_frag.offset += MAX_PACKET_LEN;
> > + }
> > + } else {
> > + buf = netdev_alloc_frag(MAX_PACKET_LEN);
> > + }
> > + if (!buf)
> > return -ENOMEM;
> >
> > - sg_init_one(rq->sg, page_address(page), PAGE_SIZE);
> > -
> > - err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, page, gfp);
> > + sg_init_one(rq->sg, buf, MAX_PACKET_LEN);
> > + err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, buf, gfp);
> > if (err < 0)
> > - give_pages(rq, page);
> > + put_page(virt_to_head_page(buf));
> >
> > return err;
> > }
> > @@ -1343,8 +1385,10 @@ static void free_unused_bufs(struct virtnet_info *vi)
> > struct virtqueue *vq = vi->rq[i].vq;
> >
> > while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> > - if (vi->mergeable_rx_bufs || vi->big_packets)
> > + if (vi->big_packets)
> > give_pages(&vi->rq[i], buf);
> > + else if (vi->mergeable_rx_bufs)
> > + put_page(virt_to_head_page(buf));
> > else
> > dev_kfree_skb(buf);
> > --vi->rq[i].num;
> > @@ -1650,6 +1694,8 @@ free_recv_bufs:
> > free_vqs:
> > cancel_delayed_work_sync(&vi->refill);
> > virtnet_del_vqs(vi);
> > + if (vi->alloc_frag.page)
> > + put_page(vi->alloc_frag.page);
> > free_index:
> > free_percpu(vi->vq_index);
> > free_stats:
> > @@ -1685,6 +1731,8 @@ static void virtnet_remove(struct virtio_device *vdev)
> > unregister_netdev(vi->dev);
> >
> > remove_vq_common(vi);
> > + if (vi->alloc_frag.page)
> > + put_page(vi->alloc_frag.page);
> >
> > flush_work(&vi->config_work);
> >
> > --
> > 1.8.4.1
> >
> > _______________________________________________
> > Virtualization mailing list
> > Virtualization@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: migrate mergeable rx buffers to page frag allocators
From: Michael Dalton @ 2013-10-29 19:05 UTC (permalink / raw)
To: Eric Northup
Cc: David S. Miller, Michael S. Tsirkin, netdev, Daniel Borkmann,
lf-virt, Eric Dumazet
In-Reply-To: <CAG7+5M1VFSZciaZf+Y_yzm=aq1Z4fZ-QgACN2srnC9twOWhVWw@mail.gmail.com>
Agreed Eric, the buffer size should be increased so that we can accommodate a
MTU-sized packet + mergeable virtio net header in a single buffer. I will send
a patch to fix shortly cleaning up the #define headers as Rusty indicated and
increasing the buffer size slightly by VirtioNet header size bytes per Eric.
Jason, I'll followup with you directly - I'd like to know your exact workload
(single steam or multi-stream netperf?), VM configuration, etc, and also see if
the nit that Erichas pointed out affects your results. It is also
worth noting that
we may want to tune the queue sizes for your benchmarks, e.g, by reducing
buffer size from 4KB to MTU-sized but keeping queue length constant, we're
implicitly decreasing the number of bytes stored in the VirtioQueue for the
VirtioNet device, so increasing the queue size may help.
Best,
Mike
^ permalink raw reply
* Re: [patch iproute2] iplink: add support for bonding netlink
From: Scott Feldman @ 2013-10-29 19:00 UTC (permalink / raw)
To: netdev
In-Reply-To: <1382111401-1233-1-git-send-email-jiri@resnulli.us>
Jiri Pirko <jiri <at> resnulli.us> writes:
> + } else if (matches(*argv, "active_slave") == 0) {
> + NEXT_ARG();
> + ifindex = if_nametoindex(*argv);
> + if (!ifindex)
> + return -1;
> + addattr32(n, 1024, IFLA_BOND_ACTIVE_SLAVE, ifindex);
> + } else if (matches(*argv, "clear_active_slave") == 0) {
> + addattr32(n, 1024, IFLA_BOND_ACTIVE_SLAVE, 0);
How do active_slave and clear_active_slave work from the ip link cmd line
when bond is added, but doesn't have any slaves yet? I did:
ip link add bond1 type bond mode 1
ip link set dev eth1 master bond1
ip link set dev eth2 master bond1
Then tried:
ip link add bond1 type bond active_slave eth1
RTNETLINK answers: File exists
Or:
ip link set dev bond1 active_slave eth1
Error: either "dev" is duplicate, or "active_slave" is a garbage.
I must be dense but I can't figure how you can set active_slave before
slaves have been added to bond, or even after slaves have been added
to bond.
> diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
> index 8b68c78..1825dc5 100644
> --- a/man/man8/ip-link.8.in
> +++ b/man/man8/ip-link.8.in
> <at> <at> -51,6 +51,7 <at> <at> ip-link \- network device configuration
> .ti -8
> .IR TYPE " := [ "
> .BR bridge " | "
> +.BR bond " ]"
That should be " | "?
With patch applied, the man page shows:
TYPE := [ bridge | bond ] can | dummy | ifb | ipoib | macvlan | ...
-scott
^ permalink raw reply
* Re: [PATCH net-next] xen-netback: allocate xenvif arrays using vzalloc.
From: Joby Poriyath @ 2013-10-29 18:46 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, wei.liu2, ian.campbell, xen-devel, andrew.bennieston,
david.vrabel, malcolm.crossley
In-Reply-To: <1383061430.5464.41.camel@edumazet-glaptop.roam.corp.google.com>
On Tue, Oct 29, 2013 at 08:43:50AM -0700, Eric Dumazet wrote:
> On Tue, 2013-10-29 at 15:27 +0000, Joby Poriyath wrote:
> > This will reduce memory pressure when allocating struct xenvif.
> >
> > The size of xenvif struct has increased from 168 to 36632 bytes (on x86-32).
> > See commit b3f980bd827e6e81a050c518d60ed7811a83061d. This resulted in
> > occasional netdev allocation failure in dom0 with 752MiB RAM, due to
> > fragmented memory.
>
> This looks overkill.
>
> Replacing a single allocation of ~36 KB into 5 vmalloc() looks like you
> did not really tried other things...
>
> This should be done generically in alloc_netdev_mqs()
Sorry Eric, I didn't quite understand how this can be generically done.
The netback interfaces are tied to the Xen guests (VMs) and these are created
when guests are started and deleted when guest are halted.
>
> Take a look at commit 60877a32bce00041
> ("net: allow large number of tx queues")
>
I could try allocating using kmalloc and if that fails, then fall back
to vmalloc.
Thanks,
Joby
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: migrate mergeable rx buffers to page frag allocators
From: Eric Northup @ 2013-10-29 18:44 UTC (permalink / raw)
To: Michael Dalton
Cc: Michael S. Tsirkin, netdev, Eric Dumazet, lf-virt,
Daniel Borkmann, David S. Miller
In-Reply-To: <1383000258-1458-1-git-send-email-mwdalton@google.com>
On Mon, Oct 28, 2013 at 10:44 PM, Michael Dalton <mwdalton@google.com> wrote:
> The virtio_net driver's mergeable receive buffer allocator
> uses 4KB packet buffers. For MTU-sized traffic, SKB truesize
> is > 4KB but only ~1500 bytes of the buffer is used to store
> packet data, reducing the effective TCP window size
> substantially. This patch addresses the performance concerns
> with mergeable receive buffers by allocating MTU-sized packet
> buffers using page frag allocators. If more than MAX_SKB_FRAGS
> buffers are needed, the SKB frag_list is used.
This looks really good, just one nit below -- should we add MTU-sized
packets, or add MTU+sizeof(virtnet_hdr)-sized packets?
Reviewed-by: Eric Northup <digitaleric@google.com>
>
> Signed-off-by: Michael Dalton <mwdalton@google.com>
> ---
> drivers/net/virtio_net.c | 164 ++++++++++++++++++++++++++++++-----------------
> 1 file changed, 106 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 9fbdfcd..113ee93 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -124,6 +124,11 @@ struct virtnet_info {
> /* Lock for config space updates */
> struct mutex config_lock;
>
> + /* Page_frag for GFP_KERNEL packet buffer allocation when we run
> + * low on memory.
> + */
> + struct page_frag alloc_frag;
> +
> /* Does the affinity hint is set for virtqueues? */
> bool affinity_hint_set;
>
> @@ -217,33 +222,18 @@ static void skb_xmit_done(struct virtqueue *vq)
> netif_wake_subqueue(vi->dev, vq2txq(vq));
> }
>
> -static void set_skb_frag(struct sk_buff *skb, struct page *page,
> - unsigned int offset, unsigned int *len)
> -{
> - int size = min((unsigned)PAGE_SIZE - offset, *len);
> - int i = skb_shinfo(skb)->nr_frags;
> -
> - __skb_fill_page_desc(skb, i, page, offset, size);
> -
> - skb->data_len += size;
> - skb->len += size;
> - skb->truesize += PAGE_SIZE;
> - skb_shinfo(skb)->nr_frags++;
> - skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
> - *len -= size;
> -}
> -
> /* Called from bottom half context */
> static struct sk_buff *page_to_skb(struct receive_queue *rq,
> - struct page *page, unsigned int len)
> + struct page *page, unsigned int offset,
> + unsigned int len, unsigned int truesize)
> {
> struct virtnet_info *vi = rq->vq->vdev->priv;
> struct sk_buff *skb;
> struct skb_vnet_hdr *hdr;
> - unsigned int copy, hdr_len, offset;
> + unsigned int copy, hdr_len, hdr_padded_len;
> char *p;
>
> - p = page_address(page);
> + p = page_address(page) + offset;
>
> /* copy small packet so we can reuse these pages for small data */
> skb = netdev_alloc_skb_ip_align(vi->dev, GOOD_COPY_LEN);
> @@ -254,16 +244,17 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
>
> if (vi->mergeable_rx_bufs) {
> hdr_len = sizeof hdr->mhdr;
> - offset = hdr_len;
> + hdr_padded_len = sizeof hdr->mhdr;
> } else {
> hdr_len = sizeof hdr->hdr;
> - offset = sizeof(struct padded_vnet_hdr);
> + hdr_padded_len = sizeof(struct padded_vnet_hdr);
> }
>
> memcpy(hdr, p, hdr_len);
>
> len -= hdr_len;
> - p += offset;
> + offset += hdr_padded_len;
> + p += hdr_padded_len;
>
> copy = len;
> if (copy > skb_tailroom(skb))
> @@ -273,6 +264,14 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
> len -= copy;
> offset += copy;
>
> + if (vi->mergeable_rx_bufs) {
> + if (len)
> + skb_add_rx_frag(skb, 0, page, offset, len, truesize);
> + else
> + put_page(page);
> + return skb;
> + }
> +
> /*
> * Verify that we can indeed put this data into a skb.
> * This is here to handle cases when the device erroneously
> @@ -284,9 +283,12 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
> dev_kfree_skb(skb);
> return NULL;
> }
> -
> + BUG_ON(offset >= PAGE_SIZE);
> while (len) {
> - set_skb_frag(skb, page, offset, &len);
> + unsigned int frag_size = min((unsigned)PAGE_SIZE - offset, len);
> + skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page, offset,
> + frag_size, truesize);
> + len -= frag_size;
> page = (struct page *)page->private;
> offset = 0;
> }
> @@ -297,33 +299,52 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
> return skb;
> }
>
> -static int receive_mergeable(struct receive_queue *rq, struct sk_buff *skb)
> +static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
> {
> - struct skb_vnet_hdr *hdr = skb_vnet_hdr(skb);
> + struct skb_vnet_hdr *hdr = skb_vnet_hdr(head_skb);
> + struct sk_buff *curr_skb = head_skb;
> + char *buf;
> struct page *page;
> - int num_buf, i, len;
> + int num_buf, len;
>
> num_buf = hdr->mhdr.num_buffers;
> while (--num_buf) {
> - i = skb_shinfo(skb)->nr_frags;
> - if (i >= MAX_SKB_FRAGS) {
> - pr_debug("%s: packet too long\n", skb->dev->name);
> - skb->dev->stats.rx_length_errors++;
> - return -EINVAL;
> - }
> - page = virtqueue_get_buf(rq->vq, &len);
> - if (!page) {
> + int num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
> + buf = virtqueue_get_buf(rq->vq, &len);
> + if (unlikely(!buf)) {
> pr_debug("%s: rx error: %d buffers missing\n",
> - skb->dev->name, hdr->mhdr.num_buffers);
> - skb->dev->stats.rx_length_errors++;
> + head_skb->dev->name, hdr->mhdr.num_buffers);
> + head_skb->dev->stats.rx_length_errors++;
> return -EINVAL;
> }
> -
> - if (len > PAGE_SIZE)
> - len = PAGE_SIZE;
> -
> - set_skb_frag(skb, page, 0, &len);
> -
> + if (unlikely(len > MAX_PACKET_LEN)) {
> + pr_debug("%s: rx error: merge buffer too long\n",
> + head_skb->dev->name);
> + len = MAX_PACKET_LEN;
> + }
> + if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) {
> + struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC);
> + if (unlikely(!nskb)) {
> + head_skb->dev->stats.rx_dropped++;
> + return -ENOMEM;
> + }
> + if (curr_skb == head_skb)
> + skb_shinfo(curr_skb)->frag_list = nskb;
> + else
> + curr_skb->next = nskb;
> + curr_skb = nskb;
> + head_skb->truesize += nskb->truesize;
> + num_skb_frags = 0;
> + }
> + if (curr_skb != head_skb) {
> + head_skb->data_len += len;
> + head_skb->len += len;
> + head_skb->truesize += MAX_PACKET_LEN;
> + }
> + page = virt_to_head_page(buf);
> + skb_add_rx_frag(curr_skb, num_skb_frags, page,
> + buf - (char *)page_address(page), len,
> + MAX_PACKET_LEN);
> --rq->num;
> }
> return 0;
> @@ -341,8 +362,10 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> if (unlikely(len < sizeof(struct virtio_net_hdr) + ETH_HLEN)) {
> pr_debug("%s: short packet %i\n", dev->name, len);
> dev->stats.rx_length_errors++;
> - if (vi->mergeable_rx_bufs || vi->big_packets)
> + if (vi->big_packets)
> give_pages(rq, buf);
> + else if (vi->mergeable_rx_bufs)
> + put_page(virt_to_head_page(buf));
> else
> dev_kfree_skb(buf);
> return;
> @@ -352,19 +375,28 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> skb = buf;
> len -= sizeof(struct virtio_net_hdr);
> skb_trim(skb, len);
> + } else if (vi->mergeable_rx_bufs) {
> + struct page *page = virt_to_head_page(buf);
> + skb = page_to_skb(rq, page,
> + (char *)buf - (char *)page_address(page),
> + len, MAX_PACKET_LEN);
> + if (unlikely(!skb)) {
> + dev->stats.rx_dropped++;
> + put_page(page);
> + return;
> + }
> + if (receive_mergeable(rq, skb)) {
> + dev_kfree_skb(skb);
> + return;
> + }
> } else {
> page = buf;
> - skb = page_to_skb(rq, page, len);
> + skb = page_to_skb(rq, page, 0, len, PAGE_SIZE);
> if (unlikely(!skb)) {
> dev->stats.rx_dropped++;
> give_pages(rq, page);
> return;
> }
> - if (vi->mergeable_rx_bufs)
> - if (receive_mergeable(rq, skb)) {
> - dev_kfree_skb(skb);
> - return;
> - }
> }
>
> hdr = skb_vnet_hdr(skb);
> @@ -501,18 +533,28 @@ static int add_recvbuf_big(struct receive_queue *rq, gfp_t gfp)
>
> static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
> {
> - struct page *page;
> + struct virtnet_info *vi = rq->vq->vdev->priv;
> + char *buf = NULL;
> int err;
>
> - page = get_a_page(rq, gfp);
> - if (!page)
> + if (gfp & __GFP_WAIT) {
Shouldn't this be MAX_PACKET_LEN + sizeof(virtio_net_hdr_mrg_rxbuf) ?
That way, we can receive an MTU-sized packet while only consuming a
single queue entry.
> + if (skb_page_frag_refill(MAX_PACKET_LEN, &vi->alloc_frag,
> + gfp)) {
> + buf = (char *)page_address(vi->alloc_frag.page) +
> + vi->alloc_frag.offset;
> + get_page(vi->alloc_frag.page);
> + vi->alloc_frag.offset += MAX_PACKET_LEN;
> + }
> + } else {
> + buf = netdev_alloc_frag(MAX_PACKET_LEN);
> + }
> + if (!buf)
> return -ENOMEM;
>
> - sg_init_one(rq->sg, page_address(page), PAGE_SIZE);
> -
> - err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, page, gfp);
> + sg_init_one(rq->sg, buf, MAX_PACKET_LEN);
> + err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, buf, gfp);
> if (err < 0)
> - give_pages(rq, page);
> + put_page(virt_to_head_page(buf));
>
> return err;
> }
> @@ -1343,8 +1385,10 @@ static void free_unused_bufs(struct virtnet_info *vi)
> struct virtqueue *vq = vi->rq[i].vq;
>
> while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> - if (vi->mergeable_rx_bufs || vi->big_packets)
> + if (vi->big_packets)
> give_pages(&vi->rq[i], buf);
> + else if (vi->mergeable_rx_bufs)
> + put_page(virt_to_head_page(buf));
> else
> dev_kfree_skb(buf);
> --vi->rq[i].num;
> @@ -1650,6 +1694,8 @@ free_recv_bufs:
> free_vqs:
> cancel_delayed_work_sync(&vi->refill);
> virtnet_del_vqs(vi);
> + if (vi->alloc_frag.page)
> + put_page(vi->alloc_frag.page);
> free_index:
> free_percpu(vi->vq_index);
> free_stats:
> @@ -1685,6 +1731,8 @@ static void virtnet_remove(struct virtio_device *vdev)
> unregister_netdev(vi->dev);
>
> remove_vq_common(vi);
> + if (vi->alloc_frag.page)
> + put_page(vi->alloc_frag.page);
>
> flush_work(&vi->config_work);
>
> --
> 1.8.4.1
>
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re:Answer back
From: Lee Hyuk @ 2013-10-29 11:16 UTC (permalink / raw)
I would like to discuss a very important crude oil project with
you,kindlyrevert back to me if this is your valid email address for
further information.
Regards,
Lee
^ permalink raw reply
* Re: [PATCH 00/19] Enable various Renesas drivers on all ARM platforms
From: Mark Brown @ 2013-10-29 17:58 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Linus Walleij,
Guennadi Liakhovetski, Thierry Reding,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Laurent Pinchart, Vinod Koul,
linux-sh-u79uwXL29TY76Z2rM5mHXA, Magnus Damm, Eduardo Valentin,
Tomi Valkeinen, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, Zhang Rui, Chris Ball,
Jean-Christophe Plagniol-Villard,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-pwm-u79uwXL29TY76Z2rM5mHXA, Samuel Ortiz,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Ian Molton, Simon Horman,
linux-arm
In-Reply-To: <1422562.0L87CDt5Gd@avalon>
[-- Attachment #1.1: Type: text/plain, Size: 1213 bytes --]
On Tue, Oct 29, 2013 at 06:29:59PM +0100, Laurent Pinchart wrote:
> On Tuesday 29 October 2013 10:23:31 Mark Brown wrote:
> > On Tue, Oct 29, 2013 at 06:05:53PM +0100, Laurent Pinchart wrote:
> > > The first one is that I can't compile-test all those drivers on all
> > > architectures. The spi-sh-msiof driver, for instance, uses
> > > io(read|write)(16|
> > Which architectures are these and is there not a symbol we can depend on
> > for them?
> arch/cris for instance. We can use readl/writel instead (maybe it would be
> time to rationalize and document the I/O accessors across all architectures,
> but that's another topic).
It'd certainly be sensible, or adding a config option to depend on if
you rely on these functions.
> My point is that there might be other issues that I won't be able to easily
> catch. This would break compilation for everybody for no reason, as the
> drivers are useless on non-SuperH, non-ARM platforms. That's why I believe
> COMPILE_TEST would be a better option as a first step.
Yes, it would - please do that. Note that it won't stop anyone running
into build issues on other architectures though, it's just about
stopping Kconfig noise.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH 00/19] Enable various Renesas drivers on all ARM platforms
From: Laurent Pinchart @ 2013-10-29 17:29 UTC (permalink / raw)
To: Mark Brown
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Linus Walleij,
Guennadi Liakhovetski, Thierry Reding,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Laurent Pinchart,
ed.com-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY, Vinod Koul,
linux-sh-u79uwXL29TY76Z2rM5mHXA, Magnus Damm, Eduardo Valentin,
Tomi Valkeinen, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, Zhang Rui, Chris Ball,
Jean-Christophe Plagniol-Villard,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-pwm-u79uwXL29TY76Z2rM5mHXA, Samuel Ortiz,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Ian Molton, Simon
In-Reply-To: <20131029172331.GA20251-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1752 bytes --]
Hi Mark,
On Tuesday 29 October 2013 10:23:31 Mark Brown wrote:
> On Tue, Oct 29, 2013 at 06:05:53PM +0100, Laurent Pinchart wrote:
> > The first one is that I can't compile-test all those drivers on all
> > architectures. The spi-sh-msiof driver, for instance, uses
> > io(read|write)(16|
>
> Which architectures are these and is there not a symbol we can depend on
> for them?
arch/cris for instance. We can use readl/writel instead (maybe it would be
time to rationalize and document the I/O accessors across all architectures,
but that's another topic).
My point is that there might be other issues that I won't be able to easily
catch. This would break compilation for everybody for no reason, as the
drivers are useless on non-SuperH, non-ARM platforms. That's why I believe
COMPILE_TEST would be a better option as a first step.
> > 32) which are not available on all architectures. There might be other
> > similar problems that I can't catch, and I don't want to introduce build
> > breakages in the kernel.
>
> This is easy enough to handle if we do run into issues, it seems better to
> get things available than try to step through architecture by architecture.
>
> > The second reason is that, as the IP cores have never been used on
> > anything but SuperH and ARM, I don't like the idea of clobbering the
> > config process with drivers that are useless on the target architecture.
> > Now that we have a COMPILE_TEST Kconfig option, my preference would thus
> > go to SUPERH || ARM || COMPILE_TEST over no dependency at all.
>
> That's not what you did, though - you're not adding COMPILE_TEST.
No, but I can add it :-) If this gets agreed upon, I'll respin the series with
|| COMPILE_TEST.
--
Regards,
Laurent Pinchart
[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH 00/19] Enable various Renesas drivers on all ARM platforms
From: Mark Brown @ 2013-10-29 17:23 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Linus Walleij,
Guennadi Liakhovetski, Thierry Reding,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Laurent Pinchart, Vinod Koul,
linux-sh-u79uwXL29TY76Z2rM5mHXA, Magnus Damm, Eduardo Valentin,
Tomi Valkeinen, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, Zhang Rui, Chris Ball,
Jean-Christophe Plagniol-Villard,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-pwm-u79uwXL29TY76Z2rM5mHXA, Samuel Ortiz,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Ian Molton, Simon Horman,
linux-arm
In-Reply-To: <1577821.Q7gttkPE2J@avalon>
[-- Attachment #1.1: Type: text/plain, Size: 1082 bytes --]
On Tue, Oct 29, 2013 at 06:05:53PM +0100, Laurent Pinchart wrote:
> The first one is that I can't compile-test all those drivers on all
> architectures. The spi-sh-msiof driver, for instance, uses io(read|write)(16|
Which architectures are these and is there not a symbol we can depend on
for them?
> 32) which are not available on all architectures. There might be other similar
> problems that I can't catch, and I don't want to introduce build breakages in
> the kernel.
This is easy enough to handle if we do run into issues, it seems better
to get things available than try to step through architecture by
architecture.
> The second reason is that, as the IP cores have never been used on anything
> but SuperH and ARM, I don't like the idea of clobbering the config process
> with drivers that are useless on the target architecture. Now that we have a
> COMPILE_TEST Kconfig option, my preference would thus go to SUPERH || ARM ||
> COMPILE_TEST over no dependency at all.
That's not what you did, though - you're not adding COMPILE_TEST.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag
From: Dan Williams @ 2013-10-29 17:21 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: David Miller, jiri, vyasevich, netdev, kuznet, jmorris, yoshfuji,
kaber, thaller, stephen
In-Reply-To: <20131029143847.GB16253@order.stressinduktion.org>
On Tue, 2013-10-29 at 15:38 +0100, Hannes Frederic Sowa wrote:
> Hi!
>
> On Tue, Oct 29, 2013 at 09:31:18AM -0500, Dan Williams wrote:
> > On Tue, 2013-10-29 at 00:48 +0100, Hannes Frederic Sowa wrote:
> > > On Mon, Oct 28, 2013 at 06:16:19PM -0500, Dan Williams wrote:
> > > > On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
> > > > > From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > > > > Date: Sun, 27 Oct 2013 17:48:35 +0100
> > > > >
> > > > > > A temporary address is also bound to a non-privacy public address so
> > > > > > it's lifetime is determined by its lifetime (e.g. if you switch the
> > > > > > network and don't receive on-link information for that prefix any
> > > > > > more). NetworkManager would have to take care about that, too. It is
> > > > > > just a question of what NetworkManager wants to handle itself or lets
> > > > > > the kernel handle for it.
> > > > >
> > > > > How much really needs to be in userspace to implement RFC4941?
> > > > >
> > > > > I don't like the idea that even for a fully up and properly
> > > > > functioning link, if NetworkManager wedges then critical things like
> > > > > temporary address (re-)generation, will cease.
> > > >
> > > > Honestly, I'd be completely happy to leave temporary address handling up
> > > > to the kernel and *not* do it in userspace; the kernel already has all
> > > > the code. There are two problems with that though, (a) it's tied to
> > > > in-kernel RA handling, and (b) it's controlled by a CONFIG option. Both
> > > > these are solvable.
> > >
> > > Ah, (a) does complicate things, I agree. But the tieing is essential
> > > currently. So it seems a netlink interface would be needed to tie a new
> > > address to an already installed one, if the kernel should still deal
> > > with the regeneration?
> >
> > I think it's simpler than that. New flag set when adding the
> > non-private address that says "create and manage privacy addresses for
> > this non-private address". The kernel then adds the privacy addresses
> > generated off the non-private address/prefixlen, and ties their lifetime
> > to the non-private address. If the non-private address is removed, the
> > privacy addresses could get removed too.
> >
> > I don't think we need API to tie addresses to already installed ones,
> > because the kernel already has the privacy address generation code, so
> > why should userspace generate the privacy address at all? Just leave
> > that to the kernel.
>
> Ok.
>
> > > > First off, what's the reasoning behind having IPv6 privacy as a config
> > > > option? It's off-by-default and must be explicitly turned on, so is
> > > > there any harm in removing the config? Or is it just for
> > > > smallest-kernel-ever folks?
> > >
> > > I don't know about the policy. Does it really matter as distributions
> > > normally switch it on? But I would not like to see the option removed
> > > entirly, maybe the default could be changed.
> > >
> > > > Would a new IFA_F_MANAGE_TEMP (or better name) work here, indicating
> > > > that for some new static address, that the kernel should create and
> > > > manage the temporary privacy addresses associated with its prefix?
> > >
> > > But this would only be needed if they were managed in user-space, no?
> >
> > "if they" == what? privacy address or static address? What
>
> With "they" I meant privacy addresses.
>
> > NetworkManager is trying to do is handle RAs in userspace with libndp
> > for various flexibility and behavioral reasons, but we'd really like to
> > leave all the temporary address stuff up to the kernel.
>
> Can you provide me with details why the Kernel RA implementation is not good
> enough? I tried to find some bugs, I found some but they were missing details
> or were not even correct or outdated.
First, RA handling is too tightly tied to interface flags. This is a
problem because interfaces are required to be IFF_UP for various
operations like carrier detection, wireless scanning, reading certain
interface properties, link statistics, etc. We can play games with
flipping accept_ra, but changing accept_ra doesn't trigger an RS. At
the moment, only interface flag changes trigger an RS, and that also can
reset a lot of L2 state.
Second, Router Solicitations are required at various times other than
NETDEV_CHANGE/NETDEV_UP. The router is not guaranteed to send (nor are
we guaranteed to receive) an RA when the RDNSS/DNSSL approach their
lifetimes, so to ensure those values are still still valid, we need to
send out an RS, especially on lossy networks where the RA might get
dropped.
Third, we need more flexibility in reading ND user options like DNSSL
and RDNSS and newer stuff. Every new option that userspace might want
to process requires some kernel code (ndisc_is_useropt()) to push it out
to userspace, and that means these options are not available on older
kernels.
Fourth, there's no opportunity to override any of the RA-derived
settings with user preferences before the kernel commits them. Perhaps
the user wishes to ignore a specific prefix (but accept other prefixes
or other RAs), or to ignore the automatically provided routes, or
whatever.
> > So NM would handle RA/RS and when it gets a prefix, it would create the
> > IPv6 non-private address and add it to the interface. When adding, it
> > would also set the "IFA_F_MANAGE_TEMP" flag (or whatever) and the kernel
> > would then handle all the privacy address generation, lifetimes, and
> > timers. Basically, break some of the privacy code away from the
> > in-kernel RA handling so that privacy addresses could be triggered from
> > userland too.
> >
> > Would that be workable?
>
> That sounds like a solid plan for me. I would actually liked to see that NM
> would use the kernel implementation but I guess there is no way back any more.
> :(
Some of the issues mentioned above might be inappropriate to solve
entirely in the kernel. I have no problem with in-kernel IPv6 for
simple use-cases and it works great for these. But if we think about
more complex functionality, especially where userland might want an
opportunity to override some of the behavior, and backwards
compatibility with kernels that don't implement these changes, we'd like
to handle some addrconf in userspace.
Dan
^ permalink raw reply
* Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag
From: Dan Williams @ 2013-10-29 17:15 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Hannes Frederic Sowa, David Miller, jiri, netdev, kuznet, jmorris,
yoshfuji, kaber, thaller, stephen
In-Reply-To: <526FE93E.3040300@gmail.com>
On Tue, 2013-10-29 at 12:58 -0400, Vlad Yasevich wrote:
> On 10/29/2013 10:31 AM, Dan Williams wrote:
> > On Tue, 2013-10-29 at 00:48 +0100, Hannes Frederic Sowa wrote:
> >> On Mon, Oct 28, 2013 at 06:16:19PM -0500, Dan Williams wrote:
> >>> On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
> >>>> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> >>>> Date: Sun, 27 Oct 2013 17:48:35 +0100
> >>>>
> >>>>> A temporary address is also bound to a non-privacy public address so
> >>>>> it's lifetime is determined by its lifetime (e.g. if you switch the
> >>>>> network and don't receive on-link information for that prefix any
> >>>>> more). NetworkManager would have to take care about that, too. It is
> >>>>> just a question of what NetworkManager wants to handle itself or lets
> >>>>> the kernel handle for it.
> >>>>
> >>>> How much really needs to be in userspace to implement RFC4941?
> >>>>
> >>>> I don't like the idea that even for a fully up and properly
> >>>> functioning link, if NetworkManager wedges then critical things like
> >>>> temporary address (re-)generation, will cease.
> >>>
> >>> Honestly, I'd be completely happy to leave temporary address handling up
> >>> to the kernel and *not* do it in userspace; the kernel already has all
> >>> the code. There are two problems with that though, (a) it's tied to
> >>> in-kernel RA handling, and (b) it's controlled by a CONFIG option. Both
> >>> these are solvable.
> >>
> >> Ah, (a) does complicate things, I agree. But the tieing is essential
> >> currently. So it seems a netlink interface would be needed to tie a new
> >> address to an already installed one, if the kernel should still deal
> >> with the regeneration?
> >
> > I think it's simpler than that. New flag set when adding the
> > non-private address that says "create and manage privacy addresses for
> > this non-private address". The kernel then adds the privacy addresses
> > generated off the non-private address/prefixlen, and ties their lifetime
> > to the non-private address. If the non-private address is removed, the
> > privacy addresses could get removed too.
> >
> > I don't think we need API to tie addresses to already installed ones,
> > because the kernel already has the privacy address generation code, so
> > why should userspace generate the privacy address at all? Just leave
> > that to the kernel.
> >
> >>> First off, what's the reasoning behind having IPv6 privacy as a config
> >>> option? It's off-by-default and must be explicitly turned on, so is
> >>> there any harm in removing the config? Or is it just for
> >>> smallest-kernel-ever folks?
> >>
> >> I don't know about the policy. Does it really matter as distributions
> >> normally switch it on? But I would not like to see the option removed
> >> entirly, maybe the default could be changed.
> >>
> >>> Would a new IFA_F_MANAGE_TEMP (or better name) work here, indicating
> >>> that for some new static address, that the kernel should create and
> >>> manage the temporary privacy addresses associated with its prefix?
> >>
> >> But this would only be needed if they were managed in user-space, no?
> >
> > "if they" == what? privacy address or static address? What
> > NetworkManager is trying to do is handle RAs in userspace with libndp
> > for various flexibility and behavioral reasons, but we'd really like to
> > leave all the temporary address stuff up to the kernel.
> >
> > So NM would handle RA/RS and when it gets a prefix, it would create the
> > IPv6 non-private address and add it to the interface. When adding, it
> > would also set the "IFA_F_MANAGE_TEMP" flag (or whatever) and the kernel
> > would then handle all the privacy address generation, lifetimes, and
> > timers. Basically, break some of the privacy code away from the
> > in-kernel RA handling so that privacy addresses could be triggered from
> > userland too.
> >
> > Would that be workable?
>
> You are still dependent on the NM/user app to do this and what happens
> if that apps wedges?
In my proposal, the kernel would still manage the lifetimes of all the
addresses, since user app would add the non-privacy address with the
correct lifetime, and the kernel would generate the privacy addresses
with a corresponding lifetime. If the app wedges for some reason, then
the kernel will deprecate and eventually remove the non-privacy *and*
privacy addresses since their lifetimes have expired.
What if your dhclient wedges? What if ovsd or teamd goes down? Or your
openvpn, vpnc, pptp, pppd, sshd, whatever wedges? There's a lot of
networking that's controlled by userland these days, and failure of
these things also potentailly wedges your network. We should be
striving to make the best userland we can instead of trying to stuff
everything into the kernel in the name of reliability.
> I think we should just do privacy addresses automatically, or based on
> some sysconfig setting per interface to give users ability to turn it
> off. But I agree with David, and I speak from experience.
> You don't whant address configuration to be done by userspace daemon.
> There are too many things that can go wrong.
Should IPv6 should be that different from IPv4? DHCP is done by a
userspace daemon in all cases (v6 and v4), and other v4 is always done
by userland (static files, avahi-autoipd, other daemons). You can never
get away from userland here, and we need more flexibility in userland
than the kernel currently provides with its addrconf implementation.
Dan
^ permalink raw reply
* [PATCH net-next] tcp: temporarily disable Fast Open on SYN timeout
From: Yuchung Cheng @ 2013-10-29 17:09 UTC (permalink / raw)
To: davem, ncardwell, edumazet; +Cc: netdev, Yuchung Cheng
Fast Open currently has a fall back feature to address SYN-data
being dropped by but it requires the middle-box to pass on regular
SYN retry after SYN-data. This is implemented in commit aab487435
("net-tcp: Fast Open client - detecting SYN-data drops")
However some NAT boxes will drop all subsequent packets after first
SYN-data and blackholes the entire connections. An example is incommit
356d7d8 "netfilter: nf_conntrack: fix tcp_in_window for Fast Open".
The sender should note such incidents and falls back to use regular
TCP handshake on subsequent attempt temporarily as well: after the
second SYN timeouts the original Fast Open SYN is most likely lost.
When such an event recurs Fast Open is disabled based on the number
of recurrences exponentially.
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_metrics.c | 5 +++--
net/ipv4/tcp_timer.c | 6 +++++-
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index 4a2a841..2ab09cb 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -671,8 +671,9 @@ void tcp_fastopen_cache_set(struct sock *sk, u16 mss,
struct tcp_fastopen_metrics *tfom = &tm->tcpm_fastopen;
write_seqlock_bh(&fastopen_seqlock);
- tfom->mss = mss;
- if (cookie->len > 0)
+ if (mss)
+ tfom->mss = mss;
+ if (cookie && cookie->len > 0)
tfom->cookie = *cookie;
if (syn_lost) {
++tfom->syn_loss;
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index af07b5b..64f0354 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -156,12 +156,16 @@ static bool retransmits_timed_out(struct sock *sk,
static int tcp_write_timeout(struct sock *sk)
{
struct inet_connection_sock *icsk = inet_csk(sk);
+ struct tcp_sock *tp = tcp_sk(sk);
int retry_until;
bool do_reset, syn_set = false;
if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) {
- if (icsk->icsk_retransmits)
+ if (icsk->icsk_retransmits) {
dst_negative_advice(sk);
+ if (tp->syn_fastopen || tp->syn_data)
+ tcp_fastopen_cache_set(sk, 0, NULL, true);
+ }
retry_until = icsk->icsk_syn_retries ? : sysctl_tcp_syn_retries;
syn_set = true;
} else {
--
1.8.4.1
^ permalink raw reply related
* Re: [PATCH 00/19] Enable various Renesas drivers on all ARM platforms
From: Laurent Pinchart @ 2013-10-29 17:05 UTC (permalink / raw)
To: Mark Brown
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Linus Walleij,
Guennadi Liakhovetski, Thierry Reding,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Laurent Pinchart,
ed.com-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY, Vinod Koul,
linux-sh-u79uwXL29TY76Z2rM5mHXA, Magnus Damm, Eduardo Valentin,
Tomi Valkeinen, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, Zhang Rui, Chris Ball,
Jean-Christophe Plagniol-Villard,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-pwm-u79uwXL29TY76Z2rM5mHXA, Samuel Ortiz,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Ian Molton, Simon
In-Reply-To: <20131029160449.GD16686-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1030 bytes --]
Hi Mark,
On Tuesday 29 October 2013 09:04:49 Mark Brown wrote:
> On Tue, Oct 29, 2013 at 03:04:27PM +0900, Simon Horman wrote:
> > I think this is a step in a good direction.
> > However, I think it would be even better if the architecture dependency
> > was removed completely.
>
> Yes, please.
I've kept it for two reasons.
The first one is that I can't compile-test all those drivers on all
architectures. The spi-sh-msiof driver, for instance, uses io(read|write)(16|
32) which are not available on all architectures. There might be other similar
problems that I can't catch, and I don't want to introduce build breakages in
the kernel.
The second reason is that, as the IP cores have never been used on anything
but SuperH and ARM, I don't like the idea of clobbering the config process
with drivers that are useless on the target architecture. Now that we have a
COMPILE_TEST Kconfig option, my preference would thus go to SUPERH || ARM ||
COMPILE_TEST over no dependency at all.
--
Regards,
Laurent Pinchart
[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [net-next 11/11] i40e: fix error return code in i40e_probe()
From: Joe Perches @ 2013-10-29 17:05 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, Wei Yongjun, netdev, gospo, sassmann
In-Reply-To: <1383048151-15002-12-git-send-email-jeffrey.t.kirsher@intel.com>
On Tue, 2013-10-29 at 05:02 -0700, Jeff Kirsher wrote:
> Fix to return -ENOMEM in the memory alloc error handling
> case instead of 0, as done elsewhere in this function.
trivial note:
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
[]
> @@ -7204,8 +7204,10 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> */
> len = sizeof(struct i40e_vsi *) * pf->hw.func_caps.num_vsis;
> pf->vsi = kzalloc(len, GFP_KERNEL);
> - if (!pf->vsi)
> + if (!pf->vsi) {
> + err = -ENOMEM;
> goto err_switch_setup;
> + }
This might be better as:
pf->vsi = kcalloc(pf->hw.func_caps.num_vsis, struct i40e_vsi *),
GFP_KERNEL);
and removing the now unused u32 len; declaration.
^ permalink raw reply
* Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag
From: Vlad Yasevich @ 2013-10-29 16:58 UTC (permalink / raw)
To: Dan Williams, Hannes Frederic Sowa
Cc: David Miller, jiri, netdev, kuznet, jmorris, yoshfuji, kaber,
thaller, stephen
In-Reply-To: <1383057078.2236.12.camel@dcbw.foobar.com>
On 10/29/2013 10:31 AM, Dan Williams wrote:
> On Tue, 2013-10-29 at 00:48 +0100, Hannes Frederic Sowa wrote:
>> On Mon, Oct 28, 2013 at 06:16:19PM -0500, Dan Williams wrote:
>>> On Mon, 2013-10-28 at 17:17 -0400, David Miller wrote:
>>>> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
>>>> Date: Sun, 27 Oct 2013 17:48:35 +0100
>>>>
>>>>> A temporary address is also bound to a non-privacy public address so
>>>>> it's lifetime is determined by its lifetime (e.g. if you switch the
>>>>> network and don't receive on-link information for that prefix any
>>>>> more). NetworkManager would have to take care about that, too. It is
>>>>> just a question of what NetworkManager wants to handle itself or lets
>>>>> the kernel handle for it.
>>>>
>>>> How much really needs to be in userspace to implement RFC4941?
>>>>
>>>> I don't like the idea that even for a fully up and properly
>>>> functioning link, if NetworkManager wedges then critical things like
>>>> temporary address (re-)generation, will cease.
>>>
>>> Honestly, I'd be completely happy to leave temporary address handling up
>>> to the kernel and *not* do it in userspace; the kernel already has all
>>> the code. There are two problems with that though, (a) it's tied to
>>> in-kernel RA handling, and (b) it's controlled by a CONFIG option. Both
>>> these are solvable.
>>
>> Ah, (a) does complicate things, I agree. But the tieing is essential
>> currently. So it seems a netlink interface would be needed to tie a new
>> address to an already installed one, if the kernel should still deal
>> with the regeneration?
>
> I think it's simpler than that. New flag set when adding the
> non-private address that says "create and manage privacy addresses for
> this non-private address". The kernel then adds the privacy addresses
> generated off the non-private address/prefixlen, and ties their lifetime
> to the non-private address. If the non-private address is removed, the
> privacy addresses could get removed too.
>
> I don't think we need API to tie addresses to already installed ones,
> because the kernel already has the privacy address generation code, so
> why should userspace generate the privacy address at all? Just leave
> that to the kernel.
>
>>> First off, what's the reasoning behind having IPv6 privacy as a config
>>> option? It's off-by-default and must be explicitly turned on, so is
>>> there any harm in removing the config? Or is it just for
>>> smallest-kernel-ever folks?
>>
>> I don't know about the policy. Does it really matter as distributions
>> normally switch it on? But I would not like to see the option removed
>> entirly, maybe the default could be changed.
>>
>>> Would a new IFA_F_MANAGE_TEMP (or better name) work here, indicating
>>> that for some new static address, that the kernel should create and
>>> manage the temporary privacy addresses associated with its prefix?
>>
>> But this would only be needed if they were managed in user-space, no?
>
> "if they" == what? privacy address or static address? What
> NetworkManager is trying to do is handle RAs in userspace with libndp
> for various flexibility and behavioral reasons, but we'd really like to
> leave all the temporary address stuff up to the kernel.
>
> So NM would handle RA/RS and when it gets a prefix, it would create the
> IPv6 non-private address and add it to the interface. When adding, it
> would also set the "IFA_F_MANAGE_TEMP" flag (or whatever) and the kernel
> would then handle all the privacy address generation, lifetimes, and
> timers. Basically, break some of the privacy code away from the
> in-kernel RA handling so that privacy addresses could be triggered from
> userland too.
>
> Would that be workable?
You are still dependent on the NM/user app to do this and what happens
if that apps wedges?
I think we should just do privacy addresses automatically, or based on
some sysconfig setting per interface to give users ability to turn it
off. But I agree with David, and I speak from experience.
You don't whant address configuration to be done by userspace daemon.
There are too many things that can go wrong.
-vlad
>
> Dan
>
^ permalink raw reply
* Re: [PATCH] netdev: octeon_mgmt: drop redundant mac address check
From: David Daney @ 2013-10-29 16:33 UTC (permalink / raw)
To: Luka Perkov, David Miller; +Cc: netdev, david.daney
In-Reply-To: <1383010061-25461-1-git-send-email-luka@openwrt.org>
On 10/28/2013 06:27 PM, Luka Perkov wrote:
> Checking if MAC address is valid using is_valid_ether_addr() is already done in
> of_get_mac_address().
>
> Signed-off-by: Luka Perkov <luka@openwrt.org>
This looks sane, but I haven't tested it...
Acked-by: David Daney <david.daney@cavium.com>
> ---
> drivers/net/ethernet/octeon/octeon_mgmt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/octeon/octeon_mgmt.c b/drivers/net/ethernet/octeon/octeon_mgmt.c
> index 622aa75..1b326cbc 100644
> --- a/drivers/net/ethernet/octeon/octeon_mgmt.c
> +++ b/drivers/net/ethernet/octeon/octeon_mgmt.c
> @@ -1545,7 +1545,7 @@ static int octeon_mgmt_probe(struct platform_device *pdev)
>
> mac = of_get_mac_address(pdev->dev.of_node);
>
> - if (mac && is_valid_ether_addr(mac))
> + if (mac)
> memcpy(netdev->dev_addr, mac, ETH_ALEN);
> else
> eth_hw_addr_random(netdev);
>
^ permalink raw reply
* Re: [PATCH 00/19] Enable various Renesas drivers on all ARM platforms
From: Linus Walleij @ 2013-10-29 16:28 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-fbdev@vger.kernel.org, linux-sh@vger.kernel.org,
Guennadi Liakhovetski, Thierry Reding,
linux-mtd@lists.infradead.org, linux-i2c@vger.kernel.org,
Vinod Koul, Joerg Roedel, Wolfram Sang, Magnus Damm,
Eduardo Valentin, Tomi Valkeinen, linux-serial@vger.kernel.org,
Linux Input, Zhang Rui, Chris Ball,
Jean-Christophe Plagniol-Villard, linux-media@vger.kernel.org,
"linux-pwm@vger.kernel.org" <linu
In-Reply-To: <1383004027-25036-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
On Mon, Oct 28, 2013 at 4:46 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> If you believe the issue should be solved in a different way (for instance by
> removing the architecture dependency completely) please reply to the cover
> letter to let other maintainers chime in.
I have no real opinions on whether this is a good itermediate step towards
complete arch-independence or not, but I just noticed that you still
have the <mach/*> hierarchy in mach-shmobile/include/mach/*, and
grep:ing around this seems totally unnecessary, I think it would be
easy to produce a patch set eliminating <mach/*> from shmobile.
(Usually we will just move all the headers down into the
mach-shmobile folder.)
Yours,
Linus Walleij
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* Re: [PATCH v2] can: c_can: Speed up rx_poll function
From: Joe Perches @ 2013-10-29 16:24 UTC (permalink / raw)
To: Markus Pargmann
Cc: Marc Kleine-Budde, Wolfgang Grandegger, linux-can, netdev,
linux-kernel, kernel
In-Reply-To: <20131029085853.GC20839@pengutronix.de>
On Tue, 2013-10-29 at 09:58 +0100, Markus Pargmann wrote:
> On Tue, Oct 29, 2013 at 01:34:48AM -0700, Joe Perches wrote:
> > On Tue, 2013-10-29 at 09:27 +0100, Markus Pargmann wrote:
> > > This patch speeds up the rx_poll function by reducing the number of
> > > register reads.
> > []
> > > 125kbit:
> > > Function Hit Time Avg s^2
> > > -------- --- ---- --- ---
> > > c_can_do_rx_poll 63960 10168178 us 158.977 us 1493056 us
> > > With patch:
> > > c_can_do_rx_poll 63939 4268457 us 66.758 us 818790.9 us
> > >
> > > 1Mbit:
> > > Function Hit Time Avg s^2
> > > -------- --- ---- --- ---
> > > c_can_do_rx_poll 69489 30049498 us 432.435 us 9271851 us
> > > With patch:
> > > c_can_do_rx_poll 103034 24220362 us 235.071 us 6016656 us
[]
> Yes I just measured the timings again:
[]
> ./perf_can_test.sh 125000 30
[]
> c_can_do_rx_poll 63941 3764057 us 58.867 us 776162.2 us
Good, it's slightly faster still.
> ./perf_can_test.sh 1000000 30
[]
> c_can_do_rx_poll 207109 24322185 us 117.436 us 171469047 us
[]
> It is interesting that the number of hits for c_can_do_rx_poll is twice as much
> as it was with find_next_bit.
How is this possible? Any idea?
^ 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