* [PATCH v2 5/8] page_frag_cache: Save memory on small machines
From: Matthew Wilcox @ 2018-03-22 15:31 UTC (permalink / raw)
To: Alexander Duyck
Cc: Matthew Wilcox, netdev, linux-mm, Jesper Dangaard Brouer,
Eric Dumazet
In-Reply-To: <20180322153157.10447-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Only allocate a single page if CONFIG_BASE_SMALL is set.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
include/linux/mm_types.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index a63b138ad1a4..0defff9e3c0e 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -216,7 +216,11 @@ struct page {
#endif
} _struct_page_alignment;
+#if CONFIG_BASE_SMALL
+#define PAGE_FRAG_CACHE_MAX_SIZE PAGE_SIZE
+#else
#define PAGE_FRAG_CACHE_MAX_SIZE __ALIGN_MASK(32768, ~PAGE_MASK)
+#endif
#define PAGE_FRAG_CACHE_MAX_ORDER get_order(PAGE_FRAG_CACHE_MAX_SIZE)
#define PFC_MEMALLOC (1U << 31)
--
2.16.2
^ permalink raw reply related
* [PATCH v2 4/8] page_frag_cache: Rename fragsz to size
From: Matthew Wilcox @ 2018-03-22 15:31 UTC (permalink / raw)
To: Alexander Duyck
Cc: Matthew Wilcox, netdev, linux-mm, Jesper Dangaard Brouer,
Eric Dumazet
In-Reply-To: <20180322153157.10447-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
The 'size' variable name used to be used for the page size.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
mm/page_alloc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c9fc76135dd8..5a2e3e293079 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4403,7 +4403,7 @@ void __page_frag_cache_drain(struct page *page, unsigned int count)
EXPORT_SYMBOL(__page_frag_cache_drain);
void *page_frag_alloc(struct page_frag_cache *pfc,
- unsigned int fragsz, gfp_t gfp_mask)
+ unsigned int size, gfp_t gfp_mask)
{
struct page *page;
int offset;
@@ -4415,7 +4415,7 @@ void *page_frag_alloc(struct page_frag_cache *pfc,
return NULL;
}
- offset = pfc->offset - fragsz;
+ offset = pfc->offset - size;
if (unlikely(offset < 0))
goto refill;
--
2.16.2
^ permalink raw reply related
* [PATCH v2 3/8] page_frag_cache: Rename 'nc' to 'pfc'
From: Matthew Wilcox @ 2018-03-22 15:31 UTC (permalink / raw)
To: Alexander Duyck
Cc: Matthew Wilcox, netdev, linux-mm, Jesper Dangaard Brouer,
Eric Dumazet
In-Reply-To: <20180322153157.10447-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
This name was a legacy from the 'netdev_alloc_cache' days.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
mm/page_alloc.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6d2c106f4e5d..c9fc76135dd8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4336,21 +4336,21 @@ EXPORT_SYMBOL(free_pages);
* drivers to provide a backing region of memory for use as either an
* sk_buff->head, or to be used in the "frags" portion of skb_shared_info.
*/
-static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
+static struct page *__page_frag_cache_refill(struct page_frag_cache *pfc,
gfp_t gfp_mask)
{
unsigned int size = PAGE_SIZE;
struct page *page = NULL;
- struct page *old = nc->va ? virt_to_page(nc->va) : NULL;
+ struct page *old = pfc->va ? virt_to_page(pfc->va) : NULL;
gfp_t gfp = gfp_mask;
- unsigned int pagecnt_bias = nc->pagecnt_bias & ~PFC_MEMALLOC;
+ unsigned int pagecnt_bias = pfc->pagecnt_bias & ~PFC_MEMALLOC;
/* If all allocations have been freed, we can reuse this page */
if (old && page_ref_sub_and_test(old, pagecnt_bias)) {
page = old;
#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
/* if size can vary use size else just use PAGE_SIZE */
- size = nc->size;
+ size = pfc->size;
#endif
/* Page count is 0, we can safely set it */
set_page_count(page, size);
@@ -4364,25 +4364,25 @@ static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
PAGE_FRAG_CACHE_MAX_ORDER);
if (page)
size = PAGE_FRAG_CACHE_MAX_SIZE;
- nc->size = size;
+ pfc->size = size;
#endif
if (unlikely(!page))
page = alloc_pages_node(NUMA_NO_NODE, gfp, 0);
if (!page) {
- nc->va = NULL;
+ pfc->va = NULL;
return NULL;
}
- nc->va = page_address(page);
+ pfc->va = page_address(page);
/* Using atomic_set() would break get_page_unless_zero() users. */
page_ref_add(page, size - 1);
reset:
/* reset page count bias and offset to start of new frag */
- nc->pagecnt_bias = size;
+ pfc->pagecnt_bias = size;
if (page_is_pfmemalloc(page))
- nc->pagecnt_bias |= PFC_MEMALLOC;
- nc->offset = size;
+ pfc->pagecnt_bias |= PFC_MEMALLOC;
+ pfc->offset = size;
return page;
}
@@ -4402,27 +4402,27 @@ void __page_frag_cache_drain(struct page *page, unsigned int count)
}
EXPORT_SYMBOL(__page_frag_cache_drain);
-void *page_frag_alloc(struct page_frag_cache *nc,
+void *page_frag_alloc(struct page_frag_cache *pfc,
unsigned int fragsz, gfp_t gfp_mask)
{
struct page *page;
int offset;
- if (unlikely(!nc->va)) {
+ if (unlikely(!pfc->va)) {
refill:
- page = __page_frag_cache_refill(nc, gfp_mask);
+ page = __page_frag_cache_refill(pfc, gfp_mask);
if (!page)
return NULL;
}
- offset = nc->offset - fragsz;
+ offset = pfc->offset - fragsz;
if (unlikely(offset < 0))
goto refill;
- nc->pagecnt_bias--;
- nc->offset = offset;
+ pfc->pagecnt_bias--;
+ pfc->offset = offset;
- return nc->va + offset;
+ return pfc->va + offset;
}
EXPORT_SYMBOL(page_frag_alloc);
--
2.16.2
^ permalink raw reply related
* [PATCH v2 2/8] page_frag_cache: Move slowpath code from page_frag_alloc
From: Matthew Wilcox @ 2018-03-22 15:31 UTC (permalink / raw)
To: Alexander Duyck
Cc: Matthew Wilcox, netdev, linux-mm, Jesper Dangaard Brouer,
Eric Dumazet
In-Reply-To: <20180322153157.10447-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Put all the unlikely code in __page_frag_cache_refill to make the
fastpath code more obvious.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
mm/page_alloc.c | 70 ++++++++++++++++++++++++++++-----------------------------
1 file changed, 34 insertions(+), 36 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 61366f23e8c8..6d2c106f4e5d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4339,20 +4339,50 @@ EXPORT_SYMBOL(free_pages);
static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
gfp_t gfp_mask)
{
+ unsigned int size = PAGE_SIZE;
struct page *page = NULL;
+ struct page *old = nc->va ? virt_to_page(nc->va) : NULL;
gfp_t gfp = gfp_mask;
+ unsigned int pagecnt_bias = nc->pagecnt_bias & ~PFC_MEMALLOC;
+
+ /* If all allocations have been freed, we can reuse this page */
+ if (old && page_ref_sub_and_test(old, pagecnt_bias)) {
+ page = old;
+#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
+ /* if size can vary use size else just use PAGE_SIZE */
+ size = nc->size;
+#endif
+ /* Page count is 0, we can safely set it */
+ set_page_count(page, size);
+ goto reset;
+ }
#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
gfp_mask |= __GFP_COMP | __GFP_NOWARN | __GFP_NORETRY |
__GFP_NOMEMALLOC;
page = alloc_pages_node(NUMA_NO_NODE, gfp_mask,
PAGE_FRAG_CACHE_MAX_ORDER);
- nc->size = page ? PAGE_FRAG_CACHE_MAX_SIZE : PAGE_SIZE;
+ if (page)
+ size = PAGE_FRAG_CACHE_MAX_SIZE;
+ nc->size = size;
#endif
if (unlikely(!page))
page = alloc_pages_node(NUMA_NO_NODE, gfp, 0);
+ if (!page) {
+ nc->va = NULL;
+ return NULL;
+ }
+
+ nc->va = page_address(page);
- nc->va = page ? page_address(page) : NULL;
+ /* Using atomic_set() would break get_page_unless_zero() users. */
+ page_ref_add(page, size - 1);
+reset:
+ /* reset page count bias and offset to start of new frag */
+ nc->pagecnt_bias = size;
+ if (page_is_pfmemalloc(page))
+ nc->pagecnt_bias |= PFC_MEMALLOC;
+ nc->offset = size;
return page;
}
@@ -4375,7 +4405,6 @@ EXPORT_SYMBOL(__page_frag_cache_drain);
void *page_frag_alloc(struct page_frag_cache *nc,
unsigned int fragsz, gfp_t gfp_mask)
{
- unsigned int size = PAGE_SIZE;
struct page *page;
int offset;
@@ -4384,42 +4413,11 @@ void *page_frag_alloc(struct page_frag_cache *nc,
page = __page_frag_cache_refill(nc, gfp_mask);
if (!page)
return NULL;
-
-#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
- /* if size can vary use size else just use PAGE_SIZE */
- size = nc->size;
-#endif
- /* Even if we own the page, we do not use atomic_set().
- * This would break get_page_unless_zero() users.
- */
- page_ref_add(page, size - 1);
-
- /* reset page count bias and offset to start of new frag */
- nc->pagecnt_bias = size;
- if (page_is_pfmemalloc(page))
- nc->pagecnt_bias |= PFC_MEMALLOC;
- nc->offset = size;
}
offset = nc->offset - fragsz;
- if (unlikely(offset < 0)) {
- unsigned int pagecnt_bias = nc->pagecnt_bias & ~PFC_MEMALLOC;
- page = virt_to_page(nc->va);
-
- if (!page_ref_sub_and_test(page, pagecnt_bias))
- goto refill;
-
-#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
- /* if size can vary use size else just use PAGE_SIZE */
- size = nc->size;
-#endif
- /* OK, page count is 0, we can safely set it */
- set_page_count(page, size);
-
- /* reset page count bias and offset to start of new frag */
- nc->pagecnt_bias = size | (nc->pagecnt_bias - pagecnt_bias);
- offset = size - fragsz;
- }
+ if (unlikely(offset < 0))
+ goto refill;
nc->pagecnt_bias--;
nc->offset = offset;
--
2.16.2
^ permalink raw reply related
* [PATCH v2 1/8] page_frag_cache: Remove pfmemalloc bool
From: Matthew Wilcox @ 2018-03-22 15:31 UTC (permalink / raw)
To: Alexander Duyck
Cc: Matthew Wilcox, netdev, linux-mm, Jesper Dangaard Brouer,
Eric Dumazet
In-Reply-To: <20180322153157.10447-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Save 4/8 bytes by moving the pfmemalloc indicator from its own bool
to the top bit of pagecnt_bias. This has no effect on the fastpath
of the allocator since the pagecnt_bias cannot go negative. It's
a couple of extra instructions in the slowpath.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
include/linux/mm_types.h | 4 +++-
mm/page_alloc.c | 8 +++++---
net/core/skbuff.c | 5 ++---
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index fd1af6b9591d..a63b138ad1a4 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -218,6 +218,7 @@ struct page {
#define PAGE_FRAG_CACHE_MAX_SIZE __ALIGN_MASK(32768, ~PAGE_MASK)
#define PAGE_FRAG_CACHE_MAX_ORDER get_order(PAGE_FRAG_CACHE_MAX_SIZE)
+#define PFC_MEMALLOC (1U << 31)
struct page_frag_cache {
void * va;
@@ -231,9 +232,10 @@ struct page_frag_cache {
* containing page->_refcount every time we allocate a fragment.
*/
unsigned int pagecnt_bias;
- bool pfmemalloc;
};
+#define page_frag_cache_pfmemalloc(pfc) ((pfc)->pagecnt_bias & PFC_MEMALLOC)
+
typedef unsigned long vm_flags_t;
/*
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 635d7dd29d7f..61366f23e8c8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4395,16 +4395,18 @@ void *page_frag_alloc(struct page_frag_cache *nc,
page_ref_add(page, size - 1);
/* reset page count bias and offset to start of new frag */
- nc->pfmemalloc = page_is_pfmemalloc(page);
nc->pagecnt_bias = size;
+ if (page_is_pfmemalloc(page))
+ nc->pagecnt_bias |= PFC_MEMALLOC;
nc->offset = size;
}
offset = nc->offset - fragsz;
if (unlikely(offset < 0)) {
+ unsigned int pagecnt_bias = nc->pagecnt_bias & ~PFC_MEMALLOC;
page = virt_to_page(nc->va);
- if (!page_ref_sub_and_test(page, nc->pagecnt_bias))
+ if (!page_ref_sub_and_test(page, pagecnt_bias))
goto refill;
#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
@@ -4415,7 +4417,7 @@ void *page_frag_alloc(struct page_frag_cache *nc,
set_page_count(page, size);
/* reset page count bias and offset to start of new frag */
- nc->pagecnt_bias = size;
+ nc->pagecnt_bias = size | (nc->pagecnt_bias - pagecnt_bias);
offset = size - fragsz;
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 0bb0d8877954..54bbde8f7541 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -412,7 +412,7 @@ struct sk_buff *__netdev_alloc_skb(struct net_device *dev, unsigned int len,
nc = this_cpu_ptr(&netdev_alloc_cache);
data = page_frag_alloc(nc, len, gfp_mask);
- pfmemalloc = nc->pfmemalloc;
+ pfmemalloc = page_frag_cache_pfmemalloc(nc);
local_irq_restore(flags);
@@ -485,8 +485,7 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
return NULL;
}
- /* use OR instead of assignment to avoid clearing of bits in mask */
- if (nc->page.pfmemalloc)
+ if (page_frag_cache_pfmemalloc(&nc->page))
skb->pfmemalloc = 1;
skb->head_frag = 1;
--
2.16.2
^ permalink raw reply related
* [PATCH v2 0/8] page_frag_cache improvements
From: Matthew Wilcox @ 2018-03-22 15:31 UTC (permalink / raw)
To: Alexander Duyck
Cc: Matthew Wilcox, netdev, linux-mm, Jesper Dangaard Brouer,
Eric Dumazet
From: Matthew Wilcox <mawilcox@microsoft.com>
Version 1 was completely wrong-headed and I have repented of the error
of my ways. Thanks for educating me.
I still think it's possible to improve on the current state of the
page_frag allocator, and here are eight patches, each of which I think
represents an improvement. They're not all that interlinked, although
there will be textual conflicts, so I'll be happy to revise and drop
any that are not actual improvements.
I have discovered (today), much to my chagrin, that testing using trinity
in KVM doesn't actually test the page_frag allocator. I don't understand
why not. So, this turns out to only be compile tested. Sorry.
The net effect of all these patches is a reduction of four instructions
in the fastpath of the allocator on x86. The page_frag_cache structure
also shrinks, to as small as 8 bytes on 32-bit with CONFIG_BASE_SMALL.
The last patch is probably wrong. It'll definitely be inaccurate
because the call to page_frag_free() may not be the call which frees
a page; there's a really unlikely race where the page cache finds a
stale RCU pointer, bumps its refcount, discovers it's not the page it
was looking for and calls put_page(), which might end up being the last
reference count. We can do something about that inaccuracy, but I don't
even know if this is the best approach to accounting these pages.
Matthew Wilcox (8):
page_frag_cache: Remove pfmemalloc bool
page_frag_cache: Move slowpath code from page_frag_alloc
page_frag_cache: Rename 'nc' to 'pfc'
page_frag_cache: Rename fragsz to size
page_frag_cache: Save memory on small machines
page_frag_cache: Use a mask instead of offset
page_frag: Update documentation
page_frag: Account allocations
Documentation/vm/page_frags | 42 -----------
Documentation/vm/page_frags.rst | 24 +++++++
include/linux/mm_types.h | 20 ++++--
include/linux/mmzone.h | 3 +-
mm/page_alloc.c | 155 ++++++++++++++++++++++++----------------
net/core/skbuff.c | 5 +-
6 files changed, 135 insertions(+), 114 deletions(-)
delete mode 100644 Documentation/vm/page_frags
create mode 100644 Documentation/vm/page_frags.rst
--
2.16.2
^ permalink raw reply
* Re: [PATCH 0/3] pull request for net-next: batman-adv 2018-03-19
From: David Miller @ 2018-03-22 15:29 UTC (permalink / raw)
To: sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r
In-Reply-To: <20180319164153.11536-1-sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
From: Simon Wunderlich <sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
Date: Mon, 19 Mar 2018 17:41:50 +0100
> here is another late feature/cleanup pull request of batman-adv to go into net-next.
>
> Please pull or let me know of any problem!
Also pulled, thanks Simon.
^ permalink raw reply
* Re: [PATCH 0/5] pull request for net: batman-adv 2018-03-19
From: David Miller @ 2018-03-22 15:26 UTC (permalink / raw)
To: sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r
In-Reply-To: <20180319163726.10921-1-sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
From: Simon Wunderlich <sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
Date: Mon, 19 Mar 2018 17:37:21 +0100
> here are some more bugfixes for net.
>
> Please pull or let me know of any problem!
Pulled, thanks Simon.
^ permalink raw reply
* Re: [PATCH net-next] rds: tcp: remove register_netdevice_notifier infrastructure.
From: David Miller @ 2018-03-22 15:21 UTC (permalink / raw)
To: sowmini.varadhan; +Cc: netdev, santosh.shilimkar, ktkhai
In-Reply-To: <1521467568-37876-1-git-send-email-sowmini.varadhan@oracle.com>
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Date: Mon, 19 Mar 2018 06:52:48 -0700
> The netns deletion path does not need to wait for all net_devices
> to be unregistered before dismantling rds_tcp state for the netns
> (we are able to dismantle this state on module unload even when
> all net_devices are active so there is no dependency here).
>
> This patch removes code related to netdevice notifiers and
> refactors all the code needed to dismantle rds_tcp state
> into a ->exit callback for the pernet_operations used with
> register_pernet_device().
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 2/2 v4] netns: send uevent messages
From: David Miller @ 2018-03-22 15:18 UTC (permalink / raw)
To: christian.brauner
Cc: ebiederm, gregkh, netdev, linux-kernel, serge, avagin, ktkhai
In-Reply-To: <20180319121731.14449-2-christian.brauner@ubuntu.com>
From: Christian Brauner <christian.brauner@ubuntu.com>
Date: Mon, 19 Mar 2018 13:17:31 +0100
> This patch adds a receive method to NETLINK_KOBJECT_UEVENT netlink sockets
> to allow sending uevent messages into the network namespace the socket
> belongs to.
>
> Currently non-initial network namespaces are already isolated and don't
> receive uevents. There are a number of cases where it is beneficial for a
> sufficiently privileged userspace process to send a uevent into a network
> namespace.
>
> One such use case would be debugging and fuzzing of a piece of software
> which listens and reacts to uevents. By running a copy of that software
> inside a network namespace, specific uevents could then be presented to it.
> More concretely, this would allow for easy testing of udevd/ueventd.
>
> This will also allow some piece of software to run components inside a
> separate network namespace and then effectively filter what that software
> can receive. Some examples of software that do directly listen to uevents
> and that we have in the past attempted to run inside a network namespace
> are rbd (CEPH client) or the X server.
...
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 1/2 v4] net: add uevent socket member
From: David Miller @ 2018-03-22 15:18 UTC (permalink / raw)
To: christian.brauner
Cc: ebiederm, gregkh, netdev, linux-kernel, serge, avagin, ktkhai
In-Reply-To: <20180319121731.14449-1-christian.brauner@ubuntu.com>
From: Christian Brauner <christian.brauner@ubuntu.com>
Date: Mon, 19 Mar 2018 13:17:30 +0100
> This commit adds struct uevent_sock to struct net. Since struct uevent_sock
> records the position of the uevent socket in the uevent socket list we can
> trivially remove it from the uevent socket list during cleanup. This speeds
> up the old removal codepath.
> Note, list_del() will hit __list_del_entry_valid() in its call chain which
> will validate that the element is a member of the list. If it isn't it will
> take care that the list is not modified.
>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: Convert nf_ct_net_ops
From: David Miller @ 2018-03-22 15:14 UTC (permalink / raw)
To: ktkhai
Cc: alex.aring, stefan, pablo, kadlec, fw, yoshfuji, brouer, keescook,
linux-wpan, netdev, netfilter-devel
In-Reply-To: <152145994646.26348.3106975491506950180.stgit@localhost.localdomain>
From: Kirill Tkhai <ktkhai@virtuozzo.com>
Date: Mon, 19 Mar 2018 14:45:46 +0300
> These pernet_operations register and unregister sysctl.
> Also, there is inet_frags_exit_net() called in exit method,
> which has to be safe after a560002437d3 "net: Fix hlist
> corruptions in inet_evict_bucket()".
>
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: Convert lowpan_frags_ops
From: David Miller @ 2018-03-22 15:14 UTC (permalink / raw)
To: ktkhai
Cc: alex.aring, stefan, pablo, kadlec, fw, yoshfuji, brouer, keescook,
linux-wpan, netdev, netfilter-devel
In-Reply-To: <152145993742.26348.7771294840455450666.stgit@localhost.localdomain>
From: Kirill Tkhai <ktkhai@virtuozzo.com>
Date: Mon, 19 Mar 2018 14:45:37 +0300
> These pernet_operations register and unregister sysctl.
> Also, there is inet_frags_exit_net() called in exit method,
> which has to be safe after a560002437d3 "net: Fix hlist
> corruptions in inet_evict_bucket()".
>
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: Convert can_pernet_ops
From: David Miller @ 2018-03-22 15:14 UTC (permalink / raw)
To: ktkhai; +Cc: socketcan, mkl, linux-can, netdev
In-Reply-To: <152145954531.26024.3744883546191004693.stgit@localhost.localdomain>
From: Kirill Tkhai <ktkhai@virtuozzo.com>
Date: Mon, 19 Mar 2018 14:39:05 +0300
> These pernet_operations create and destroy /proc entries
> and cancel per-net timer.
>
> Also, there are unneed iterations over empty list of net
> devices, since all net devices must be already moved
> to init_net or unregistered by default_device_ops. This
> already was mentioned here:
>
> https://marc.info/?l=linux-can&m=150169589119335&w=2
>
> So, it looks safe to make them async.
>
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
Applied.
^ permalink raw reply
* [PATCH v3 2/2] i40e: add support for XDP_REDIRECT
From: Björn Töpel @ 2018-03-22 15:14 UTC (permalink / raw)
To: jeffrey.t.kirsher, intel-wired-lan
Cc: Björn Töpel, magnus.karlsson, netdev, alexander.h.duyck,
alexander.duyck
In-Reply-To: <20180322151434.24338-1-bjorn.topel@gmail.com>
From: Björn Töpel <bjorn.topel@intel.com>
The driver now acts upon the XDP_REDIRECT return action. Two new ndos
are implemented, ndo_xdp_xmit and ndo_xdp_flush.
XDP_REDIRECT action enables XDP program to redirect frames to other
netdevs.
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 2 +
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 74 +++++++++++++++++++++++++----
drivers/net/ethernet/intel/i40e/i40e_txrx.h | 2 +
3 files changed, 68 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 79ab52276d12..2fb4261b4fd9 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11815,6 +11815,8 @@ static const struct net_device_ops i40e_netdev_ops = {
.ndo_bridge_getlink = i40e_ndo_bridge_getlink,
.ndo_bridge_setlink = i40e_ndo_bridge_setlink,
.ndo_bpf = i40e_xdp,
+ .ndo_xdp_xmit = i40e_xdp_xmit,
+ .ndo_xdp_flush = i40e_xdp_flush,
};
/**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 2f817d1466eb..53dc74d4d1db 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2214,7 +2214,7 @@ static int i40e_xmit_xdp_ring(struct xdp_buff *xdp,
static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring,
struct xdp_buff *xdp)
{
- int result = I40E_XDP_PASS;
+ int err, result = I40E_XDP_PASS;
struct i40e_ring *xdp_ring;
struct bpf_prog *xdp_prog;
u32 act;
@@ -2233,6 +2233,10 @@ static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring,
xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
result = i40e_xmit_xdp_ring(xdp, xdp_ring);
break;
+ case XDP_REDIRECT:
+ err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog);
+ result = !err ? I40E_XDP_TX : I40E_XDP_CONSUMED;
+ break;
default:
bpf_warn_invalid_xdp_action(act);
case XDP_ABORTED:
@@ -2268,6 +2272,15 @@ static void i40e_rx_buffer_flip(struct i40e_ring *rx_ring,
#endif
}
+static inline void i40e_xdp_ring_update_tail(struct i40e_ring *xdp_ring)
+{
+ /* Force memory writes to complete before letting h/w
+ * know there are new descriptors to fetch.
+ */
+ wmb();
+ writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail);
+}
+
/**
* i40e_clean_rx_irq - Clean completed descriptors from Rx ring - bounce buf
* @rx_ring: rx descriptor ring to transact packets on
@@ -2402,16 +2415,11 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
}
if (xdp_xmit) {
- struct i40e_ring *xdp_ring;
-
- xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
+ struct i40e_ring *xdp_ring =
+ rx_ring->vsi->xdp_rings[rx_ring->queue_index];
- /* Force memory writes to complete before letting h/w
- * know there are new descriptors to fetch.
- */
- wmb();
-
- writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail);
+ i40e_xdp_ring_update_tail(xdp_ring);
+ xdp_do_flush_map();
}
rx_ring->skb = skb;
@@ -3659,3 +3667,49 @@ netdev_tx_t i40e_lan_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
return i40e_xmit_frame_ring(skb, tx_ring);
}
+
+/**
+ * i40e_xdp_xmit - Implements ndo_xdp_xmit
+ * @dev: netdev
+ * @xdp: XDP buffer
+ *
+ * Returns Zero if sent, else an error code
+ **/
+int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp)
+{
+ struct i40e_netdev_priv *np = netdev_priv(dev);
+ unsigned int queue_index = smp_processor_id();
+ struct i40e_vsi *vsi = np->vsi;
+ int err;
+
+ if (test_bit(__I40E_VSI_DOWN, vsi->state))
+ return -ENETDOWN;
+
+ if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs)
+ return -ENXIO;
+
+ err = i40e_xmit_xdp_ring(xdp, vsi->xdp_rings[queue_index]);
+ if (err != I40E_XDP_TX)
+ return -ENOSPC;
+
+ return 0;
+}
+
+/**
+ * i40e_xdp_flush - Implements ndo_xdp_flush
+ * @dev: netdev
+ **/
+void i40e_xdp_flush(struct net_device *dev)
+{
+ struct i40e_netdev_priv *np = netdev_priv(dev);
+ unsigned int queue_index = smp_processor_id();
+ struct i40e_vsi *vsi = np->vsi;
+
+ if (test_bit(__I40E_VSI_DOWN, vsi->state))
+ return;
+
+ if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs)
+ return;
+
+ i40e_xdp_ring_update_tail(vsi->xdp_rings[queue_index]);
+}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index 2444f338bb0c..a97e59721393 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -510,6 +510,8 @@ u32 i40e_get_tx_pending(struct i40e_ring *ring, bool in_sw);
void i40e_detect_recover_hung(struct i40e_vsi *vsi);
int __i40e_maybe_stop_tx(struct i40e_ring *tx_ring, int size);
bool __i40e_chk_linearize(struct sk_buff *skb);
+int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp);
+void i40e_xdp_flush(struct net_device *dev);
/**
* i40e_get_head - Retrieve head from head writeback
--
2.7.4
^ permalink raw reply related
* [PATCH v3 1/2] i40e: tweak page counting for XDP_REDIRECT
From: Björn Töpel @ 2018-03-22 15:14 UTC (permalink / raw)
To: jeffrey.t.kirsher, intel-wired-lan
Cc: Björn Töpel, magnus.karlsson, netdev, alexander.h.duyck,
alexander.duyck
From: Björn Töpel <bjorn.topel@intel.com>
This commit tweaks the page counting for XDP_REDIRECT to function
properly. XDP_REDIRECT support will be added in a future commit.
The current page counting scheme assumes that the reference count
cannot decrease until the received frame is sent to the upper layers
of the networking stack. This assumption does not hold for the
XDP_REDIRECT action, since a page (pointed out by xdp_buff) can have
its reference count decreased via the xdp_do_redirect call.
To work around that, we now start off by a large page count and then
don't allow a refcount less than two.
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index e8eef9a56b6b..2f817d1466eb 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1588,9 +1588,8 @@ static bool i40e_alloc_mapped_page(struct i40e_ring *rx_ring,
bi->dma = dma;
bi->page = page;
bi->page_offset = i40e_rx_offset(rx_ring);
-
- /* initialize pagecnt_bias to 1 representing we fully own page */
- bi->pagecnt_bias = 1;
+ page_ref_add(page, USHRT_MAX - 1);
+ bi->pagecnt_bias = USHRT_MAX;
return true;
}
@@ -1956,8 +1955,8 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer)
* the pagecnt_bias and page count so that we fully restock the
* number of references the driver holds.
*/
- if (unlikely(!pagecnt_bias)) {
- page_ref_add(page, USHRT_MAX);
+ if (unlikely(pagecnt_bias == 1)) {
+ page_ref_add(page, USHRT_MAX - 1);
rx_buffer->pagecnt_bias = USHRT_MAX;
}
--
2.7.4
^ permalink raw reply related
* RE: [PATCH v5 0/2] Remove false-positive VLAs when using max()
From: David Laight @ 2018-03-22 15:13 UTC (permalink / raw)
To: 'Kees Cook', Linus Torvalds
Cc: Al Viro, Florian Weimer, Andrew Morton, Josh Poimboeuf,
Rasmus Villemoes, Randy Dunlap, Miguel Ojeda, Ingo Molnar,
Ian Abbott, linux-input, linux-btrfs, Network Development,
Linux Kernel Mailing List, Kernel Hardening
In-Reply-To: <CAGXu5j+5i+56R0KDLMDA=+_DRW5w9aUGCEo0dq6PZvHPBWkM1g@mail.gmail.com>
From: Kees Cook
> Sent: 22 March 2018 15:01
...
> > /* Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de> */
> > #define __is_constant(a) \
> > (sizeof(int) == sizeof(*(1 ? ((void*)((a) * 0l)) : (int*)1)))
...
> So, this time it's not a catastrophic failure with gcc 4.4. Instead it
> fails in 11 distinct places:
...
> Seems like it doesn't like void * arguments:
>
> mm/percpu.c:
> void *ptr;
> ...
> base = min(ptr, base);
Try adding (unsigned long) before the (a).
David
^ permalink raw reply
* Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()
From: Kees Cook @ 2018-03-22 15:01 UTC (permalink / raw)
To: Linus Torvalds
Cc: Al Viro, Florian Weimer, Andrew Morton, Josh Poimboeuf,
Rasmus Villemoes, Randy Dunlap, Miguel Ojeda, Ingo Molnar,
David Laight, Ian Abbott, linux-input, linux-btrfs,
Network Development, Linux Kernel Mailing List, Kernel Hardening
In-Reply-To: <CA+55aFwxk=tUECYQkd4cog08qW4ZT=r2K7FQXzGnc-zuMc7JQA@mail.gmail.com>
On Tue, Mar 20, 2018 at 4:23 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Sat, Mar 17, 2018 at 1:07 PM, Kees Cook <keescook@chromium.org> wrote:
>>
>> No luck! :( gcc 4.4 refuses to play along. And, hilariously, not only
>> does it not change the complaint about __builtin_choose_expr(), it
>> also thinks that's a VLA now.
>
> Hmm. So thanks to the diseased mind of Martin Uecker, there's a better
> test for "__is_constant()":
>
> /* Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de> */
> #define __is_constant(a) \
> (sizeof(int) == sizeof(*(1 ? ((void*)((a) * 0l)) : (int*)1)))
>
> that is actually *specified* by the C standard to work, and doesn't
> even depend on any gcc extensions.
I feel we risk awakening Cthulhu with this. :)
> The reason is some really subtle pointer conversion rules, where the
> type of the ternary operator will depend on whether one of the
> pointers is NULL or not.
>
> And the definition of NULL, in turn, very much depends on "integer
> constant expression that has the value 0".
>
> Are you willing to do one final try on a generic min/max? Same as my
> last patch, but using the above __is_constant() test instead of
> __builtin_constant_p?
So, this time it's not a catastrophic failure with gcc 4.4. Instead it
fails in 11 distinct places:
$ grep "first argument to ‘__builtin_choose_expr’ not a constant" log
| cut -d: -f1-2
crypto/ablkcipher.c:71
crypto/blkcipher.c:70
crypto/skcipher.c:95
mm/percpu.c:2453
net/ceph/osdmap.c:1545
net/ceph/osdmap.c:1756
net/ceph/osdmap.c:1763
mm/kmemleak.c:1371
mm/kmemleak.c:1403
drivers/infiniband/hw/hfi1/pio_copy.c:421
drivers/infiniband/hw/hfi1/pio_copy.c:547
Seems like it doesn't like void * arguments:
mm/percpu.c:
void *ptr;
...
base = min(ptr, base);
mm/kmemleak.c:
static void scan_large_block(void *start, void *end)
...
next = min(start + MAX_SCAN_SIZE, end);
I'll poke a bit more...
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [patch net-next RFC 00/12] devlink: introduce port flavours and common phys_port_name generation
From: Jiri Pirko @ 2018-03-22 14:58 UTC (permalink / raw)
To: Roopa Prabhu
Cc: netdev, David Miller, Ido Schimmel, Jakub Kicinski, mlxsw,
Andrew Lunn, Vivien Didelot, Florian Fainelli, Michael Chan,
ganeshgr, Saeed Mahameed, Simon Horman, pieter.jansenvanvuuren,
John Hurley, Dirk van der Merwe, Alexander Duyck, Or Gerlitz,
David Ahern, vijaya.guvva, satananda.burla, raghu.vatsavayi,
felix.manlunas, Andy Gospodarek, sathya.perla, vasundhara-v.volam,
tariqt, eranbe, Jeff Kirsher
In-Reply-To: <CAJieiUhiOmxjuGZ=P48j7eBz57oapS53=dvE7ZxvioxKENxbcA@mail.gmail.com>
Thu, Mar 22, 2018 at 03:40:02PM CET, roopa@cumulusnetworks.com wrote:
>On Thu, Mar 22, 2018 at 3:55 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> This patchset resolves 2 issues we have right now:
>> 1) There are many netdevices / ports in the system, for port, pf, vf
>> represenatation but the user has no way to see which is which
>> 2) The ndo_get_phys_port_name is implemented in each driver separatelly,
>> which may lead to inconsistent names between drivers.
>>
>> This patchset introduces port flavours which should address the first
>> problem. I'm testing this with Netronome nfp hardware. When the user
>> has 2 physical ports, 1 pf, and 4 vfs, he should see something like this:
>> # devlink port
>> pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0
>> pci/0000:05:00.0/268435456: type eth netdev eth0 flavour physical number 0
>> pci/0000:05:00.0/268435460: type eth netdev enp5s0np1 flavour physical number 1
>> pci/0000:05:00.0/536875008: type eth netdev eth2 flavour pf_rep number 536875008
>> pci/0000:05:00.0/536870912: type eth netdev eth1 flavour vf_rep number 0
>> pci/0000:05:00.0/536870976: type eth netdev eth3 flavour vf_rep number 1
>> pci/0000:05:00.0/536871040: type eth netdev eth4 flavour vf_rep number 2
>> pci/0000:05:00.0/536871104: type eth netdev eth5 flavour vf_rep number 3
>>
>> The indexes are weird numbers now. That needs to be fixed. Also, netdev
>> renaming does not work correctly for me now for some reason.
>> Also, there is one extra port that I don't understand what
>> is the purpose for it - something nfp specific perhaps.
>>
>> The desired output should look like this:
>> # devlink port
>> pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0
>> pci/0000:05:00.0/1: type eth netdev enp5s0np1 flavour physical number 1
>> pci/0000:05:00.0/2: type eth netdev enp5s0npf0 flavour pf_rep number 0
>> pci/0000:05:00.0/3: type eth netdev enp5s0nvf0 flavour vf_rep number 0
>> pci/0000:05:00.0/4: type eth netdev enp5s0nvf1 flavour vf_rep number 1
>> pci/0000:05:00.0/5: type eth netdev enp5s0nvf2 flavour vf_rep number 2
>> pci/0000:05:00.0/6: type eth netdev enp5s0nvf3 flavour vf_rep number 3
>>
>> As you can see, the netdev names are generated according to the flavour
>> and port number. In case the port is split, the split subnumber is also
>> included.
>>
>> I tested this for mlxsw and nfp. I have no way to test this on DSA hw,
>> I would really appretiate DSA guys to test this. Thanks!
>>
>
>nice series, I like that the user can query a ports flavor (I get this
>ask all the time).
Yeah, it is really needed. I would like to fix this jungle so all
drivers behave the same. Started with nfp as they are leading with what
they have implemented. But I expect others to join in (please).
Many drivers just create devlink instance without any ports. Odd. I will
write some Documentation file as a part of this patchset. Also, I'm
thinking about adding some warnings in care driver does some crippled
implementation.
^ permalink raw reply
* Re: [PATCH v2 2/2] i40e: add support for XDP_REDIRECT
From: Alexander Duyck @ 2018-03-22 14:52 UTC (permalink / raw)
To: Björn Töpel
Cc: Jesper Dangaard Brouer, Jeff Kirsher, intel-wired-lan,
Björn Töpel, Karlsson, Magnus, Netdev,
Duyck, Alexander H
In-Reply-To: <CAJ+HfNiGWgJEE9K1rh3OrgixFaTJE4mDCBzYVGQJv3R8_rpLTQ@mail.gmail.com>
On Thu, Mar 22, 2018 at 5:20 AM, Björn Töpel <bjorn.topel@gmail.com> wrote:
> 2018-03-22 12:58 GMT+01:00 Jesper Dangaard Brouer <brouer@redhat.com>:
>>
>> On Thu, 22 Mar 2018 10:03:07 +0100 Björn Töpel <bjorn.topel@gmail.com> wrote:
>>
>>> +/**
>>> + * i40e_xdp_xmit - Implements ndo_xdp_xmit
>>> + * @dev: netdev
>>> + * @xdp: XDP buffer
>>> + *
>>> + * Returns Zero if sent, else an error code
>>> + **/
>>> +int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp)
>>> +{
>>
>> The return code is used by the XDP redirect tracepoint... this is the
>> only way we have to debug/troubleshoot runtime issues with XDP. Thus,
>> these need to be consistent across drives and distinguishable.
>>
>
> Thanks for pointing this out! I'll address all your comments and do a
> respin (but I'll wait for Alex' comments, if any).
>
>
> Björn
>
The patch mostly looks okay to me. Maybe a bit of reverse xmas tree
formatting needs to be addressed for the variable declarations in your
two new functions but that is about it in terms of what I see.
- Alex
^ permalink raw reply
* Re: [RFC PATCH 1/5] net: macb: Check MDIO state before read/write and use timeouts
From: Andrew Lunn @ 2018-03-22 14:47 UTC (permalink / raw)
To: harinikatakamlinux
Cc: nicolas.ferre, davem, netdev, linux-kernel, harinik, michals,
appanad, Shubhrajyoti Datta
In-Reply-To: <1521726700-22634-2-git-send-email-harinikatakamlinux@gmail.com>
On Thu, Mar 22, 2018 at 07:21:36PM +0530, harinikatakamlinux@gmail.com wrote:
> From: Harini Katakam <harinik@xilinx.com>
>
> Replace the while loop in MDIO read/write functions with a timeout.
> In addition, add a check for MDIO bus busy before initiating a new
> operation as well to make sure there is no ongoing MDIO operation.
>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> Signed-off-by: Harini Katakam <harinik@xilinx.com>
> ---
> drivers/net/ethernet/cadence/macb_main.c | 54 ++++++++++++++++++++++++++++++--
> 1 file changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index d09bd43..f4030c1 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -321,6 +321,21 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> {
> struct macb *bp = bus->priv;
> int value;
> + ulong timeout;
> +
> + timeout = jiffies + msecs_to_jiffies(1000);
> + /* wait for end of transfer */
> + do {
> + if (MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
> + break;
> +
> + cpu_relax();
> + } while (!time_after_eq(jiffies, timeout));
> +
> + if (time_after_eq(jiffies, timeout)) {
> + netdev_err(bp->dev, "wait for end of transfer timed out\n");
> + return -ETIMEDOUT;
> + }
Hi Harini
It looks like you have repeated the same code 4 times. Please move it
into a helper function.
Andrew
^ permalink raw reply
* [PATCH v2 net-next] virtio-net: Fix operstate for virtio when no VIRTIO_NET_F_STATUS
From: Jay Vosburgh @ 2018-03-22 14:42 UTC (permalink / raw)
To: netdev; +Cc: Michael S. Tsirkin, Jason Wang, David Miller, Ben Hutchings
The operstate update logic will leave an interface in the
default UNKNOWN operstate if the interface carrier state never changes
from the default carrier up state set at creation. This includes the
case of an explicit call to netif_carrier_on, as the carrier on to on
transition has no effect on operstate.
This affects virtio-net for the case that the virtio peer does
not support VIRTIO_NET_F_STATUS (the feature that provides carrier state
updates). Without this feature, the virtio specification states that
"the link should be assumed active," so, logically, the operstate should
be UP instead of UNKNOWN. This has impact on user space applications
that use the operstate to make availability decisions for the interface.
Resolve this by changing the virtio probe logic slightly to call
netif_carrier_off for both the "with" and "without" VIRTIO_NET_F_STATUS
cases, and then the existing call to netif_carrier_on for the "without"
case will cause an operstate transition.
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Jay Vosburgh <jay.vosburgh@canonical.com>
---
I considered resolving this by changing linkwatch_init_dev to
unconditionally call rfc2863_policy, as that would always set operstate
for all interfaces.
This would not have any impact on most cases (as most drivers
call netif_carrier_off during probe), except for the loopback device,
which currently has an operstate of UNKNOWN (because it never does any
carrier state transitions). This change would add a round trip on the
dev_base_lock for every loopback device creation, which could have a
negative impact when creating many loopback devices, e.g., when
concurrently creating large numbers of containers.
drivers/net/virtio_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 23374603e4d9..7b187ec7411e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2857,8 +2857,8 @@ static int virtnet_probe(struct virtio_device *vdev)
/* Assume link up if device can't report link status,
otherwise get link status from config. */
+ netif_carrier_off(dev);
if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
- netif_carrier_off(dev);
schedule_work(&vi->config_work);
} else {
vi->status = VIRTIO_NET_S_LINK_UP;
--
2.14.1
^ permalink raw reply related
* Re: [patch net-next RFC 00/12] devlink: introduce port flavours and common phys_port_name generation
From: Roopa Prabhu @ 2018-03-22 14:40 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Ido Schimmel, Jakub Kicinski, mlxsw,
Andrew Lunn, Vivien Didelot, Florian Fainelli, Michael Chan,
ganeshgr, Saeed Mahameed, Simon Horman, pieter.jansenvanvuuren,
John Hurley, Dirk van der Merwe, Alexander Duyck, Or Gerlitz,
David Ahern, vijaya.guvva, satananda.burla, raghu.vatsavayi,
felix.manlunas, Andy Gospodarek, sathya.perla, vasundhara-v.volam,
tariqt, eranbe, Jeff Kirsher
In-Reply-To: <20180322105522.8186-1-jiri@resnulli.us>
On Thu, Mar 22, 2018 at 3:55 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> This patchset resolves 2 issues we have right now:
> 1) There are many netdevices / ports in the system, for port, pf, vf
> represenatation but the user has no way to see which is which
> 2) The ndo_get_phys_port_name is implemented in each driver separatelly,
> which may lead to inconsistent names between drivers.
>
> This patchset introduces port flavours which should address the first
> problem. I'm testing this with Netronome nfp hardware. When the user
> has 2 physical ports, 1 pf, and 4 vfs, he should see something like this:
> # devlink port
> pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0
> pci/0000:05:00.0/268435456: type eth netdev eth0 flavour physical number 0
> pci/0000:05:00.0/268435460: type eth netdev enp5s0np1 flavour physical number 1
> pci/0000:05:00.0/536875008: type eth netdev eth2 flavour pf_rep number 536875008
> pci/0000:05:00.0/536870912: type eth netdev eth1 flavour vf_rep number 0
> pci/0000:05:00.0/536870976: type eth netdev eth3 flavour vf_rep number 1
> pci/0000:05:00.0/536871040: type eth netdev eth4 flavour vf_rep number 2
> pci/0000:05:00.0/536871104: type eth netdev eth5 flavour vf_rep number 3
>
> The indexes are weird numbers now. That needs to be fixed. Also, netdev
> renaming does not work correctly for me now for some reason.
> Also, there is one extra port that I don't understand what
> is the purpose for it - something nfp specific perhaps.
>
> The desired output should look like this:
> # devlink port
> pci/0000:05:00.0/0: type eth netdev enp5s0np0 flavour physical number 0
> pci/0000:05:00.0/1: type eth netdev enp5s0np1 flavour physical number 1
> pci/0000:05:00.0/2: type eth netdev enp5s0npf0 flavour pf_rep number 0
> pci/0000:05:00.0/3: type eth netdev enp5s0nvf0 flavour vf_rep number 0
> pci/0000:05:00.0/4: type eth netdev enp5s0nvf1 flavour vf_rep number 1
> pci/0000:05:00.0/5: type eth netdev enp5s0nvf2 flavour vf_rep number 2
> pci/0000:05:00.0/6: type eth netdev enp5s0nvf3 flavour vf_rep number 3
>
> As you can see, the netdev names are generated according to the flavour
> and port number. In case the port is split, the split subnumber is also
> included.
>
> I tested this for mlxsw and nfp. I have no way to test this on DSA hw,
> I would really appretiate DSA guys to test this. Thanks!
>
nice series, I like that the user can query a ports flavor (I get this
ask all the time).
thanks
^ permalink raw reply
* Re: [PATCH net] virtio-net: Fix operstate for virtio when no VIRTIO_NET_F_STATUS
From: Jay Vosburgh @ 2018-03-22 14:34 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, Jason Wang, David Miller, Ben Hutchings
In-Reply-To: <20180322160132-mutt-send-email-mst@kernel.org>
Michael S. Tsirkin <mst@redhat.com> wrote:
>On Thu, Mar 22, 2018 at 12:02:10PM +0000, Jay Vosburgh wrote:
>> Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> >On Thu, Mar 22, 2018 at 09:05:52AM +0000, Jay Vosburgh wrote:
>> >> The operstate update logic will leave an interface in the
>> >> default UNKNOWN operstate if the interface carrier state never changes
>> >> from the default carrier up state set at creation. This includes the
>> >> case of an explicit call to netif_carrier_on, as the carrier on to on
>> >> transition has no effect on operstate.
>> >>
>> >> This affects virtio-net for the case that the virtio peer does
>> >> not support VIRTIO_NET_F_STATUS (the feature that provides carrier state
>> >> updates). Without this feature, the virtio specification states that
>> >> "the link should be assumed active," so, logically, the operstate should
>> >> be UP instead of UNKNOWN. This has impact on user space applications
>> >> that use the operstate to make availability decisions for the interface.
>> >>
>> >> Resolve this by changing the virtio probe logic slightly to call
>> >> netif_carrier_off for both the "with" and "without" VIRTIO_NET_F_STATUS
>> >> cases, and then the existing call to netif_carrier_on for the "without"
>> >> case will cause an operstate transition.
>> >>
>> >> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> >> Cc: Jason Wang <jasowang@redhat.com>
>> >> Cc: Ben Hutchings <ben@decadent.org.uk>
>> >> Fixes: 167c25e4c550 ("virtio-net: init link state correctly")
>> >
>> >I'd say that's an abuse of this notation. openstate was UNKNOWN
>> >even before that fix.
>>
>> I went back to the commit that added the dependency on
>> VIRTIO_NET_F_STATUS (and that this patch would thus apply on top of).
>> If that's an issue, I can resubmit without it.
>>
>> -J
>
>The patch can be trivially backported to any version that has virtio.
>
>The issue was present since the original version of virtio.
>VIRTIO_NET_F_STATUS fixed it for new devices.
>So the tag is incorrectly blaming a partial fix for not being a full
>one.
>
>Also, I think it's more appropriate for net-next - it's a
>minor ABI change (previously presence of VIRTIO_NET_F_STATUS
>could be detected by looking at operstate, now it can't).
>Hopefully this makes more apps work than it breaks.
>
>So yes, pls repost without Fixes and with net-next unless
>davem can make the change himself.
Reposting with requested changes.
-J
>> >> Signed-off-by: Jay Vosburgh <jay.vosburgh@canonical.com>
>> >
>> >Acked-by: Michael S. Tsirkin <mst@redhat.com>
>> >
>> >
>> >> ---
>> >>
>> >> I considered resolving this by changing linkwatch_init_dev to
>> >> unconditionally call rfc2863_policy, as that would always set operstate
>> >> for all interfaces.
>> >>
>> >> This would not have any impact on most cases (as most drivers
>> >> call netif_carrier_off during probe), except for the loopback device,
>> >> which currently has an operstate of UNKNOWN (because it never does any
>> >> carrier state transitions). This change would add a round trip on the
>> >> dev_base_lock for every loopback device creation, which could have a
>> >> negative impact when creating many loopback devices, e.g., when
>> >> concurrently creating large numbers of containers.
>> >>
>> >>
>> >> drivers/net/virtio_net.c | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> >> index 23374603e4d9..7b187ec7411e 100644
>> >> --- a/drivers/net/virtio_net.c
>> >> +++ b/drivers/net/virtio_net.c
>> >> @@ -2857,8 +2857,8 @@ static int virtnet_probe(struct virtio_device *vdev)
>> >>
>> >> /* Assume link up if device can't report link status,
>> >> otherwise get link status from config. */
>> >> + netif_carrier_off(dev);
>> >> if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
>> >> - netif_carrier_off(dev);
>> >> schedule_work(&vi->config_work);
>> >> } else {
>> >> vi->status = VIRTIO_NET_S_LINK_UP;
>> >> --
>> >> 2.14.1
^ permalink raw reply
* Re: [PATCH net-next 1/1] net/ipv4: disable SMC TCP option with SYN Cookies
From: Eric Dumazet @ 2018-03-22 14:30 UTC (permalink / raw)
To: Ursula Braun, davem
Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl
In-Reply-To: <9b88f9f8-1d3b-7d7d-f612-b823069afa75@linux.vnet.ibm.com>
On 03/22/2018 06:23 AM, Ursula Braun wrote:
> We moved the clear to cookie_v4_check()/cookie_v6_check. However, this does not seem to
> be sufficient to prevent the SYNACK from containing the SMC experimental option.
> We found that an additional check in tcp_conn_request() helps:
>
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -6248,6 +6248,9 @@ int tcp_conn_request(struct request_sock
> if (want_cookie && !tmp_opt.saw_tstamp)
> tcp_clear_options(&tmp_opt);
>
> + if (IS_ENABLED(CONFIG_SMC) && want_cookie && tmp_opt.smc_ok)
> + tmp_opt.smc_ok = 0;
> +
> tmp_opt.tstamp_ok = tmp_opt.saw_tstamp;
> tcp_openreq_init(req, &tmp_opt, skb, sk);
> inet_rsk(req)->no_srccheck = inet_sk(sk)->transparent;
>
> Do you think this could be the right place for clearing the smc_ok bit?
Yes, but since tmp_opt is private to this thread/cpu, no false sharing to be afraid of
if (IS_ENABLED(CONFIG_SMC) && want_cookie)
tmp_opt.smc_ok = 0;
^ 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