* Re: [PATCH] net/ethernet/intel/ixgbe/ixgbe_debugfs.c: fix error handling in ixgbe_dbg_reg_ops_read().
From: Jeff Kirsher @ 2012-11-16 4:42 UTC (permalink / raw)
To: Cyril Roelandt
Cc: e1000-devel, netdev, linux-kernel, Hay, Joshua A, Dan Carpenter
In-Reply-To: <1353036410-14439-1-git-send-email-tipecaml@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1205 bytes --]
On Fri, 2012-11-16 at 04:26 +0100, Cyril Roelandt wrote:
>
> copy_to_user() cannot return a negative value: it returns the number
> of bytes
> that could not be copied.
>
> Return -EFAULT on failure rather than the number of bytes that could
> not be
> copied, as this seems more standard.
>
> Signed-off-by: Cyril Roelandt <tipecaml@gmail.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
Actually, I already have a similar patch in my queue reported by Dan
Carpenter, and created by Josh Hay which fixes this issue. I should be
pushing the patch in my queue in the next week. Here is the patch I am
referring to:
ixgbe: eliminate Smatch warnings in ixgbe_debugfs.c
This patch replaces calls to copy_to_user, copy_from_user, and the
associated logic, with calls to simple_read_from_buffer and
simple_write_to_buffer respectively. This was done to eliminate
warnings generated by the Smatch static analysis tool.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
CC: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Josh Hay <joshua.a.hay@intel.com>
Cheers,
Jeff
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [Xen-devel] [PATCH 3/4] Xen/netfront: Implement persistent grant in netfront.
From: ANNIE LI @ 2012-11-16 5:22 UTC (permalink / raw)
To: Roger Pau Monné
Cc: xen-devel@lists.xensource.com, netdev@vger.kernel.org,
konrad.wilk@oracle.com, Ian Campbell
In-Reply-To: <50A4C987.3020308@citrix.com>
On 2012-11-15 18:52, Roger Pau Monné wrote:
> On 15/11/12 08:05, Annie Li wrote:
>> Tx/rx page pool are maintained. New grant is mapped and put into
>> pool, unmap only happens when releasing/removing device.
>>
>> Signed-off-by: Annie Li<annie.li@oracle.com>
>> ---
>> drivers/net/xen-netfront.c | 372 +++++++++++++++++++++++++++++++++++++-------
>> 1 files changed, 315 insertions(+), 57 deletions(-)
>>
>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
>> index 0ebbb19..17b81c0 100644
>> --- a/drivers/net/xen-netfront.c
>> +++ b/drivers/net/xen-netfront.c
>> @@ -79,6 +79,13 @@ struct netfront_stats {
>> struct u64_stats_sync syncp;
>> };
>>
>> +struct gnt_list {
>> + grant_ref_t gref;
>> + struct page *gnt_pages;
>> + void *gnt_target;
>> + struct gnt_list *tail;
>> +};
> This could also be shared with blkfront.
Netfront does not have the shadow like blkfront, and it needs the
gnt_target to save skb address of rx path. So we can share this too?
blkfront would not use it actually.
>
>> +
>> struct netfront_info {
>> struct list_head list;
>> struct net_device *netdev;
>> @@ -109,6 +116,10 @@ struct netfront_info {
>> grant_ref_t grant_tx_ref[NET_TX_RING_SIZE];
>> unsigned tx_skb_freelist;
>>
>> + struct gnt_list *tx_grant[NET_TX_RING_SIZE];
>> + struct gnt_list *tx_gnt_list;
>> + unsigned int tx_gnt_cnt;
> I don't understand this, why do you need both an array and a list?
The array tx_grant is just like other tx_skbs, grant_tx_ref. It saves
grant entries corresponding every request in the ring. This is what
netfront different from blkfront, netfront does not have shadow ring,
and it only uses a ring size array to track every request in the ring.
The list is like a pool to save all available persistent grants.
> I'm
> not familiar with net code, so I don't know if this is some kind of
> special netfront thing?
Yes, this is different from blkfront. netfront uses ring size arrays to
track every request in the ring.
>
> Anyway if you have to use a list I would recommend using one of the list
> constructions that's already in the kernel, it simplifies the code and
> makes it more easy to understand than creating your own list structure.
Ok, thanks.
>
>> +
>> spinlock_t rx_lock ____cacheline_aligned_in_smp;
>> struct xen_netif_rx_front_ring rx;
>> int rx_ring_ref;
>> @@ -126,6 +137,10 @@ struct netfront_info {
>> grant_ref_t gref_rx_head;
>> grant_ref_t grant_rx_ref[NET_RX_RING_SIZE];
>>
>> + struct gnt_list *rx_grant[NET_RX_RING_SIZE];
>> + struct gnt_list *rx_gnt_list;
>> + unsigned int rx_gnt_cnt;
> Same comment above here.
Same as above.
>
>> +
>> unsigned long rx_pfn_array[NET_RX_RING_SIZE];
>> struct multicall_entry rx_mcl[NET_RX_RING_SIZE+1];
>> struct mmu_update rx_mmu[NET_RX_RING_SIZE];
>> @@ -134,6 +149,7 @@ struct netfront_info {
>> struct netfront_stats __percpu *stats;
>>
>> unsigned long rx_gso_checksum_fixup;
>> + u8 persistent_gnt:1;
>> };
>>
>> struct netfront_rx_info {
>> @@ -194,6 +210,16 @@ static grant_ref_t xennet_get_rx_ref(struct netfront_info *np,
>> return ref;
>> }
>>
>> +static struct gnt_list *xennet_get_rx_grant(struct netfront_info *np,
>> + RING_IDX ri)
>> +{
>> + int i = xennet_rxidx(ri);
>> + struct gnt_list *gntlist = np->rx_grant[i];
>> + np->rx_grant[i] = NULL;
> Ok, I think I get why do you need both an array and a list, is that
> because netfront doesn't have some kind of shadow ring to keep track of
> issued requests?
Yes.
>
> So each issued request has an associated gnt_list with the list of used
> grants?
gnt_list is kind of free grants. It is like a pool of free grants. If
free grants exist in this list, free grant will be gotten from this
list. If no, new grant will be allocated. In xennet_tx_buf_gc, free
grants will be put into the list again if response status is OK.
> If so it would be good to add a comment about it.
>
>> + return gntlist;
>> +}
>> +
>> #ifdef CONFIG_SYSFS
>> static int xennet_sysfs_addif(struct net_device *netdev);
>> static void xennet_sysfs_delif(struct net_device *netdev);
>> @@ -231,6 +257,68 @@ static void xennet_maybe_wake_tx(struct net_device *dev)
>> netif_wake_queue(dev);
>> }
>>
>> +static grant_ref_t xennet_alloc_rx_ref(struct net_device *dev,
>> + unsigned long mfn, void *vaddr,
>> + unsigned int id,
>> + grant_ref_t ref)
>> +{
>> + struct netfront_info *np = netdev_priv(dev);
>> + grant_ref_t gnt_ref;
>> + struct gnt_list *gnt_list_entry;
>> +
>> + if (np->persistent_gnt&& np->rx_gnt_cnt) {
>> + gnt_list_entry = np->rx_gnt_list;
>> + np->rx_gnt_list = np->rx_gnt_list->tail;
>> + np->rx_gnt_cnt--;
>> +
>> + gnt_list_entry->gnt_target = vaddr;
>> + gnt_ref = gnt_list_entry->gref;
>> + np->rx_grant[id] = gnt_list_entry;
>> + } else {
>> + struct page *page;
>> +
>> + BUG_ON(!np->persistent_gnt&& np->rx_gnt_cnt);
>> + if (!ref)
>> + gnt_ref =
>> + gnttab_claim_grant_reference(&np->gref_rx_head);
>> + else
>> + gnt_ref = ref;
>> + BUG_ON((signed short)gnt_ref< 0);
>> +
>> + if (np->persistent_gnt) {
> So you are only using persistent grants if the backend also supports
> them.
Current implementation is:
If netback supports persistent grant, the frontend will work with
persistent grant feature too.
If netback does not support persistent grant, the frontend will work
without persistent grant feature.
> Have you benchmarked the performance of a persistent frontend with
> a non-persistent backend.
I remember I did some test before, not so sure. Will check it.
> In the block case, usign a persistent frontend
> with a non-persistent backend let to an overall performance improvement,
> so blkfront uses persistent grants even if blkback doesn't support them.
> Take a look at the following graph:
>
> http://xenbits.xen.org/people/royger/persistent_grants/nonpers_read.png
Good idea, that makes sense. I will change netfront too, thanks.
>
>> + page = alloc_page(GFP_KERNEL);
>> + if (!page) {
>> + if (!ref)
>> + gnttab_release_grant_reference(
>> +&np->gref_rx_head, ref);
>> + return -ENOMEM;
>> + }
>> + mfn = pfn_to_mfn(page_to_pfn(page));
>> +
>> + gnt_list_entry = kmalloc(sizeof(struct gnt_list),
>> + GFP_KERNEL);
>> + if (!gnt_list_entry) {
>> + __free_page(page);
>> + if (!ref)
>> + gnttab_release_grant_reference(
>> +&np->gref_rx_head, ref);
>> + return -ENOMEM;
>> + }
>> + gnt_list_entry->gref = gnt_ref;
>> + gnt_list_entry->gnt_pages = page;
>> + gnt_list_entry->gnt_target = vaddr;
>> +
>> + np->rx_grant[id] = gnt_list_entry;
>> + }
>> +
>> + gnttab_grant_foreign_access_ref(gnt_ref, np->xbdev->otherend_id,
>> + mfn, 0);
>> + }
>> + np->grant_rx_ref[id] = gnt_ref;
>> +
>> + return gnt_ref;
>> +}
>> +
>> static void xennet_alloc_rx_buffers(struct net_device *dev)
>> {
>> unsigned short id;
>> @@ -240,8 +328,6 @@ static void xennet_alloc_rx_buffers(struct net_device *dev)
>> int i, batch_target, notify;
>> RING_IDX req_prod = np->rx.req_prod_pvt;
>> grant_ref_t ref;
>> - unsigned long pfn;
>> - void *vaddr;
>> struct xen_netif_rx_request *req;
>>
>> if (unlikely(!netif_carrier_ok(dev)))
>> @@ -306,19 +392,16 @@ no_skb:
>> BUG_ON(np->rx_skbs[id]);
>> np->rx_skbs[id] = skb;
>>
>> - ref = gnttab_claim_grant_reference(&np->gref_rx_head);
>> - BUG_ON((signed short)ref< 0);
>> - np->grant_rx_ref[id] = ref;
>> + page = skb_frag_page(&skb_shinfo(skb)->frags[0]);
>>
>> - pfn = page_to_pfn(skb_frag_page(&skb_shinfo(skb)->frags[0]));
>> - vaddr = page_address(skb_frag_page(&skb_shinfo(skb)->frags[0]));
>> + ref = xennet_alloc_rx_ref(dev, pfn_to_mfn(page_to_pfn(page)),
>> + page_address(page), id, 0);
>> + if ((signed short)ref< 0) {
>> + __skb_queue_tail(&np->rx_batch, skb);
>> + break;
>> + }
>>
>> req = RING_GET_REQUEST(&np->rx, req_prod + i);
>> - gnttab_grant_foreign_access_ref(ref,
>> - np->xbdev->otherend_id,
>> - pfn_to_mfn(pfn),
>> - 0);
>> -
>> req->id = id;
>> req->gref = ref;
>> }
>> @@ -375,17 +458,30 @@ static void xennet_tx_buf_gc(struct net_device *dev)
>>
>> id = txrsp->id;
>> skb = np->tx_skbs[id].skb;
>> - if (unlikely(gnttab_query_foreign_access(
>> - np->grant_tx_ref[id]) != 0)) {
>> - printk(KERN_ALERT "xennet_tx_buf_gc: warning "
>> - "-- grant still in use by backend "
>> - "domain.\n");
>> - BUG();
>> +
>> + if (np->persistent_gnt) {
>> + struct gnt_list *gnt_list_entry;
>> +
>> + gnt_list_entry = np->tx_grant[id];
>> + BUG_ON(!gnt_list_entry);
>> +
>> + gnt_list_entry->tail = np->tx_gnt_list;
>> + np->tx_gnt_list = gnt_list_entry;
>> + np->tx_gnt_cnt++;
>> + } else {
>> + if (unlikely(gnttab_query_foreign_access(
>> + np->grant_tx_ref[id]) != 0)) {
>> + printk(KERN_ALERT "xennet_tx_buf_gc: warning "
>> + "-- grant still in use by backend "
>> + "domain.\n");
>> + BUG();
>> + }
>> +
>> + gnttab_end_foreign_access_ref(
>> + np->grant_tx_ref[id], GNTMAP_readonly);
> If I've read the code correctly, you are giving this frame both
> read/write permissions to the other end on xennet_alloc_tx_ref, but then
> you are only removing the read permissions? (see comment below on the
> xennet_alloc_tx_ref function).
Yes, this is a bug.
For non persistent grant, it should remove the read permissions. For
persistent grant, it should remove both.
As mentioned above, it is better to enable persistent grant, I will
change code and not consider non persistent grant.
See comments below about why needing both permissions in
xennet_alloc_tx_ref.
>
>> + gnttab_release_grant_reference(
>> +&np->gref_tx_head, np->grant_tx_ref[id]);
>> }
>> - gnttab_end_foreign_access_ref(
>> - np->grant_tx_ref[id], GNTMAP_readonly);
>> - gnttab_release_grant_reference(
>> -&np->gref_tx_head, np->grant_tx_ref[id]);
>> np->grant_tx_ref[id] = GRANT_INVALID_REF;
>> add_id_to_freelist(&np->tx_skb_freelist, np->tx_skbs, id);
>> dev_kfree_skb_irq(skb);
>> @@ -409,6 +505,59 @@ static void xennet_tx_buf_gc(struct net_device *dev)
>> xennet_maybe_wake_tx(dev);
>> }
>>
>> +static grant_ref_t xennet_alloc_tx_ref(struct net_device *dev,
>> + unsigned long mfn,
>> + unsigned int id)
>> +{
>> + struct netfront_info *np = netdev_priv(dev);
>> + grant_ref_t ref;
>> + struct page *granted_page;
>> +
>> + if (np->persistent_gnt&& np->tx_gnt_cnt) {
>> + struct gnt_list *gnt_list_entry;
>> +
>> + gnt_list_entry = np->tx_gnt_list;
>> + np->tx_gnt_list = np->tx_gnt_list->tail;
>> + np->tx_gnt_cnt--;
>> +
>> + ref = gnt_list_entry->gref;
>> + np->tx_grant[id] = gnt_list_entry;
>> + } else {
>> + struct gnt_list *gnt_list_entry;
>> +
>> + BUG_ON(!np->persistent_gnt&& np->tx_gnt_cnt);
>> + ref = gnttab_claim_grant_reference(&np->gref_tx_head);
>> + BUG_ON((signed short)ref< 0);
>> +
>> + if (np->persistent_gnt) {
>> + granted_page = alloc_page(GFP_KERNEL);
>> + if (!granted_page) {
>> + gnttab_release_grant_reference(
>> +&np->gref_tx_head, ref);
>> + return -ENOMEM;
>> + }
>> +
>> + mfn = pfn_to_mfn(page_to_pfn(granted_page));
>> + gnt_list_entry = kmalloc(sizeof(struct gnt_list),
>> + GFP_KERNEL);
>> + if (!gnt_list_entry) {
>> + __free_page(granted_page);
>> + gnttab_release_grant_reference(
>> +&np->gref_tx_head, ref);
>> + return -ENOMEM;
>> + }
>> +
>> + gnt_list_entry->gref = ref;
>> + gnt_list_entry->gnt_pages = granted_page;
>> + np->tx_grant[id] = gnt_list_entry;
>> + }
>> + gnttab_grant_foreign_access_ref(ref, np->xbdev->otherend_id,
>> + mfn, 0);
> If you are not always using persistent grants I guess you need to give
> read only permissions to this frame (GNTMAP_readonly).
We can not use GNTMAP_readonly here because tx path packet data will be
copied into these persistent grant pages. Mabbe it is better to use
GNTMAP_readonly for nonpersistent and 0 for persistent grant.
As mentioned above, it is better to enable persistent grant, I will
change code and not consider non persistent grant.
> Also, for keeping
> things in logical order, isn't it best that this function comes before
> xennet_tx_buf_gc?
xennet_alloc_tx_ref is called by following function xennet_make_frags,
so I assume xennet_alloc_tx_ref is better to be close to
xennet_make_frags. Xennet_tx_buf_gc does not have any connection with
xennet_alloc_tx_ref, did I miss something?
>
>> + }
>> +
>> + return ref;
>> +}
>> +
>> @@ -1132,8 +1357,10 @@ static void xennet_release_rx_bufs(struct netfront_info *np)
>> }
>>
>> skb = np->rx_skbs[id];
>> - mfn = gnttab_end_foreign_transfer_ref(ref);
>> - gnttab_release_grant_reference(&np->gref_rx_head, ref);
>> + if (!np->persistent_gnt) {
>> + mfn = gnttab_end_foreign_transfer_ref(ref);
>> + gnttab_release_grant_reference(&np->gref_rx_head, ref);
>> + }
>> np->grant_rx_ref[id] = GRANT_INVALID_REF;
>>
>> if (0 == mfn) {
>> @@ -1607,6 +1834,13 @@ again:
>> goto abort_transaction;
>> }
>>
>> + err = xenbus_printf(xbt, dev->nodename, "feature-persistent-grants",
>> + "%u", info->persistent_gnt);
> As in netback, I think "feature-persistent" should be used.
Same in blkback, I assume it is "feature-persistent-grants", right?
I referred your RFC patch, did you change it later? Or I missed something?
Thanks
Annie
>
>> + if (err) {
>> + message = "writing feature-persistent-grants";
>> + xenbus_dev_fatal(dev, err, "%s", message);
>> + }
>> +
>> err = xenbus_transaction_end(xbt, 0);
>> if (err) {
>> if (err == -EAGAIN)
>> @@ -1634,6 +1868,7 @@ static int xennet_connect(struct net_device *dev)
>> grant_ref_t ref;
>> struct xen_netif_rx_request *req;
>> unsigned int feature_rx_copy;
>> + int ret, val;
>>
>> err = xenbus_scanf(XBT_NIL, np->xbdev->otherend,
>> "feature-rx-copy", "%u",&feature_rx_copy);
>> @@ -1646,6 +1881,13 @@ static int xennet_connect(struct net_device *dev)
>> return -ENODEV;
>> }
>>
>> + err = xenbus_scanf(XBT_NIL, np->xbdev->otherend,
>> + "feature-persistent-grants", "%u",&val);
>> + if (err != 1)
>> + val = 0;
>> +
>> + np->persistent_gnt = !!val;
>> +
>> err = talk_to_netback(np->xbdev, np);
>> if (err)
>> return err;
>> @@ -1657,9 +1899,24 @@ static int xennet_connect(struct net_device *dev)
>> spin_lock_bh(&np->rx_lock);
>> spin_lock_irq(&np->tx_lock);
>>
>> + np->tx_gnt_cnt = 0;
>> + np->rx_gnt_cnt = 0;
>> +
>> /* Step 1: Discard all pending TX packet fragments. */
>> xennet_release_tx_bufs(np);
>>
>> + if (np->persistent_gnt) {
>> + struct gnt_list *gnt_list_entry;
>> +
>> + while (np->rx_gnt_list) {
>> + gnt_list_entry = np->rx_gnt_list;
>> + np->rx_gnt_list = np->rx_gnt_list->tail;
>> + gnttab_end_foreign_access(gnt_list_entry->gref, 0, 0UL);
>> + __free_page(gnt_list_entry->gnt_pages);
>> + kfree(gnt_list_entry);
>> + }
>> + }
>> +
>> /* Step 2: Rebuild the RX buffer freelist and the RX ring itself. */
>> for (requeue_idx = 0, i = 0; i< NET_RX_RING_SIZE; i++) {
>> skb_frag_t *frag;
>> @@ -1673,10 +1930,11 @@ static int xennet_connect(struct net_device *dev)
>>
>> frag =&skb_shinfo(skb)->frags[0];
>> page = skb_frag_page(frag);
>> - gnttab_grant_foreign_access_ref(
>> - ref, np->xbdev->otherend_id,
>> - pfn_to_mfn(page_to_pfn(page)),
>> - 0);
>> + ret = xennet_alloc_rx_ref(dev, pfn_to_mfn(page_to_pfn(page)),
>> + page_address(page), requeue_idx, ref);
>> + if ((signed short)ret< 0)
>> + break;
>> +
>> req->gref = ref;
>> req->id = requeue_idx;
>>
>> --
>> 1.7.3.4
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>>
^ permalink raw reply
* Re: Optics (SFP) monitoring on ixgbe and igbe
From: Jeff Kirsher @ 2012-11-16 6:25 UTC (permalink / raw)
To: footplus; +Cc: Ben Hutchings, netdev, Skidmore, Donald C
In-Reply-To: <CAPN4dA_ymo-Bx+GM+JLLKGHghq+qBYxR0zO7K6H6Nn0pqZsJRg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 809 bytes --]
On Fri, 2012-11-16 at 03:23 +0100, Aurélien wrote:
> On Fri, Nov 16, 2012 at 12:30 AM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> >
> > Yes, Jeff's the one you should be talking to about these drivers. I
> > just look after the ethtool utility and API.
> >
>
> Ok, so I will discuss the ixgbe patch with Jeff :)
Aurélien-
As far as your driver changes go and the questions you have, Don
Skidmore (ixgbe maintainer) and I have been discussing your patch and
may have some implementation changes possibly to suggest. I will talk
with Don more tomorrow about it before I suggest any changes.
As far as your questions regarding ixgbe PHY's, I will have Don respond
your questions. I have CC'd Don <donald.c.skidmore@intel.com> as well
on this email thread.
Cheers,
Jeff
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: GRO + splice panics in 3.7.0-rc5
From: Willy Tarreau @ 2012-11-16 6:39 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1353023344.10798.8.camel@edumazet-glaptop>
Hi Eric,
On Thu, Nov 15, 2012 at 03:49:04PM -0800, Eric Dumazet wrote:
> On Thu, 2012-11-15 at 23:28 +0100, Willy Tarreau wrote:
> > Hello,
> >
> > I was just about to make a quick comparison between LRO and GRO in
> > 3.7.0-rc5 to see if LRO still had the big advantage I've always observed,
> > but I failed the test because as soon as I enable LRO + splice, the kernel
> > panics and reboots.
> >
> > I could not yet manage to catch the panic output, I could just reliably
> > reproduce it, it crashes instantly.
> >
> > All I can say at the moment is the following :
> > - test consist in forwarding HTTP traffic between two NICs via haproxy
> > - driver used was myri10ge
> > - LRO + recv+send : OK
> > - LRO + splice : OK
> > - GRO + recv+send : OK
> > - GRO + splice : panic
> > - no such problem was observed in 3.6.6 so I think this is a recent
> > regression.
> >
> > I'll go back digging for more information, but as I'm used to often see
> > Eric suggest the right candidates for reverting, I wanted to report the
> > issue here in case there are easy ones to try first.
>
> Hi Willy
>
> Nothing particular comes to mind, there were a lot of recent changes
> that could trigger this kind of bug.
OK, no problem.
> A stack trace would be useful of course ;)
I'll arrange to get one, then to bisect. I'll also try to reproduce with
the onboard NIC (sky2) to see if this is something related to the driver
itself. I can't promise anything before the end of the week-end, though.
Thanks !
Willy
^ permalink raw reply
* Re: [PATCH] tcp: handle tcp_net_metrics_init() order-5 memory allocation failures
From: David Miller @ 2012-11-16 6:39 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, jln
In-Reply-To: <1353022864.10798.6.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 15 Nov 2012 15:41:04 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> order-5 allocations can fail with current kernels, we should
> try to reduce allocation sizes to allow network namespace
> creation.
>
> Reported-by: Julien Tinnes <jln@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Indeed, this has to be done better.
But this kind of retry solution results in non-deterministic behavior.
Yes the tcp metrics cache is best effort, but it's size can influence
behavior in a substantial way depending upon the workload.
I would suggest that we instead use different limits, ones which the
page allocator will satisfy for us always with GFP_KERNEL.
1) include linux/mmzone.h
2) Make the two limits based upon PAGE_ALLOC_COSTLY_ORDER.
That is, make the larger table size PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER
and the smaller one PAGE_SIZE << (PAGE_ALLOC_COSTLY_ORDER - 1).
How about something like this?
diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index 53bc584..d4b2d42 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -1,7 +1,6 @@
#include <linux/rcupdate.h>
#include <linux/spinlock.h>
#include <linux/jiffies.h>
-#include <linux/bootmem.h>
#include <linux/module.h>
#include <linux/cache.h>
#include <linux/slab.h>
@@ -9,6 +8,7 @@
#include <linux/tcp.h>
#include <linux/hash.h>
#include <linux/tcp_metrics.h>
+#include <linux/mmzone.h>
#include <net/inet_connection_sock.h>
#include <net/net_namespace.h>
@@ -1025,10 +1025,12 @@ static int __net_init tcp_net_metrics_init(struct net *net)
slots = tcpmhash_entries;
if (!slots) {
- if (totalram_pages >= 128 * 1024)
- slots = 16 * 1024;
- else
- slots = 8 * 1024;
+ int order = PAGE_ALLOC_COSTLY_ORDER;
+
+ if (totalram_pages < 128 * 1024)
+ order--;
+ slots = (PAGE_SIZE << order) /
+ sizeof(struct tcpm_hash_bucket);
}
net->ipv4.tcp_metrics_hash_log = order_base_2(slots);
^ permalink raw reply related
* Re: [PATCH] tilegx: request_irq with a non-null device name
From: David Miller @ 2012-11-16 6:40 UTC (permalink / raw)
To: simon.marchi; +Cc: netdev, linux-kernel, cmetcalf
In-Reply-To: <1353039199-12520-1-git-send-email-simon.marchi@polymtl.ca>
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Thu, 15 Nov 2012 23:13:19 -0500
> This patch simply makes the tilegx net driver call request_irq with a
> non-null name. It makes the output in /proc/interrupts more obvious, but
> also helps tools that don't expect to find null there.
>
> Signed-off-by: Simon Marchi <simon.marchi@polymtl.ca>
> Acked-by: Chris Metcalf <cmetcalf@tilera.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: export IP6_RT_PRIO_* to userland
From: David Miller @ 2012-11-16 6:48 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: netdev
In-Reply-To: <5098127A.2040405@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Mon, 05 Nov 2012 20:24:42 +0100
> In IPv4, there is no such default metric. If you add a route with
> metric X, it remains X in the kernel, even if it's 0.
Ok, fair enough, applied to net-next, thanks.
^ permalink raw reply
* [PATCH 00/14] Modify signed comparisons of unsigned variables
From: Tushar Behera @ 2012-11-16 6:50 UTC (permalink / raw)
To: linux-kernel
Cc: patches, Mauro Carvalho Chehab, Linus Walleij, Ian Campbell,
Konrad Rzeszutek Wilk, Jeremy Fitzhardinge, Chas Williams,
Jack Steiner, Arnd Bergmann, Luciano Coelho, Jiri Kosina,
ivtv-devel, linux-media, xen-devel, netdev, virtualization,
linux-atm-general, linux-usb, linux-input, linux-wireless
The occurrences were identified through the coccinelle script at
following location.
http://www.emn.fr/z-info/coccinelle/rules/find_unsigned.cocci
Signed checks for unsigned variables are removed if it is also checked
for upper error limit. For error checks, IS_ERR_VALUE() macros is used.
Tushar Behera (14):
[media] ivtv: Remove redundant check on unsigned variable
[media] meye: Remove redundant check on unsigned variable
[media] saa7134: Remove redundant check on unsigned variable
[media] tlg2300: Remove redundant check on unsigned variable
[media] atmel-isi: Update error check for unsigned variables
pinctrl: samsung: Update error check for unsigned variables
pinctrl: SPEAr: Update error check for unsigned variables
xen: netback: Remove redundant check on unsigned variable
xen: events: Remove redundant check on unsigned variable
atm: Removed redundant check on unsigned variable
HID: hiddev: Remove redundant check on unsigned variable
gru: Remove redundant check on unsigned variable
misc: tsl2550: Remove redundant check on unsigned variable
wlcore: Remove redundant check on unsigned variable
drivers/atm/fore200e.c | 2 +-
drivers/hid/usbhid/hiddev.c | 2 +-
drivers/media/pci/ivtv/ivtv-ioctl.c | 2 +-
drivers/media/pci/meye/meye.c | 2 +-
drivers/media/pci/saa7134/saa7134-video.c | 2 +-
drivers/media/platform/soc_camera/atmel-isi.c | 2 +-
drivers/media/usb/tlg2300/pd-video.c | 2 +-
drivers/misc/sgi-gru/grukdump.c | 2 +-
drivers/misc/tsl2550.c | 4 ++--
drivers/net/wireless/ti/wlcore/debugfs.c | 2 +-
drivers/net/xen-netback/netback.c | 4 ++--
drivers/pinctrl/pinctrl-samsung.c | 2 +-
drivers/pinctrl/spear/pinctrl-plgpio.c | 2 +-
drivers/xen/events.c | 2 +-
14 files changed, 16 insertions(+), 16 deletions(-)
--
1.7.4.1
CC: Mauro Carvalho Chehab <mchehab@infradead.org>
CC: Linus Walleij <linus.walleij@linaro.org>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Jeremy Fitzhardinge <jeremy@goop.org>
CC: Chas Williams <chas@cmf.nrl.navy.mil>
CC: Jack Steiner <steiner@sgi.com>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Luciano Coelho <coelho@ti.com>
CC: Jiri Kosina <jkosina@suse.cz>
CC: ivtv-devel@ivtvdriver.org
CC: linux-media@vger.kernel.org
CC: xen-devel@lists.xensource.com
CC: netdev@vger.kernel.org
CC: virtualization@lists.linux-foundation.org
CC: linux-atm-general@lists.sourceforge.net
CC: linux-usb@vger.kernel.org
CC: linux-input@vger.kernel.org
CC: linux-wireless@vger.kernel.org
^ permalink raw reply
* [PATCH 08/14] xen: netback: Remove redundant check on unsigned variable
From: Tushar Behera @ 2012-11-16 6:50 UTC (permalink / raw)
To: linux-kernel; +Cc: patches, Ian Campbell, xen-devel, netdev
In-Reply-To: <1353048646-10935-1-git-send-email-tushar.behera@linaro.org>
No need to check whether unsigned variable is less than 0.
CC: Ian Campbell <ian.campbell@citrix.com>
CC: xen-devel@lists.xensource.com
CC: netdev@vger.kernel.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
drivers/net/xen-netback/netback.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index aab8677..515e10c 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -190,14 +190,14 @@ static int get_page_ext(struct page *pg,
group = ext.e.group - 1;
- if (group < 0 || group >= xen_netbk_group_nr)
+ if (group >= xen_netbk_group_nr)
return 0;
netbk = &xen_netbk[group];
idx = ext.e.idx;
- if ((idx < 0) || (idx >= MAX_PENDING_REQS))
+ if (idx >= MAX_PENDING_REQS)
return 0;
if (netbk->mmap_pages[idx] != pg)
--
1.7.4.1
^ permalink raw reply related
* [PATCH 10/14] atm: Removed redundant check on unsigned variable
From: Tushar Behera @ 2012-11-16 6:50 UTC (permalink / raw)
To: linux-kernel; +Cc: patches, Chas Williams, linux-atm-general, netdev
In-Reply-To: <1353048646-10935-1-git-send-email-tushar.behera@linaro.org>
No need to check whether unsigned variable is less than 0.
CC: Chas Williams <chas@cmf.nrl.navy.mil>
CC: linux-atm-general@lists.sourceforge.net
CC: netdev@vger.kernel.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
drivers/atm/fore200e.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/atm/fore200e.c b/drivers/atm/fore200e.c
index 361f5ae..fdd3fe7 100644
--- a/drivers/atm/fore200e.c
+++ b/drivers/atm/fore200e.c
@@ -972,7 +972,7 @@ int bsq_audit(int where, struct host_bsq* bsq, int scheme, int magn)
where, scheme, magn, buffer->index, buffer->scheme);
}
- if ((buffer->index < 0) || (buffer->index >= fore200e_rx_buf_nbr[ scheme ][ magn ])) {
+ if (buffer->index >= fore200e_rx_buf_nbr[ scheme ][ magn ]) {
printk(FORE200E "bsq_audit(%d): queue %d.%d, out of range buffer index = %ld !\n",
where, scheme, magn, buffer->index);
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 14/14] wlcore: Remove redundant check on unsigned variable
From: Tushar Behera @ 2012-11-16 6:50 UTC (permalink / raw)
To: linux-kernel; +Cc: patches, Luciano Coelho, linux-wireless, netdev
In-Reply-To: <1353048646-10935-1-git-send-email-tushar.behera@linaro.org>
No need to check whether unsigned variable is less than 0.
CC: Luciano Coelho <coelho@ti.com>
CC: linux-wireless@vger.kernel.org
CC: netdev@vger.kernel.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
drivers/net/wireless/ti/wlcore/debugfs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/debugfs.c b/drivers/net/wireless/ti/wlcore/debugfs.c
index c86bb00..93f801d 100644
--- a/drivers/net/wireless/ti/wlcore/debugfs.c
+++ b/drivers/net/wireless/ti/wlcore/debugfs.c
@@ -993,7 +993,7 @@ static ssize_t sleep_auth_write(struct file *file,
return -EINVAL;
}
- if (value < 0 || value > WL1271_PSM_MAX) {
+ if (value > WL1271_PSM_MAX) {
wl1271_warning("sleep_auth must be between 0 and %d",
WL1271_PSM_MAX);
return -ERANGE;
--
1.7.4.1
^ permalink raw reply related
* Re: [tcpdump-workers] vlan tagged packets and libpcap breakage
From: Eric W. Biederman @ 2012-11-16 6:51 UTC (permalink / raw)
To: Ani Sinha; +Cc: Michael Richardson, netdev, tcpdump-workers, Francesco Ruggeri
In-Reply-To: <CAOxq_8PsXb-oUy85Eji7GSAbUZD_Bds8skmGNP4BWSewQWx8PA@mail.gmail.com>
Ani Sinha <ani@aristanetworks.com> writes:
> cc'ing netdev.
>
>
> On Wed, Oct 31, 2012 at 2:01 PM, Michael Richardson <mcr@sandelman.ca> wrote:
>>
>> Thanks for this email.
>>
>>>>>>> "Ani" == Ani Sinha <ani@aristanetworks.com> writes:
>> Ani> remove "inline" from vlan_core.c functions
>> Ani> Signed-off-by: David S. Miller <davem@davemloft.net>
>>
>> Ani> As a result of this patch, with or without HW acceleration support in
>> Ani> the kernel driver, the vlan tag information is pulled out from the
>> Ani> packet and is inserted in the skb metadata. Thereafter, the vlan tag
>> Ani> information for a 802.1q packet can be obtained my applications
>> Ani> running in the user space by using the auxdata and cmsg
>> Ani> structure.
>>
>> Do you think that the existance of this behaviour could be exposed in
>> sysctl, /proc/net or /sys equivalent (we still don't have /sys/net...)?
>> As a read only file that had a 0/1 in it?
>
> yes, we definitely need a run time check. Whether this could be in the
> form of a socket option or a /proc entry I don't know.
I don't see any need to add any kernel code to allow checking if vlan
tags are stripped. Vlan headers are stripped on all kernel interfaces
today. Vlan headers have been stripped on all but a handful of software
interfaces for 6+ years. For all kernels if the vlan header is stripped
it is reported in the auxdata, upon packet reception. Careful code
should also look for TP_STATUS_VLAN_VALID which allows for
distinguishing a striped vlan header of 0 from no vlan header.
The safe assumption then is that testing for vlan headers and vlan
values in bpf filters is not possible without the new bpf extentions.
It is possible to test for the presence of support of the new vlan bpf
extensions by attempting to load a filter that uses them. As only valid
filters can be loaded, old kernels that do not support filtering of vlan
tags will fail to load the a test filter with uses them.
For old kernels that do not support the new extensions it is possible to
generate code that looks at the ethernet header and sees if the
ethertype is 0x8100 and then does things with it, but that will only
work on a small handful of software only interfaces.
Eric
^ permalink raw reply
* Re: [Xen-devel] [PATCH 1/4] xen/netback: implements persistent grant with one page pool.
From: ANNIE LI @ 2012-11-16 7:57 UTC (permalink / raw)
To: Roger Pau Monné
Cc: xen-devel@lists.xensource.com, netdev@vger.kernel.org,
konrad.wilk@oracle.com, Ian Campbell
In-Reply-To: <50A5A9CF.8030008@oracle.com>
On 2012-11-16 10:49, ANNIE LI wrote:
>
>
> On 2012-11-15 17:57, Roger Pau Monné wrote:
>>>
>>> @@ -453,7 +460,12 @@ static int connect_rings(struct backend_info *be)
>>> val = 0;
>>> vif->csum = !val;
>>>
>>> - /* Map the shared frame, irq etc. */
>>> + if (xenbus_scanf(XBT_NIL, dev->otherend,
>>> "feature-persistent-grants",
>>> + "%u",&val)< 0)
>> In block devices "feature-persistent" is used, so I think that for
>> clearness it should be announced the same way in net.
> Is it "feature-persistent" ? I checked your RFC patch, the key is
> "feature-persistent-grants".
>
>
My mistake.
In your v2 patch, it is "feature-persistent". I will change the code as
blkback/blkfront.
Thanks
Annie
^ permalink raw reply
* Re: [Xen-devel] [PATCH 3/4] Xen/netfront: Implement persistent grant in netfront.
From: ANNIE LI @ 2012-11-16 7:58 UTC (permalink / raw)
To: Roger Pau Monné
Cc: netdev@vger.kernel.org, xen-devel@lists.xensource.com,
Ian Campbell, konrad.wilk@oracle.com
In-Reply-To: <50A5CD7F.2010609@oracle.com>
On 2012-11-16 13:22, ANNIE LI wrote:
>
>
> On 2012-11-15 18:52, Roger Pau Monné wrote:
>>> + err = xenbus_printf(xbt, dev->nodename,
>>> "feature-persistent-grants",
>>> + "%u", info->persistent_gnt);
>> As in netback, I think "feature-persistent" should be used.
>
> Same in blkback, I assume it is "feature-persistent-grants", right?
> I referred your RFC patch, did you change it later? Or I missed
> something?
>
>
My mistake.
In your v2 patch, it is "feature-persistent". I will change the code as
blkback/blkfront.
Thanks
Annie
^ permalink raw reply
* Re: [PATCH 0/9 v4] use efficient this_cpu_* helper
From: Shan Wei @ 2012-11-16 8:30 UTC (permalink / raw)
To: Tejun Heo, David Miller, paulmck, rostedt
Cc: Christoph Lameter, NetDev, Kernel-Maillist
In-Reply-To: <20121115145325.GC7306@mtj.dyndns.org>
Hi Tejun Heo:
Tejun Heo said, at 2012/11/15 22:53:
> On Thu, Nov 15, 2012 at 02:19:38PM +0000, Christoph Lameter wrote:
>> Tejon: Could you pick up this patchset?
>
> Sure, but, Shan, when posting patchset, please make the patches
> replies to the head message; otherwise, it's pretty difficult to track
> what's going on with the patchset as a whole. I see that some patches
> are being picked up by respective subsystems. If you have patches
> left, please let me know.
OK, next time i will do as you suggest.
This patchset include more subsystem, i.e network, rcu, trace.
The best way to avoid code conflict is subsystem maintainer to pick them up
to their code tree. I will remind them in each patch that not yet applied and
add you to the receiver list.
Best Regards
Shan Wei
>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH v4 4/9] net: openvswitch: use this_cpu_ptr per-cpu helper
From: Shan Wei @ 2012-11-16 8:35 UTC (permalink / raw)
To: jesse-l0M0P4e3n4LQT0dZR+AlfA
Cc: dev-yBygre7rU0TnMu66kgdUjQ, Tejun Heo,
cl-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, NetDev, Kernel-Maillist,
Shan Wei, David Miller
In-Reply-To: <50A1A7D9.2040506-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Shan Wei said, at 2012/11/13 9:52:
> From: Shan Wei <davidshan-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
>
> just use more faster this_cpu_ptr instead of per_cpu_ptr(p, smp_processor_id());
>
>
> Signed-off-by: Shan Wei <davidshan-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
> Reviewed-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
Jesse Gross, would you like to pick it up to your tree?
> ---
> no changes vs v3,v2.
> ---
> net/openvswitch/datapath.c | 4 ++--
> net/openvswitch/vport.c | 5 ++---
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 4c4b62c..77d16a5 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -208,7 +208,7 @@ void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
> int error;
> int key_len;
>
> - stats = per_cpu_ptr(dp->stats_percpu, smp_processor_id());
> + stats = this_cpu_ptr(dp->stats_percpu);
>
> /* Extract flow from 'skb' into 'key'. */
> error = ovs_flow_extract(skb, p->port_no, &key, &key_len);
> @@ -282,7 +282,7 @@ int ovs_dp_upcall(struct datapath *dp, struct sk_buff *skb,
> return 0;
>
> err:
> - stats = per_cpu_ptr(dp->stats_percpu, smp_processor_id());
> + stats = this_cpu_ptr(dp->stats_percpu);
>
> u64_stats_update_begin(&stats->sync);
> stats->n_lost++;
> diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
> index 03779e8..70af0be 100644
> --- a/net/openvswitch/vport.c
> +++ b/net/openvswitch/vport.c
> @@ -333,8 +333,7 @@ void ovs_vport_receive(struct vport *vport, struct sk_buff *skb)
> {
> struct vport_percpu_stats *stats;
>
> - stats = per_cpu_ptr(vport->percpu_stats, smp_processor_id());
> -
> + stats = this_cpu_ptr(vport->percpu_stats);
> u64_stats_update_begin(&stats->sync);
> stats->rx_packets++;
> stats->rx_bytes += skb->len;
> @@ -359,7 +358,7 @@ int ovs_vport_send(struct vport *vport, struct sk_buff *skb)
> if (likely(sent)) {
> struct vport_percpu_stats *stats;
>
> - stats = per_cpu_ptr(vport->percpu_stats, smp_processor_id());
> + stats = this_cpu_ptr(vport->percpu_stats);
>
> u64_stats_update_begin(&stats->sync);
> stats->tx_packets++;
>
^ permalink raw reply
* Re: [PATCH v4 1/9] net: core: use this_cpu_ptr per-cpu helper
From: Shan Wei @ 2012-11-16 8:38 UTC (permalink / raw)
To: David Miller
Cc: Shan Wei, timo.teras, steffen.klassert, NetDev, Kernel-Maillist,
cl, Tejun Heo
In-Reply-To: <50A1A7BA.5000507@gmail.com>
Shan Wei said, at 2012/11/13 9:51:
> From: Shan Wei <davidshan@tencent.com>
>
> flush_tasklet is a struct, not a pointer in percpu var.
> so use this_cpu_ptr to get the member pointer.
>
> Signed-off-by: Shan Wei <davidshan@tencent.com>
> Reviewed-by: Christoph Lameter <cl@linux.com>
David Miller, would you like to pick it up to your net-next tree?
> ---
> no changes vs v3.
> ---
> net/core/flow.c | 4 +---
> 1 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/flow.c b/net/core/flow.c
> index e318c7e..b0901ee 100644
> --- a/net/core/flow.c
> +++ b/net/core/flow.c
> @@ -327,11 +327,9 @@ static void flow_cache_flush_tasklet(unsigned long data)
> static void flow_cache_flush_per_cpu(void *data)
> {
> struct flow_flush_info *info = data;
> - int cpu;
> struct tasklet_struct *tasklet;
>
> - cpu = smp_processor_id();
> - tasklet = &per_cpu_ptr(info->cache->percpu, cpu)->flush_tasklet;
> + tasklet = this_cpu_ptr(&info->cache->percpu->flush_tasklet);
> tasklet->data = (unsigned long)info;
> tasklet_schedule(tasklet);
> }
>
^ permalink raw reply
* Re: [PATCH v4 2/9] net: rds: use this_cpu_* per-cpu helper
From: Shan Wei @ 2012-11-16 8:38 UTC (permalink / raw)
To: David Miller
Cc: Shan Wei, venkat.x.venkatsubra, rds-devel, NetDev,
Kernel-Maillist, cl, Tejun Heo
In-Reply-To: <50A1A7C1.50308@gmail.com>
Shan Wei said, at 2012/11/13 9:52:
> From: Shan Wei <davidshan@tencent.com>
>
>
> Signed-off-by: Shan Wei <davidshan@tencent.com>
> Reviewed-by: Christoph Lameter <cl@linux.com>
David Miller, would you like to pick it up to your net-next tree?
> ---
> v4:
> 1. add missing __percpu annotations.
> 2. [read|write]ing fields of struct rds_ib_cache_head
> using __this_cpu_* operation, drop per_cpu_ptr.
> ---
> net/rds/ib.h | 2 +-
> net/rds/ib_recv.c | 24 +++++++++++++-----------
> 2 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/net/rds/ib.h b/net/rds/ib.h
> index 8d2b3d5..7280ab8 100644
> --- a/net/rds/ib.h
> +++ b/net/rds/ib.h
> @@ -50,7 +50,7 @@ struct rds_ib_cache_head {
> };
>
> struct rds_ib_refill_cache {
> - struct rds_ib_cache_head *percpu;
> + struct rds_ib_cache_head __percpu *percpu;
> struct list_head *xfer;
> struct list_head *ready;
> };
> diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
> index 8d19491..8c5bc85 100644
> --- a/net/rds/ib_recv.c
> +++ b/net/rds/ib_recv.c
> @@ -418,20 +418,21 @@ static void rds_ib_recv_cache_put(struct list_head *new_item,
> struct rds_ib_refill_cache *cache)
> {
> unsigned long flags;
> - struct rds_ib_cache_head *chp;
> struct list_head *old;
> + struct list_head __percpu *chpfirst;
>
> local_irq_save(flags);
>
> - chp = per_cpu_ptr(cache->percpu, smp_processor_id());
> - if (!chp->first)
> + chpfirst = __this_cpu_read(cache->percpu->first);
> + if (!chpfirst)
> INIT_LIST_HEAD(new_item);
> else /* put on front */
> - list_add_tail(new_item, chp->first);
> - chp->first = new_item;
> - chp->count++;
> + list_add_tail(new_item, chpfirst);
>
> - if (chp->count < RDS_IB_RECYCLE_BATCH_COUNT)
> + __this_cpu_write(chpfirst, new_item);
> + __this_cpu_inc(cache->percpu->count);
> +
> + if (__this_cpu_read(cache->percpu->count) < RDS_IB_RECYCLE_BATCH_COUNT)
> goto end;
>
> /*
> @@ -443,12 +444,13 @@ static void rds_ib_recv_cache_put(struct list_head *new_item,
> do {
> old = xchg(&cache->xfer, NULL);
> if (old)
> - list_splice_entire_tail(old, chp->first);
> - old = cmpxchg(&cache->xfer, NULL, chp->first);
> + list_splice_entire_tail(old, chpfirst);
> + old = cmpxchg(&cache->xfer, NULL, chpfirst);
> } while (old);
>
> - chp->first = NULL;
> - chp->count = 0;
> +
> + __this_cpu_write(chpfirst, NULL);
> + __this_cpu_write(cache->percpu->count, 0);
> end:
> local_irq_restore(flags);
> }
>
^ permalink raw reply
* pull request: batman-adv 2012-11-16
From: Antonio Quartulli @ 2012-11-16 8:49 UTC (permalink / raw)
To: davem
Cc: netdev, Simon Wunderlich, Marek Lindner, Sven Eckelmann,
Antonio Quartulli
Hello David,
here is small set of fixes intended for net/linux-3.7.
These patches are fixing some interoperability problems due to the features we
added in 3.7. Mainly we have two big issues: one is preventing clients connected
to the mesh network to contact any other hosts, caused by a not proper
translation table handling; the second one compromises the AP isolation feature
causing it to be completely useless, no matter it was on or off.
Please, let me know if there is any problem.
Thank you very much!
Antonio
The following changes since commit 80d11788fb8f4d9fcfae5ad508c7f1b65e8b28a3:
Revert "drivers/net/phy/mdio-bitbang.c: Call mdiobus_unregister before mdiobus_free" (2012-11-14 22:32:15 -0500)
are available in the git repository at:
git://git.open-mesh.org/linux-merge.git tags/batman-adv-fix-for-davem
for you to fetch changes up to 74490f969155caf1ec945ad2d35d3a8eec6be71d:
batman-adv: process broadcast packets in BLA earlier (2012-11-16 09:36:54 +0100)
----------------------------------------------------------------
Included fixes are:
- update the client entry status flags when using the "early client
detection". This makes the Distributed AP isolation correctly work;
- transfer the client entry status flags when recovering the translation
table from another node. This makes the Distributed AP isolation correctly
work;
- prevent the "early client detection mechanism" to add clients belonging to
other backbone nodes in the same LAN. This breaks connectivity when using this
mechanism together with the Bridge Loop Avoidance
- process broadcast packets with the Bridge Loop Avoidance before any other
component. BLA can possibly drop the packets based on the source address. This
makes the "early client detection mechanism" correctly work when used with
BLA.
----------------------------------------------------------------
Antonio Quartulli (4):
batman-adv: fix tt_global_entries flags update
batman-adv: correctly pass the client flag on tt_response
batman-adv: don't add TEMP clients belonging to other backbone nodes
batman-adv: process broadcast packets in BLA earlier
net/batman-adv/soft-interface.c | 12 ++++++------
net/batman-adv/translation-table.c | 15 ++++++++++++++-
2 files changed, 20 insertions(+), 7 deletions(-)
^ permalink raw reply
* [PATCH 1/4] batman-adv: fix tt_global_entries flags update
From: Antonio Quartulli @ 2012-11-16 8:49 UTC (permalink / raw)
To: davem
Cc: netdev, Simon Wunderlich, Marek Lindner, Sven Eckelmann,
Antonio Quartulli
In-Reply-To: <1353055758-2901-1-git-send-email-ordex@autistici.org>
Flags carried by a change_entry have to be always copied into the
client entry as they may contain important attributes (e.g.
TT_CLIENT_WIFI).
For instance, a client added by means of the "early detection
mechanism" has no flag set at the beginning, so they must be updated once the
proper ADD event is received.
This was introduced by 30cfd02b60e1cb16f5effb0a01f826c5bb7e4c59
("batman-adv: detect not yet announced clients")
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
net/batman-adv/translation-table.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 112edd3..64c0012 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -769,6 +769,12 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
*/
tt_global_entry->common.flags &= ~BATADV_TT_CLIENT_TEMP;
+ /* the change can carry possible "attribute" flags like the
+ * TT_CLIENT_WIFI, therefore they have to be copied in the
+ * client entry
+ */
+ tt_global_entry->common.flags |= flags;
+
/* If there is the BATADV_TT_CLIENT_ROAM flag set, there is only
* one originator left in the list and we previously received a
* delete + roaming change for this originator.
--
1.8.0
^ permalink raw reply related
* [PATCH 2/4] batman-adv: correctly pass the client flag on tt_response
From: Antonio Quartulli @ 2012-11-16 8:49 UTC (permalink / raw)
To: davem
Cc: netdev, Simon Wunderlich, Marek Lindner, Sven Eckelmann,
Antonio Quartulli
In-Reply-To: <1353055758-2901-1-git-send-email-ordex@autistici.org>
When a TT response with the full table is sent, the client flags
should be sent as well. This patch fix the flags assignment when
populating the tt_response to send back
This was introduced by 30cfd02b60e1cb16f5effb0a01f826c5bb7e4c59
("batman-adv: detect not yet announced clients")
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
net/batman-adv/translation-table.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 64c0012..fec1a00 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -1502,7 +1502,7 @@ batadv_tt_response_fill_table(uint16_t tt_len, uint8_t ttvn,
memcpy(tt_change->addr, tt_common_entry->addr,
ETH_ALEN);
- tt_change->flags = BATADV_NO_FLAGS;
+ tt_change->flags = tt_common_entry->flags;
tt_count++;
tt_change++;
--
1.8.0
^ permalink raw reply related
* [PATCH 3/4] batman-adv: don't add TEMP clients belonging to other backbone nodes
From: Antonio Quartulli @ 2012-11-16 8:49 UTC (permalink / raw)
To: davem
Cc: netdev, Simon Wunderlich, Marek Lindner, Sven Eckelmann,
Antonio Quartulli
In-Reply-To: <1353055758-2901-1-git-send-email-ordex@autistici.org>
The "early client detection" mechanism must not add clients belonging
to other backbone nodes. Such clients must be reached by directly
using the LAN instead of the mesh.
This was introduced by 30cfd02b60e1cb16f5effb0a01f826c5bb7e4c59
("batman-adv: detect not yet announced clients")
Reported-by: Glen Page <glen.page@thet.net>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
net/batman-adv/translation-table.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index fec1a00..baae715 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -2456,6 +2456,13 @@ bool batadv_tt_add_temporary_global_entry(struct batadv_priv *bat_priv,
{
bool ret = false;
+ /* if the originator is a backbone node (meaning it belongs to the same
+ * LAN of this node) the temporary client must not be added because to
+ * reach such destination the node must use the LAN instead of the mesh
+ */
+ if (batadv_bla_is_backbone_gw_orig(bat_priv, orig_node->orig))
+ goto out;
+
if (!batadv_tt_global_add(bat_priv, orig_node, addr,
BATADV_TT_CLIENT_TEMP,
atomic_read(&orig_node->last_ttvn)))
--
1.8.0
^ permalink raw reply related
* [PATCH 4/4] batman-adv: process broadcast packets in BLA earlier
From: Antonio Quartulli @ 2012-11-16 8:49 UTC (permalink / raw)
To: davem
Cc: netdev, Simon Wunderlich, Marek Lindner, Sven Eckelmann,
Antonio Quartulli, Simon Wunderlich
In-Reply-To: <1353055758-2901-1-git-send-email-ordex@autistici.org>
The logic in the BLA mechanism may decide to drop broadcast packets
because the node may still be in the setup phase. For this reason,
further broadcast processing like the early client detection mechanism
must be done only after the BLA check.
This patches moves the invocation to BLA before any other broadcast
processing.
This was introduced 30cfd02b60e1cb16f5effb0a01f826c5bb7e4c59
("batman-adv: detect not yet announced clients")
Reported-by: Glen Page <glen.page@thet.net>
Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
net/batman-adv/soft-interface.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index b9a28d2..ce0684a 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -325,6 +325,12 @@ void batadv_interface_rx(struct net_device *soft_iface,
soft_iface->last_rx = jiffies;
+ /* Let the bridge loop avoidance check the packet. If will
+ * not handle it, we can safely push it up.
+ */
+ if (batadv_bla_rx(bat_priv, skb, vid, is_bcast))
+ goto out;
+
if (orig_node)
batadv_tt_add_temporary_global_entry(bat_priv, orig_node,
ethhdr->h_source);
@@ -332,12 +338,6 @@ void batadv_interface_rx(struct net_device *soft_iface,
if (batadv_is_ap_isolated(bat_priv, ethhdr->h_source, ethhdr->h_dest))
goto dropped;
- /* Let the bridge loop avoidance check the packet. If will
- * not handle it, we can safely push it up.
- */
- if (batadv_bla_rx(bat_priv, skb, vid, is_bcast))
- goto out;
-
netif_rx(skb);
goto out;
--
1.8.0
^ permalink raw reply related
* Re: [PATCH 08/14] xen: netback: Remove redundant check on unsigned variable
From: Ian Campbell @ 2012-11-16 9:16 UTC (permalink / raw)
To: Tushar Behera
Cc: linux-kernel@vger.kernel.org, patches@linaro.org,
xen-devel@lists.xensource.com, netdev@vger.kernel.org
In-Reply-To: <1353048646-10935-9-git-send-email-tushar.behera@linaro.org>
On Fri, 2012-11-16 at 06:50 +0000, Tushar Behera wrote:
> No need to check whether unsigned variable is less than 0.
>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: xen-devel@lists.xensource.com
> CC: netdev@vger.kernel.org
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Thanks.
> ---
> drivers/net/xen-netback/netback.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index aab8677..515e10c 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -190,14 +190,14 @@ static int get_page_ext(struct page *pg,
>
> group = ext.e.group - 1;
>
> - if (group < 0 || group >= xen_netbk_group_nr)
> + if (group >= xen_netbk_group_nr)
> return 0;
>
> netbk = &xen_netbk[group];
>
> idx = ext.e.idx;
>
> - if ((idx < 0) || (idx >= MAX_PENDING_REQS))
> + if (idx >= MAX_PENDING_REQS)
> return 0;
>
> if (netbk->mmap_pages[idx] != pg)
^ permalink raw reply
* Re: [Xen-devel] [PATCH 1/4] xen/netback: implements persistent grant with one page pool.
From: Ian Campbell @ 2012-11-16 9:27 UTC (permalink / raw)
To: ANNIE LI
Cc: xen-devel@lists.xensource.com, netdev@vger.kernel.org,
konrad.wilk@oracle.com
In-Reply-To: <50A5A285.1030805@oracle.com>
On Fri, 2012-11-16 at 02:18 +0000, ANNIE LI wrote:
> In this patch,
> The maximum of memory overhead is about
>
> (XEN_NETIF_TX_RING_SIZE+XEN_NETIF_RX_RING_SIZE)*PAGE_SIZE (plus size of grant_ref_t and handle)
> which is about 512 PAGE_SIZE. Normally, without heavy network offload, this maximum can not be reached.
>
> In next patch of splitting tx/rx pool, the maximum is about
"about" or just "is"?
> (256+512)PAGE_SIZE.
IOW 3MB.
> >
> >> +
> >> + return NULL;
> >> +}
> >> +
> >> @@ -1338,7 +1497,11 @@ static unsigned xen_netbk_tx_build_gops(struct xen_netbk *netbk)
> >> gop->source.domid = vif->domid;
> >> gop->source.offset = txreq.offset;
> >>
> >> - gop->dest.u.gmfn = virt_to_mfn(page_address(page));
> >> + if (!vif->persistent_grant)
> >> + gop->dest.u.gmfn = virt_to_mfn(page_address(page));
> >> + else
> >> + gop->dest.u.gmfn = (unsigned long)page_address(page);
> > page_address doesn't return any sort of frame number, does it? This is
> > rather confusing...
>
> Yes. I only use dest.u.gmfn element to save the page_address here for
> future memcpy, and it does not mean to use frame number actually. To
> avoid confusion, here I can use
>
> gop->dest.u.gmfn = virt_to_mfn(page_address(page));
>
> and then call mfn_to_virt when doing memcpy.
It seems a bit odd to be using the gop structure in this way when you
aren't actually doing a grant op on it.
While investigating I noticed:
+static int
+grant_memory_copy_op(unsigned int cmd, void *vuop, unsigned int count,
+ struct xen_netbk *netbk, bool tx_pool)
...
+ struct gnttab_copy *uop = vuop;
Why void *vuop? Why not struct gnttab_copy * in the parameter?
I also noticed your new grant_memory_copy_op() seems to have unbatched
the grant ops in the non-persistent case, which is going to suck for
performance in non-persistent mode. You need to pull the conditional and
the HYPERVISOR_grant_table_op outside the loop and pass it full array
instead of doing them one at a time.
Ian
^ 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