* [PATCH 10/10] sunrpc: use SKB fragment destructors to delay completion until page is released by network stack.
From: Ian Campbell @ 2012-04-10 14:26 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: David Miller, Eric Dumazet, Michael S. Tsirkin, Wei Liu,
xen-devel-GuqFBffKawuEi8DpZVb4nw, Ian Campbell, Neil Brown,
J. Bruce Fields, linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1334067965.5394.22.camel-o4Be2W7LfRlXesXXhkcM7miJhflN2719@public.gmane.org>
This prevents an issue where an ACK is delayed, a retransmit is queued (either
at the RPC or TCP level) and the ACK arrives before the retransmission hits the
wire. If this happens to an NFS WRITE RPC then the write() system call
completes and the userspace process can continue, potentially modifying data
referenced by the retransmission before the retransmission occurs.
Signed-off-by: Ian Campbell <ian.campbell-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
Acked-by: Trond Myklebust <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: Neil Brown <neilb-l3A5Bk7waGM@public.gmane.org>
Cc: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
include/linux/sunrpc/xdr.h | 2 ++
include/linux/sunrpc/xprt.h | 5 ++++-
net/sunrpc/clnt.c | 27 ++++++++++++++++++++++-----
net/sunrpc/svcsock.c | 3 ++-
net/sunrpc/xprt.c | 12 ++++++++++++
net/sunrpc/xprtsock.c | 3 ++-
6 files changed, 44 insertions(+), 8 deletions(-)
diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index af70af3..ff1b121 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -16,6 +16,7 @@
#include <asm/byteorder.h>
#include <asm/unaligned.h>
#include <linux/scatterlist.h>
+#include <linux/skbuff.h>
/*
* Buffer adjustment
@@ -57,6 +58,7 @@ struct xdr_buf {
tail[1]; /* Appended after page data */
struct page ** pages; /* Array of contiguous pages */
+ struct skb_frag_destructor *destructor;
unsigned int page_base, /* Start of page data */
page_len, /* Length of page data */
flags; /* Flags for data disposition */
diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index 77d278d..e8d3f18 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -92,7 +92,10 @@ struct rpc_rqst {
/* A cookie used to track the
state of the transport
connection */
-
+ struct skb_frag_destructor destructor; /* SKB paged fragment
+ * destructor for
+ * transmitted pages*/
+
/*
* Partial send handling
*/
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 7a4cb5f..4e94e2a 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -62,6 +62,7 @@ static void call_reserve(struct rpc_task *task);
static void call_reserveresult(struct rpc_task *task);
static void call_allocate(struct rpc_task *task);
static void call_decode(struct rpc_task *task);
+static void call_complete(struct rpc_task *task);
static void call_bind(struct rpc_task *task);
static void call_bind_status(struct rpc_task *task);
static void call_transmit(struct rpc_task *task);
@@ -1417,6 +1418,8 @@ rpc_xdr_encode(struct rpc_task *task)
(char *)req->rq_buffer + req->rq_callsize,
req->rq_rcvsize);
+ req->rq_snd_buf.destructor = &req->destructor;
+
p = rpc_encode_header(task);
if (p == NULL) {
printk(KERN_INFO "RPC: couldn't encode RPC header, exit EIO\n");
@@ -1582,6 +1585,7 @@ call_connect_status(struct rpc_task *task)
static void
call_transmit(struct rpc_task *task)
{
+ struct rpc_rqst *req = task->tk_rqstp;
dprint_status(task);
task->tk_action = call_status;
@@ -1615,8 +1619,8 @@ call_transmit(struct rpc_task *task)
call_transmit_status(task);
if (rpc_reply_expected(task))
return;
- task->tk_action = rpc_exit_task;
- rpc_wake_up_queued_task(&task->tk_xprt->pending, task);
+ task->tk_action = call_complete;
+ skb_frag_destructor_unref(&req->destructor);
}
/*
@@ -1689,7 +1693,8 @@ call_bc_transmit(struct rpc_task *task)
return;
}
- task->tk_action = rpc_exit_task;
+ task->tk_action = call_complete;
+ skb_frag_destructor_unref(&req->destructor);
if (task->tk_status < 0) {
printk(KERN_NOTICE "RPC: Could not send backchannel reply "
"error: %d\n", task->tk_status);
@@ -1729,7 +1734,6 @@ call_bc_transmit(struct rpc_task *task)
"error: %d\n", task->tk_status);
break;
}
- rpc_wake_up_queued_task(&req->rq_xprt->pending, task);
}
#endif /* CONFIG_SUNRPC_BACKCHANNEL */
@@ -1907,12 +1911,14 @@ call_decode(struct rpc_task *task)
return;
}
- task->tk_action = rpc_exit_task;
+ task->tk_action = call_complete;
if (decode) {
task->tk_status = rpcauth_unwrap_resp(task, decode, req, p,
task->tk_msg.rpc_resp);
}
+ rpc_sleep_on(&req->rq_xprt->pending, task, NULL);
+ skb_frag_destructor_unref(&req->destructor);
dprintk("RPC: %5u call_decode result %d\n", task->tk_pid,
task->tk_status);
return;
@@ -1927,6 +1933,17 @@ out_retry:
}
}
+/*
+ * 8. Wait for pages to be released by the network stack.
+ */
+static void
+call_complete(struct rpc_task *task)
+{
+ dprintk("RPC: %5u call_complete result %d\n",
+ task->tk_pid, task->tk_status);
+ task->tk_action = rpc_exit_task;
+}
+
static __be32 *
rpc_encode_header(struct rpc_task *task)
{
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 706305b..efa95df 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -198,7 +198,8 @@ int svc_send_common(struct socket *sock, struct xdr_buf *xdr,
while (pglen > 0) {
if (slen == size)
flags = 0;
- result = kernel_sendpage(sock, *ppage, NULL, base, size, flags);
+ result = kernel_sendpage(sock, *ppage, xdr->destructor,
+ base, size, flags);
if (result > 0)
len += result;
if (result != size)
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 0cbcd1a..a252759 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -1108,6 +1108,16 @@ static inline void xprt_init_xid(struct rpc_xprt *xprt)
xprt->xid = net_random();
}
+static int xprt_complete_skb_pages(struct skb_frag_destructor *destroy)
+{
+ struct rpc_rqst *req =
+ container_of(destroy, struct rpc_rqst, destructor);
+
+ dprintk("RPC: %5u completing skb pages\n", req->rq_task->tk_pid);
+ rpc_wake_up_queued_task(&req->rq_xprt->pending, req->rq_task);
+ return 0;
+}
+
static void xprt_request_init(struct rpc_task *task, struct rpc_xprt *xprt)
{
struct rpc_rqst *req = task->tk_rqstp;
@@ -1120,6 +1130,8 @@ static void xprt_request_init(struct rpc_task *task, struct rpc_xprt *xprt)
req->rq_xid = xprt_alloc_xid(xprt);
req->rq_release_snd_buf = NULL;
xprt_reset_majortimeo(req);
+ atomic_set(&req->destructor.ref, 1);
+ req->destructor.destroy = &xprt_complete_skb_pages;
dprintk("RPC: %5u reserved req %p xid %08x\n", task->tk_pid,
req, ntohl(req->rq_xid));
}
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index f05082b..b6ee8b7 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -408,7 +408,8 @@ static int xs_send_pagedata(struct socket *sock, struct xdr_buf *xdr, unsigned i
remainder -= len;
if (remainder != 0 || more)
flags |= MSG_MORE;
- err = sock->ops->sendpage(sock, *ppage, NULL, base, len, flags);
+ err = sock->ops->sendpage(sock, *ppage, xdr->destructor,
+ base, len, flags);
if (remainder == 0 || err != len)
break;
sent += err;
--
1.7.2.5
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] pppoatm: Fix excessive queue bloat
From: chas williams - CONTRACTOR @ 2012-04-10 14:26 UTC (permalink / raw)
To: David Woodhouse; +Cc: netdev, David Miller, paulus, Eric Dumazet
In-Reply-To: <1333914837.31812.57.camel@shinybook.infradead.org>
On Sun, 08 Apr 2012 21:53:57 +0200
David Woodhouse <dwmw2@infradead.org> wrote:
> Seriously, this gets *much* easier if we just ditch the checks against
> sk_sndbuf. We just wake up whenever decrementing ->inflight from zero.
> Can I?
i dont know. on a 'low' speed connection like queuing 2 packets might
be enough to keep something busy but imagine an interace like oc3 or
oc12. i dont know anything running pppoatm on such an interface but it
seems like just dropping sk_sndbuf isnt right either.
sk_sndbuf is per vcc and that isnt right either since the transmit
limit is actually the atm device's transmit queue depth. in your case,
since you are only using a single vcc, this problem does somewhat map
to a sk_sndbuf. but sk_sndbuf counts bytes and not entries in a queue
which is typically the limit imposed by the hardware.
it makes me wonder if we dont need to add another stub to the atm
driver interface to check if the device can transmit instead of 'can we
queue on the vcc'. atm_may_send() then would be changed to check to see
if a transmit queue entry is available and there is sk_sndbuf queue
space. if a driver doesnt have this stub to check for queue space,
atm_may_send() can just assume there is a queue entry and we get the
previous default behavior. what is the "queue depth" of your atm
device's transmit queue?
yes, the vcc->sk_sndbuf should be MAX(vcc->sk_sndbuf, pppoatm->MTU) but
since people dont often change the sk_sndbuf i guess no one has run
into this issue. pppoatm (et al) should enforce this limit though.
^ permalink raw reply
* [PATCH 03/10] chelsio: use SKB_WITH_OVERHEAD
From: Ian Campbell @ 2012-04-10 14:26 UTC (permalink / raw)
To: netdev
Cc: David Miller, Eric Dumazet, Michael S. Tsirkin, Wei Liu,
xen-devel, Ian Campbell, Divy Le Ray
In-Reply-To: <1334067965.5394.22.camel@zakaz.uk.xensource.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Divy Le Ray <divy@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb/sge.c | 3 +--
drivers/net/ethernet/chelsio/cxgb3/sge.c | 6 +++---
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb/sge.c b/drivers/net/ethernet/chelsio/cxgb/sge.c
index 47a8435..52373db 100644
--- a/drivers/net/ethernet/chelsio/cxgb/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb/sge.c
@@ -599,8 +599,7 @@ static int alloc_rx_resources(struct sge *sge, struct sge_params *p)
sizeof(struct cpl_rx_data) +
sge->freelQ[!sge->jumbo_fl].dma_offset;
- size = (16 * 1024) -
- SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ size = SKB_WITH_OVERHEAD(16 * 1024);
sge->freelQ[sge->jumbo_fl].rx_buffer_size = size;
diff --git a/drivers/net/ethernet/chelsio/cxgb3/sge.c b/drivers/net/ethernet/chelsio/cxgb3/sge.c
index cfb60e1..b804470 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/sge.c
@@ -3043,7 +3043,7 @@ int t3_sge_alloc_qset(struct adapter *adapter, unsigned int id, int nports,
q->fl[1].buf_size = FL1_PG_CHUNK_SIZE;
#else
q->fl[1].buf_size = is_offload(adapter) ?
- (16 * 1024) - SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) :
+ SKB_WITH_OVERHEAD(16 * 1024) :
MAX_FRAME_SIZE + 2 + sizeof(struct cpl_rx_pkt);
#endif
@@ -3282,8 +3282,8 @@ void t3_sge_prep(struct adapter *adap, struct sge_params *p)
{
int i;
- p->max_pkt_size = (16 * 1024) - sizeof(struct cpl_rx_data) -
- SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ p->max_pkt_size =
+ SKB_WITH_OVERHEAD((16*1024) - sizeof(struct cpl_rx_data));
for (i = 0; i < SGE_QSETS; ++i) {
struct qset_params *q = p->qset + i;
--
1.7.2.5
^ permalink raw reply related
* [PATCH 05/10] net: move destructor_arg to the front of sk_buff.
From: Ian Campbell @ 2012-04-10 14:26 UTC (permalink / raw)
To: netdev
Cc: David Miller, Eric Dumazet, Michael S. Tsirkin, Wei Liu,
xen-devel, Ian Campbell
In-Reply-To: <1334067965.5394.22.camel@zakaz.uk.xensource.com>
As of the previous patch we align the end (rather than the start) of the struct
to a cache line and so, with 32 and 64 byte cache lines and the shinfo size
increase from the next patch, the first 8 bytes of the struct end up on a
different cache line to the rest of it so make sure it is something relatively
unimportant to avoid hitting an extra cache line on hot operations such as
kfree_skb.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/skbuff.h | 15 ++++++++++-----
net/core/skbuff.c | 5 ++++-
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 0ad6a46..f0ae39c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -265,6 +265,15 @@ struct ubuf_info {
* the end of the header data, ie. at skb->end.
*/
struct skb_shared_info {
+ /* Intermediate layers must ensure that destructor_arg
+ * remains valid until skb destructor */
+ void *destructor_arg;
+
+ /*
+ * Warning: all fields from here until dataref are cleared in
+ * __alloc_skb()
+ *
+ */
unsigned char nr_frags;
__u8 tx_flags;
unsigned short gso_size;
@@ -276,14 +285,10 @@ struct skb_shared_info {
__be32 ip6_frag_id;
/*
- * Warning : all fields before dataref are cleared in __alloc_skb()
+ * Warning: all fields before dataref are cleared in __alloc_skb()
*/
atomic_t dataref;
- /* Intermediate layers must ensure that destructor_arg
- * remains valid until skb destructor */
- void * destructor_arg;
-
/* must be last field, see pskb_expand_head() */
skb_frag_t frags[MAX_SKB_FRAGS];
};
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d4e139e..b8a41d6 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -214,7 +214,10 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
/* make sure we initialize shinfo sequentially */
shinfo = skb_shinfo(skb);
- memset(shinfo, 0, offsetof(struct skb_shared_info, dataref));
+
+ memset(&shinfo->nr_frags, 0,
+ offsetof(struct skb_shared_info, dataref)
+ - offsetof(struct skb_shared_info, nr_frags));
atomic_set(&shinfo->dataref, 1);
kmemcheck_annotate_variable(shinfo->destructor_arg);
--
1.7.2.5
^ permalink raw reply related
* [PATCH 06/10] net: add support for per-paged-fragment destructors
From: Ian Campbell @ 2012-04-10 14:26 UTC (permalink / raw)
To: netdev
Cc: David Miller, Eric Dumazet, Michael S. Tsirkin, Wei Liu,
xen-devel, Ian Campbell, Michał Mirosław
In-Reply-To: <1334067965.5394.22.camel@zakaz.uk.xensource.com>
Entities which care about the complete lifecycle of pages which they inject
into the network stack via an skb paged fragment can choose to set this
destructor in order to receive a callback when the stack is really finished
with a page (including all clones, retransmits, pull-ups etc etc).
This destructor will always be propagated alongside the struct page when
copying skb_frag_t->page. This is the reason I chose to embed the destructor in
a "struct { } page" within the skb_frag_t, rather than as a separate field,
since it allows existing code which propagates ->frags[N].page to Just
Work(tm).
When the destructor is present the page reference counting is done slightly
differently. No references are held by the network stack on the struct page (it
is up to the caller to manage this as necessary) instead the network stack will
track references via the count embedded in the destructor structure. When this
reference count reaches zero then the destructor will be called and the caller
can take the necesary steps to release the page (i.e. release the struct page
reference itself).
The intention is that callers can use this callback to delay completion to
_their_ callers until the network stack has completely released the page, in
order to prevent use-after-free or modification of data pages which are still
in use by the stack.
It is allowable (indeed expected) for a caller to share a single destructor
instance between multiple pages injected into the stack e.g. a group of pages
included in a single higher level operation might share a destructor which is
used to complete that higher level operation.
With this change and the previous two changes to shinfo alignment and field
orderring it is now the case tyhat on a 64 bit system with 64 byte cache lines,
everything from nr_frags until the end of frags[0] is on the same cacheline.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: "Michał Mirosław" <mirq-linux@rere.qmqm.pl>
Cc: netdev@vger.kernel.org
---
include/linux/skbuff.h | 43 +++++++++++++++++++++++++++++++++++++++++++
net/core/skbuff.c | 17 +++++++++++++++++
2 files changed, 60 insertions(+), 0 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f0ae39c..6ac283e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -166,9 +166,15 @@ struct sk_buff;
typedef struct skb_frag_struct skb_frag_t;
+struct skb_frag_destructor {
+ atomic_t ref;
+ int (*destroy)(struct skb_frag_destructor *destructor);
+};
+
struct skb_frag_struct {
struct {
struct page *p;
+ struct skb_frag_destructor *destructor;
} page;
#if (BITS_PER_LONG > 32) || (PAGE_SIZE >= 65536)
__u32 page_offset;
@@ -1221,6 +1227,31 @@ static inline int skb_pagelen(const struct sk_buff *skb)
}
/**
+ * skb_frag_set_destructor - set destructor for a paged fragment
+ * @skb: buffer containing fragment to be initialised
+ * @i: paged fragment index to initialise
+ * @destroy: the destructor to use for this fragment
+ *
+ * Sets @destroy as the destructor to be called when all references to
+ * the frag @i in @skb (tracked over skb_clone, retransmit, pull-ups,
+ * etc) are released.
+ *
+ * When a destructor is set then reference counting is performed on
+ * @destroy->ref. When the ref reaches zero then @destroy->destroy
+ * will be called. The caller is responsible for holding and managing
+ * any other references (such a the struct page reference count).
+ *
+ * This function must be called before any use of skb_frag_ref() or
+ * skb_frag_unref().
+ */
+static inline void skb_frag_set_destructor(struct sk_buff *skb, int i,
+ struct skb_frag_destructor *destroy)
+{
+ skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+ frag->page.destructor = destroy;
+}
+
+/**
* __skb_fill_page_desc - initialise a paged fragment in an skb
* @skb: buffer containing fragment to be initialised
* @i: paged fragment index to initialise
@@ -1239,6 +1270,7 @@ static inline void __skb_fill_page_desc(struct sk_buff *skb, int i,
skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
frag->page.p = page;
+ frag->page.destructor = NULL;
frag->page_offset = off;
skb_frag_size_set(frag, size);
}
@@ -1743,6 +1775,9 @@ static inline struct page *skb_frag_page(const skb_frag_t *frag)
return frag->page.p;
}
+extern void skb_frag_destructor_ref(struct skb_frag_destructor *destroy);
+extern void skb_frag_destructor_unref(struct skb_frag_destructor *destroy);
+
/**
* __skb_frag_ref - take an addition reference on a paged fragment.
* @frag: the paged fragment
@@ -1751,6 +1786,10 @@ static inline struct page *skb_frag_page(const skb_frag_t *frag)
*/
static inline void __skb_frag_ref(skb_frag_t *frag)
{
+ if (unlikely(frag->page.destructor)) {
+ skb_frag_destructor_ref(frag->page.destructor);
+ return;
+ }
get_page(skb_frag_page(frag));
}
@@ -1774,6 +1813,10 @@ static inline void skb_frag_ref(struct sk_buff *skb, int f)
*/
static inline void __skb_frag_unref(skb_frag_t *frag)
{
+ if (unlikely(frag->page.destructor)) {
+ skb_frag_destructor_unref(frag->page.destructor);
+ return;
+ }
put_page(skb_frag_page(frag));
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index b8a41d6..9ec88ce 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -349,6 +349,23 @@ struct sk_buff *dev_alloc_skb(unsigned int length)
}
EXPORT_SYMBOL(dev_alloc_skb);
+void skb_frag_destructor_ref(struct skb_frag_destructor *destroy)
+{
+ BUG_ON(destroy == NULL);
+ atomic_inc(&destroy->ref);
+}
+EXPORT_SYMBOL(skb_frag_destructor_ref);
+
+void skb_frag_destructor_unref(struct skb_frag_destructor *destroy)
+{
+ if (destroy == NULL)
+ return;
+
+ if (atomic_dec_and_test(&destroy->ref))
+ destroy->destroy(destroy);
+}
+EXPORT_SYMBOL(skb_frag_destructor_unref);
+
static void skb_drop_list(struct sk_buff **listp)
{
struct sk_buff *list = *listp;
--
1.7.2.5
^ permalink raw reply related
* [PATCH 08/10] net: add skb_orphan_frags to copy aside frags with destructors
From: Ian Campbell @ 2012-04-10 14:26 UTC (permalink / raw)
To: netdev
Cc: David Miller, Eric Dumazet, Michael S. Tsirkin, Wei Liu,
xen-devel, Ian Campbell
In-Reply-To: <1334067965.5394.22.camel@zakaz.uk.xensource.com>
This should be used by drivers which need to hold on to an skb for an extended
(perhaps unbounded) period of time. e.g. the tun driver which relies on
userspace consuming the skb.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: mst@redhat.com
---
drivers/net/tun.c | 1 +
include/linux/skbuff.h | 11 ++++++++
net/core/skbuff.c | 67 ++++++++++++++++++++++++++++++++++-------------
3 files changed, 60 insertions(+), 19 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 74d7f76..b20789e 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -416,6 +416,7 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
/* Orphan the skb - required as we might hang on to it
* for indefinite time. */
skb_orphan(skb);
+ skb_orphan_frags(skb, GFP_KERNEL);
/* Enqueue packet */
skb_queue_tail(&tun->socket.sk->sk_receive_queue, skb);
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 8593ac2..77145f0 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1688,6 +1688,17 @@ static inline void skb_orphan(struct sk_buff *skb)
}
/**
+ * skb_orphan_frags - orphan the frags contained in a buffer
+ * @skb: buffer to orphan frags from
+ * @gfp_mask: allocation mask for replacement pages
+ *
+ * For each frag in the SKB which has a destructor (i.e. has an
+ * owner) create a copy of that frag and release the original
+ * page by calling the destructor.
+ */
+extern int skb_orphan_frags(struct sk_buff *skb, gfp_t gfp_mask);
+
+/**
* __skb_queue_purge - empty a list
* @list: list to empty
*
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index e63a4a6..d0a1603 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -688,31 +688,25 @@ struct sk_buff *skb_morph(struct sk_buff *dst, struct sk_buff *src)
}
EXPORT_SYMBOL_GPL(skb_morph);
-/* skb_copy_ubufs - copy userspace skb frags buffers to kernel
- * @skb: the skb to modify
- * @gfp_mask: allocation priority
- *
- * This must be called on SKBTX_DEV_ZEROCOPY skb.
- * It will copy all frags into kernel and drop the reference
- * to userspace pages.
- *
- * If this function is called from an interrupt gfp_mask() must be
- * %GFP_ATOMIC.
- *
- * Returns 0 on success or a negative error code on failure
- * to allocate kernel memory to copy to.
+/*
+ * If uarg != NULL copy and replace all frags.
+ * If uarg == NULL then only copy and replace those which have a destructor
+ * pointer.
*/
-int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
+static int skb_copy_frags(struct sk_buff *skb, gfp_t gfp_mask,
+ struct ubuf_info *uarg)
{
int i;
int num_frags = skb_shinfo(skb)->nr_frags;
struct page *page, *head = NULL;
- struct ubuf_info *uarg = skb_shinfo(skb)->destructor_arg;
for (i = 0; i < num_frags; i++) {
u8 *vaddr;
skb_frag_t *f = &skb_shinfo(skb)->frags[i];
+ if (!uarg && !f->page.destructor)
+ continue;
+
page = alloc_page(GFP_ATOMIC);
if (!page) {
while (head) {
@@ -730,11 +724,16 @@ int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
head = page;
}
- /* skb frags release userspace buffers */
- for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
+ /* skb frags release buffers */
+ for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+ skb_frag_t *f = &skb_shinfo(skb)->frags[i];
+ if (!uarg && !f->page.destructor)
+ continue;
skb_frag_unref(skb, i);
+ }
- uarg->callback(uarg);
+ if (uarg)
+ uarg->callback(uarg);
/* skb frags point to kernel buffers */
for (i = skb_shinfo(skb)->nr_frags; i > 0; i--) {
@@ -743,10 +742,40 @@ int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
head = (struct page *)head->private;
}
- skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
return 0;
}
+/* skb_copy_ubufs - copy userspace skb frags buffers to kernel
+ * @skb: the skb to modify
+ * @gfp_mask: allocation priority
+ *
+ * This must be called on SKBTX_DEV_ZEROCOPY skb.
+ * It will copy all frags into kernel and drop the reference
+ * to userspace pages.
+ *
+ * If this function is called from an interrupt gfp_mask() must be
+ * %GFP_ATOMIC.
+ *
+ * Returns 0 on success or a negative error code on failure
+ * to allocate kernel memory to copy to.
+ */
+int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
+{
+ struct ubuf_info *uarg = skb_shinfo(skb)->destructor_arg;
+ int rc;
+
+ rc = skb_copy_frags(skb, gfp_mask, uarg);
+
+ if (rc == 0)
+ skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
+
+ return rc;
+}
+
+int skb_orphan_frags(struct sk_buff *skb, gfp_t gfp_mask)
+{
+ return skb_copy_frags(skb, gfp_mask, NULL);
+}
/**
* skb_clone - duplicate an sk_buff
--
1.7.2.5
^ permalink raw reply related
* [PATCH 01/10] net: add and use SKB_ALLOCSIZE
From: Ian Campbell @ 2012-04-10 14:26 UTC (permalink / raw)
To: netdev
Cc: David Miller, Eric Dumazet, Michael S. Tsirkin, Wei Liu,
xen-devel, Ian Campbell
In-Reply-To: <1334067965.5394.22.camel@zakaz.uk.xensource.com>
This gives the allocation size required for an skb containing X bytes of data
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
drivers/net/ethernet/broadcom/bnx2.c | 7 +++----
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 3 +--
drivers/net/ethernet/broadcom/tg3.c | 3 +--
include/linux/skbuff.h | 12 ++++++++++++
net/core/skbuff.c | 8 +-------
5 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c
index 8297e28..dede71f 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -5321,8 +5321,7 @@ bnx2_set_rx_ring_size(struct bnx2 *bp, u32 size)
/* 8 for CRC and VLAN */
rx_size = bp->dev->mtu + ETH_HLEN + BNX2_RX_OFFSET + 8;
- rx_space = SKB_DATA_ALIGN(rx_size + BNX2_RX_ALIGN) + NET_SKB_PAD +
- SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ rx_space = SKB_ALLOCSIZE(rx_size + BNX2_RX_ALIGN) + NET_SKB_PAD;
bp->rx_copy_thresh = BNX2_RX_COPY_THRESH;
bp->rx_pg_ring_size = 0;
@@ -5345,8 +5344,8 @@ bnx2_set_rx_ring_size(struct bnx2 *bp, u32 size)
bp->rx_buf_use_size = rx_size;
/* hw alignment + build_skb() overhead*/
- bp->rx_buf_size = SKB_DATA_ALIGN(bp->rx_buf_use_size + BNX2_RX_ALIGN) +
- NET_SKB_PAD + SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ bp->rx_buf_size = SKB_ALLOCSIZE(bp->rx_buf_use_size + BNX2_RX_ALIGN) +
+ NET_SKB_PAD;
bp->rx_jumbo_thresh = rx_size - BNX2_RX_OFFSET;
bp->rx_ring_size = size;
bp->rx_max_ring = bnx2_find_max_ring(size, MAX_RX_RINGS);
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index e37161f..12f2ceb 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -1229,8 +1229,7 @@ struct bnx2x {
#define BNX2X_FW_RX_ALIGN_START (1UL << BNX2X_RX_ALIGN_SHIFT)
#define BNX2X_FW_RX_ALIGN_END \
- max(1UL << BNX2X_RX_ALIGN_SHIFT, \
- SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
+ max(1UL << BNX2X_RX_ALIGN_SHIFT, SKB_ALLOCSIZE(0))
#define BNX2X_PXP_DRAM_ALIGN (BNX2X_RX_ALIGN_SHIFT - 5)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 7b71387..4d4b063 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -5672,8 +5672,7 @@ static int tg3_alloc_rx_data(struct tg3 *tp, struct tg3_rx_prodring_set *tpr,
* Callers depend upon this behavior and assume that
* we leave everything unchanged if we fail.
*/
- skb_size = SKB_DATA_ALIGN(data_size + TG3_RX_OFFSET(tp)) +
- SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ skb_size = SKB_ALLOCSIZE(data_size + TG3_RX_OFFSET(tp));
data = kmalloc(skb_size, GFP_ATOMIC);
if (!data)
return -ENOMEM;
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 192250b..fbc92b2 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -41,8 +41,20 @@
#define SKB_DATA_ALIGN(X) (((X) + (SMP_CACHE_BYTES - 1)) & \
~(SMP_CACHE_BYTES - 1))
+/* maximum data size which can fit into an allocation of X bytes */
#define SKB_WITH_OVERHEAD(X) \
((X) - SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
+/*
+ * minimum allocation size required for an skb containing X bytes of data
+ *
+ * We do our best to align skb_shared_info on a separate cache
+ * line. It usually works because kmalloc(X > SMP_CACHE_BYTES) gives
+ * aligned memory blocks, unless SLUB/SLAB debug is enabled. Both
+ * skb->head and skb_shared_info are cache line aligned.
+ */
+#define SKB_ALLOCSIZE(X) \
+ (SKB_DATA_ALIGN((X)) + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
+
#define SKB_MAX_ORDER(X, ORDER) \
SKB_WITH_OVERHEAD((PAGE_SIZE << (ORDER)) - (X))
#define SKB_MAX_HEAD(X) (SKB_MAX_ORDER((X), 0))
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index a690cae..59a1ecb 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -184,13 +184,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
goto out;
prefetchw(skb);
- /* We do our best to align skb_shared_info on a separate cache
- * line. It usually works because kmalloc(X > SMP_CACHE_BYTES) gives
- * aligned memory blocks, unless SLUB/SLAB debug is enabled.
- * Both skb->head and skb_shared_info are cache line aligned.
- */
- size = SKB_DATA_ALIGN(size);
- size += SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ size = SKB_ALLOCSIZE(size);
data = kmalloc_node_track_caller(size, gfp_mask, node);
if (!data)
goto nodata;
--
1.7.2.5
^ permalink raw reply related
* [PATCH 02/10] net: Use SKB_WITH_OVERHEAD in build_skb
From: Ian Campbell @ 2012-04-10 14:26 UTC (permalink / raw)
To: netdev
Cc: David Miller, Eric Dumazet, Michael S. Tsirkin, Wei Liu,
xen-devel, Ian Campbell
In-Reply-To: <1334067965.5394.22.camel@zakaz.uk.xensource.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
net/core/skbuff.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 59a1ecb..d4e139e 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -264,7 +264,7 @@ struct sk_buff *build_skb(void *data)
if (!skb)
return NULL;
- size = ksize(data) - SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ size = SKB_WITH_OVERHEAD(ksize(data));
memset(skb, 0, offsetof(struct sk_buff, tail));
skb->truesize = SKB_TRUESIZE(size);
--
1.7.2.5
^ permalink raw reply related
* [PATCH 07/10] net: only allow paged fragments with the same destructor to be coalesced.
From: Ian Campbell @ 2012-04-10 14:26 UTC (permalink / raw)
To: netdev
Cc: David Miller, Eric Dumazet, Michael S. Tsirkin, Wei Liu,
xen-devel, Ian Campbell, Alexey Kuznetsov, Pekka Savola (ipv6),
James Morris, Hideaki YOSHIFUJI, Patrick McHardy,
Michał Mirosław
In-Reply-To: <1334067965.5394.22.camel@zakaz.uk.xensource.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: "Pekka Savola (ipv6)" <pekkas@netcore.fi>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: "Michał Mirosław" <mirq-linux@rere.qmqm.pl>
Cc: netdev@vger.kernel.org
---
include/linux/skbuff.h | 7 +++++--
net/core/skbuff.c | 1 +
net/ipv4/ip_output.c | 2 +-
net/ipv4/tcp.c | 4 ++--
4 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6ac283e..8593ac2 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2014,13 +2014,16 @@ static inline int skb_add_data(struct sk_buff *skb,
}
static inline int skb_can_coalesce(struct sk_buff *skb, int i,
- const struct page *page, int off)
+ const struct page *page,
+ const struct skb_frag_destructor *destroy,
+ int off)
{
if (i) {
const struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[i - 1];
return page == skb_frag_page(frag) &&
- off == frag->page_offset + skb_frag_size(frag);
+ off == frag->page_offset + skb_frag_size(frag) &&
+ frag->page.destructor == destroy;
}
return 0;
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 9ec88ce..e63a4a6 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2323,6 +2323,7 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
*/
if (!to ||
!skb_can_coalesce(tgt, to, skb_frag_page(fragfrom),
+ fragfrom->page.destructor,
fragfrom->page_offset)) {
merge = -1;
} else {
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index ff302bd..9e4eca6 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1243,7 +1243,7 @@ ssize_t ip_append_page(struct sock *sk, struct flowi4 *fl4, struct page *page,
i = skb_shinfo(skb)->nr_frags;
if (len > size)
len = size;
- if (skb_can_coalesce(skb, i, page, offset)) {
+ if (skb_can_coalesce(skb, i, page, NULL, offset)) {
skb_frag_size_add(&skb_shinfo(skb)->frags[i-1], len);
} else if (i < MAX_SKB_FRAGS) {
get_page(page);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index cfd7edd..b1612e9 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -804,7 +804,7 @@ new_segment:
copy = size;
i = skb_shinfo(skb)->nr_frags;
- can_coalesce = skb_can_coalesce(skb, i, page, offset);
+ can_coalesce = skb_can_coalesce(skb, i, page, NULL, offset);
if (!can_coalesce && i >= MAX_SKB_FRAGS) {
tcp_mark_push(tp, skb);
goto new_segment;
@@ -1013,7 +1013,7 @@ new_segment:
off = sk->sk_sndmsg_off;
- if (skb_can_coalesce(skb, i, page, off) &&
+ if (skb_can_coalesce(skb, i, page, NULL, off) &&
off != PAGE_SIZE) {
/* We can extend the last page
* fragment. */
--
1.7.2.5
^ permalink raw reply related
* [PATCH 04/10] net: pad skb data and shinfo as a whole rather than individually
From: Ian Campbell @ 2012-04-10 14:26 UTC (permalink / raw)
To: netdev
Cc: David Miller, Eric Dumazet, Michael S. Tsirkin, Wei Liu,
xen-devel, Ian Campbell
In-Reply-To: <1334067965.5394.22.camel@zakaz.uk.xensource.com>
This reduces the minimum overhead required for this allocation such that the
shinfo can be grown in the following patch without overflowing 2048 bytes for a
1500 byte frame.
Reducing this overhead while also growing the shinfo means that sometimes the
tail end of the data can end up in the same cache line as the beginning of the
shinfo. Specifically in the case of the 64 byte cache lines on a 64 bit system
the first 8 bytes of shinfo can overlap the tail cacheline of the data. In many
cases the allocation slop means that there is no overlap.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/skbuff.h | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index fbc92b2..0ad6a46 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -43,17 +43,20 @@
~(SMP_CACHE_BYTES - 1))
/* maximum data size which can fit into an allocation of X bytes */
#define SKB_WITH_OVERHEAD(X) \
- ((X) - SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
+ ((X) - sizeof(struct skb_shared_info))
/*
* minimum allocation size required for an skb containing X bytes of data
*
* We do our best to align skb_shared_info on a separate cache
* line. It usually works because kmalloc(X > SMP_CACHE_BYTES) gives
- * aligned memory blocks, unless SLUB/SLAB debug is enabled. Both
- * skb->head and skb_shared_info are cache line aligned.
+ * aligned memory blocks, unless SLUB/SLAB debug is enabled.
+ * skb->head is aligned to a cache line while the tail of
+ * skb_shared_info is cache line aligned. We arrange that the order
+ * of the fields in skb_shared_info is such that the interesting
+ * fields are cache line aligned and fit within a 64 byte cache line.
*/
#define SKB_ALLOCSIZE(X) \
- (SKB_DATA_ALIGN((X)) + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
+ (SKB_DATA_ALIGN((X) + sizeof(struct skb_shared_info)))
#define SKB_MAX_ORDER(X, ORDER) \
SKB_WITH_OVERHEAD((PAGE_SIZE << (ORDER)) - (X))
@@ -63,7 +66,7 @@
/* return minimum truesize of one skb containing X bytes of data */
#define SKB_TRUESIZE(X) ((X) + \
SKB_DATA_ALIGN(sizeof(struct sk_buff)) + \
- SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
+ sizeof(struct skb_shared_info))
/* A. Checksumming of received packets by device.
*
--
1.7.2.5
^ permalink raw reply related
* Re: [net-next PATCH v1 7/7] macvlan: add FDB bridge ops and new macvlan mode
From: Michael S. Tsirkin @ 2012-04-10 14:33 UTC (permalink / raw)
To: John Fastabend
Cc: roprabhu, stephen.hemminger, davem, hadi, bhutchings,
jeffrey.t.kirsher, netdev, gregory.v.rose, krkumar2, sri
In-Reply-To: <4F843AB2.5060901@intel.com>
On Tue, Apr 10, 2012 at 06:50:42AM -0700, John Fastabend wrote:
> On 4/10/2012 1:14 AM, Michael S. Tsirkin wrote:
> > On Tue, Apr 10, 2012 at 11:09:16AM +0300, Michael S. Tsirkin wrote:
> >> On Mon, Apr 09, 2012 at 03:00:54PM -0700, John Fastabend wrote:
> >>> This adds a new macvlan mode MACVLAN_PASSTHRU_NOPROMISC
> >>> this mode acts the same as the original passthru mode _except_
> >>> it does not set promiscuous mode on the lowerdev. Because the
> >>> lowerdev is not put in promiscuous mode any unicast or multicast
> >>> addresses the device should receive must be explicitely added
> >>> with the FDB bridge ops. In many use cases the management stack
> >>> will know the mac addresses needed (maybe negotiated via EVB/VDP)
> >>> or may require only receiving known "good" mac addresses. This
> >>> mode with the FDB ops supports this usage model.
> >>
> >>
> >> Looks good to me. Some questions below:
> >>
> >>> This patch is a result of Roopa Prabhu's work. Follow up
> >>> patches are needed for VEPA and VEB macvlan modes.
> >>
> >> And bridge too?
> >>
> >> Also, my understanding is that other modes won't need a flag
> >> like this since they don't put the device in promisc mode initially,
> >> so no assumptions are broken if we require all addresses
> >> to be declared, right?
> >>
> >> A final question: I think we'll later add a macvlan mode
> >> that does not flood all multicasts. This would change behaviour
> >> in an incompatible way so we'll probably need yet another
> >> flag. Would it make sense to combine this functionality
> >> with nopromisc so we have less modes to support?
> >
> > One other question I forgot:
> >
>
> [...]
>
> >>>
> >>> @@ -344,12 +346,15 @@ static int macvlan_stop(struct net_device *dev)
> >>> struct macvlan_dev *vlan = netdev_priv(dev);
> >>> struct net_device *lowerdev = vlan->lowerdev;
> >>>
> >>> + dev_uc_unsync(lowerdev, dev);
> >>> + dev_mc_unsync(lowerdev, dev);
> >>> +
> >>> if (vlan->port->passthru) {
> >>> - dev_set_promiscuity(lowerdev, -1);
> >>> + if (vlan->mode == MACVLAN_MODE_PASSTHRU)
> >>> + dev_set_promiscuity(lowerdev, 1);
> >>> goto hash_del;
> >>> }
> >>>
> >>> - dev_mc_unsync(lowerdev, dev);
> >>> if (dev->flags & IFF_ALLMULTI)
> >>> dev_set_allmulti(lowerdev, -1);
> >>>
> >>> @@ -399,10 +404,11 @@ static void macvlan_change_rx_flags(struct net_device *dev, int change)
> >>> dev_set_allmulti(lowerdev, dev->flags & IFF_ALLMULTI ? 1 : -1);
> >
> > In the new mode, do we want to have promisc on lowerdev follow whatever
> > is set on the macvlan, like we do for allmulti?
> > I'm not sure at this point - what do others think?
> >
>
> Just to enumerate why you would need this: (1) socket set with
> PACKET_MR_MULTICAST and (2) something like mrouted is running
> on the macvlan (3) maybe some case I missed?
>
> Don't you need CAP_NET_RAW to set these though anyways? So I
> wouldn't think it would be a problem. I assume if a user has
> CAP_NET_RAW or UUID 0 they really should be able to set this
> up.
>
> .John
I am not sure, really.
But I note that with a security mechanism such as selinux, CAP_NET_RAW
might be insufficient to change the underlying device.
So there might be value in being able to change it in
a controlled manner through macvlan.
There's also something to be said for being able to let
management deal with macvlan devices (and there are
some very complex tools for that around) while
keeping a simple script around for the physical
one and knowing that they won't disrupt each other.
--
MST
^ permalink raw reply
* Re: [net-next PATCH v1 7/7] macvlan: add FDB bridge ops and new macvlan mode
From: Michael S. Tsirkin @ 2012-04-10 14:35 UTC (permalink / raw)
To: John Fastabend
Cc: roprabhu, stephen.hemminger, davem, hadi, bhutchings,
jeffrey.t.kirsher, netdev, gregory.v.rose, krkumar2, sri
In-Reply-To: <4F8442F6.5070707@intel.com>
On Tue, Apr 10, 2012 at 07:25:58AM -0700, John Fastabend wrote:
> > Hmm okay, but this would mean we should convert
> > MACVLAN_MODE_PASSTHRU_NOPROMISC to something
> > that can combined with all modes. E.g.
> > MACVLAN_MODE_BRIDGE | MACVLAN_MODE_FLAG_XXXXX
> >
> > and document that it does not promise to flood
> > multicast.
> >
>
> How about changing MACVLAN_MODE_PASSTHRU_NOPROMISC -> MACVLAN_MODE_NOPORMISC
> for this patch. Then a follow on series can rework bridge
> and VEPA to use it as well.
Right. We probably need a better name if it's going to
affect other things besides promisc though.
> >> If you want
> >> the flood behavior you can create it by adding the addr to all
> >> the devices or just to a subset of them to get the non-flooding
> >> capabilities.
> >>
> >> .John
> >
> > BTW we seem to try to flood in pass-through too, not sure why.
> >
>
> Suppose your looking at macvlan_handle_frame()? It does
> seem unneeded.
>
> .John
^ permalink raw reply
* Re: net: more accurate skb truesize - regression on Microblaze
From: Michal Simek @ 2012-04-10 14:38 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev, john.williams
In-Reply-To: <20120410.101400.1433881148267812355.davem@davemloft.net>
On 04/10/2012 04:14 PM, David Miller wrote:
> From: Michal Simek<monstr@monstr.eu>
> Date: Tue, 10 Apr 2012 16:04:18 +0200
>
>> I didn't report problem on any private held driver. I think you have
>> misunderstand our later discussion about ll_temac driver where are
>> some problems which at least
>
> Thanks for the clarification.
No worries.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: [PATCH 01/10] net: add and use SKB_ALLOCSIZE
From: Eric Dumazet @ 2012-04-10 14:57 UTC (permalink / raw)
To: Ian Campbell; +Cc: netdev, David Miller, Michael S. Tsirkin, Wei Liu, xen-devel
In-Reply-To: <1334067984-7706-1-git-send-email-ian.campbell@citrix.com>
On Tue, 2012-04-10 at 15:26 +0100, Ian Campbell wrote:
> This gives the allocation size required for an skb containing X bytes of data
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> drivers/net/ethernet/broadcom/bnx2.c | 7 +++----
> drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 3 +--
> drivers/net/ethernet/broadcom/tg3.c | 3 +--
> include/linux/skbuff.h | 12 ++++++++++++
> net/core/skbuff.c | 8 +-------
> 5 files changed, 18 insertions(+), 15 deletions(-)
>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH 0/2] adding tracepoints to vhost
From: Zhi Yong Wu @ 2012-04-10 14:58 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization, linux-kernel, kvm, netdev
In-Reply-To: <20120410134522.GC18899@redhat.com>
On Tue, Apr 10, 2012 at 9:45 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Apr 10, 2012 at 09:10:48PM +0800, Zhi Yong Wu wrote:
>> >> Perhaps this can replace the vhost log feature? I'm not sure if
>> >> tracepoints support the right data types but it seems like vhost
>> >> debugging could be done using tracing with less code.
>> >>
>> >> Stefan
>> >
>> > vhost log is not a debugging tool, it logs memory accesses for
>> > migration.
>> Great, it is very appreciated if there's some docs about this
>
> About what? vhost logging? See the comment near the
Yeah, thanks
> definition of VHOST_SET_LOG_BASE in vhost.h
>
>> > _______________________________________________
>> > Virtualization mailing list
>> > Virtualization@lists.linux-foundation.org
>> > https://lists.linuxfoundation.org/mailman/listinfo/virtualization
>>
>>
>>
>> --
>> Regards,
>>
>> Zhi Yong Wu
--
Regards,
Zhi Yong Wu
^ permalink raw reply
* Re: [PATCH v4 0/10] skb paged fragment destructors
From: Michael S. Tsirkin @ 2012-04-10 14:58 UTC (permalink / raw)
To: Ian Campbell
Cc: netdev, David Miller, Eric Dumazet, Wei Liu, David VomLehn,
Bart Van Assche, xen-devel
In-Reply-To: <1334067965.5394.22.camel@zakaz.uk.xensource.com>
On Tue, Apr 10, 2012 at 03:26:05PM +0100, Ian Campbell wrote:
> * I can't for the life of me get anything to actually hit
> this code path. I've been trying with an NFS server
> running in a Xen HVM domain with emulated (e.g. tap)
> networking and a client in domain 0, using the NFS fix
> in this series which generates SKBs with destructors
> set, so far -- nothing. I suspect that lack of TSO/GSO
> etc on the TAP interface is causing the frags to be
> copied to normal pages during skb_segment().
To enable gso you need to call TUNSETOFFLOAD.
--
MST
^ permalink raw reply
* Re: [PATCH 02/10] net: Use SKB_WITH_OVERHEAD in build_skb
From: Eric Dumazet @ 2012-04-10 14:58 UTC (permalink / raw)
To: Ian Campbell; +Cc: netdev, David Miller, Michael S. Tsirkin, Wei Liu, xen-devel
In-Reply-To: <1334067984-7706-2-git-send-email-ian.campbell@citrix.com>
On Tue, 2012-04-10 at 15:26 +0100, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> net/core/skbuff.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 59a1ecb..d4e139e 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -264,7 +264,7 @@ struct sk_buff *build_skb(void *data)
> if (!skb)
> return NULL;
>
> - size = ksize(data) - SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> + size = SKB_WITH_OVERHEAD(ksize(data));
>
> memset(skb, 0, offsetof(struct sk_buff, tail));
> skb->truesize = SKB_TRUESIZE(size);
Well, why not ;)
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH 03/10] chelsio: use SKB_WITH_OVERHEAD
From: Eric Dumazet @ 2012-04-10 14:59 UTC (permalink / raw)
To: Ian Campbell
Cc: netdev, David Miller, Michael S. Tsirkin, Wei Liu, xen-devel,
Divy Le Ray
In-Reply-To: <1334067984-7706-3-git-send-email-ian.campbell@citrix.com>
On Tue, 2012-04-10 at 15:26 +0100, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Divy Le Ray <divy@chelsio.com>
> ---
> drivers/net/ethernet/chelsio/cxgb/sge.c | 3 +--
> drivers/net/ethernet/chelsio/cxgb3/sge.c | 6 +++---
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH v4 0/10] skb paged fragment destructors
From: Michael S. Tsirkin @ 2012-04-10 15:00 UTC (permalink / raw)
To: Ian Campbell
Cc: netdev, David Miller, Eric Dumazet, Wei Liu, David VomLehn,
Bart Van Assche, xen-devel
In-Reply-To: <1334067965.5394.22.camel@zakaz.uk.xensource.com>
On Tue, Apr 10, 2012 at 03:26:05PM +0100, Ian Campbell wrote:
> I think this is v4, but I've sort of lost count, sorry that it's taken
> me so long to get back to this stuff.
>
> The following series makes use of the skb fragment API (which is in 3.2
> +) to add a per-paged-fragment destructor callback. This can be used by
> creators of skbs who are interested in the lifecycle of the pages
> included in that skb after they have handed it off to the network stack.
>
> The mail at [0] contains some more background and rationale but
> basically the completed series will allow entities which inject pages
> into the networking stack to receive a notification when the stack has
> really finished with those pages (i.e. including retransmissions,
> clones, pull-ups etc) and not just when the original skb is finished
> with, which is beneficial to many subsystems which wish to inject pages
> into the network stack without giving up full ownership of those page's
> lifecycle. It implements something broadly along the lines of what was
> described in [1].
>
> I have also included a patch to the RPC subsystem which uses this API to
> fix the bug which I describe at [2].
>
> I've also had some interest from David VemLehn and Bart Van Assche
> regarding using this functionality in the context of vmsplice and iSCSI
> targets respectively (I think).
>
> Changes since last time:
>
> * Added skb_orphan_frags API for the use of recipients of SKBs who
> may hold onto the SKB for a long time (this is analogous to
> skb_orphan). This was pointed out by Michael. The TUN driver is
> currently the only user.
> * I can't for the life of me get anything to actually hit
> this code path. I've been trying with an NFS server
> running in a Xen HVM domain with emulated (e.g. tap)
> networking and a client in domain 0, using the NFS fix
> in this series which generates SKBs with destructors
> set, so far -- nothing. I suspect that lack of TSO/GSO
> etc on the TAP interface is causing the frags to be
> copied to normal pages during skb_segment().
Will take a look tomorrow, thanks!
> * Various fixups related to the change of alignment/padding in
> shinfo, in particular to build_skb as pointed out by Eric.
> * Tweaked ordering of shinfo members to ensure that all hotpath
> variables up to and including the first frag fit within (and are
> aligned to) a single 64 byte cache line. (Eric again)
>
> I ran a monothread UDP benchmark (similar to that described by Eric in
> e52fcb2462ac) and don't see any difference in pps throughput, it was
> ~810,000 pps both before and after.
>
> Cheers,
> Ian.
>
> [0] http://marc.info/?l=linux-netdev&m=131072801125521&w=2
> [1] http://marc.info/?l=linux-netdev&m=130925719513084&w=2
> [2] http://marc.info/?l=linux-nfs&m=122424132729720&w=2
>
>
>
>
^ permalink raw reply
* Re: [PATCH 04/10] net: pad skb data and shinfo as a whole rather than individually
From: Eric Dumazet @ 2012-04-10 15:01 UTC (permalink / raw)
To: Ian Campbell; +Cc: netdev, David Miller, Michael S. Tsirkin, Wei Liu, xen-devel
In-Reply-To: <1334067984-7706-4-git-send-email-ian.campbell@citrix.com>
On Tue, 2012-04-10 at 15:26 +0100, Ian Campbell wrote:
> This reduces the minimum overhead required for this allocation such that the
> shinfo can be grown in the following patch without overflowing 2048 bytes for a
> 1500 byte frame.
>
> Reducing this overhead while also growing the shinfo means that sometimes the
> tail end of the data can end up in the same cache line as the beginning of the
> shinfo. Specifically in the case of the 64 byte cache lines on a 64 bit system
> the first 8 bytes of shinfo can overlap the tail cacheline of the data. In many
> cases the allocation slop means that there is no overlap.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> ---
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* via-rhine: Problem with lost link after a while
From: Bjarke Istrup Pedersen @ 2012-04-10 15:03 UTC (permalink / raw)
To: Francois Romieu; +Cc: linux-kernel, netdev
Hey Francois,
There seems to be a few problems with the new via-rhine driver.
After going to 3.3, it seems to drop link from time to time (randomly
it seems) - this didn't happen when I was running the patches from you
on top of 3.2 when we where testing them back in january.
It seems like other people have having a few problems too:
https://bugzilla.redhat.com/show_bug.cgi?id=805801 and on the
soekris-tech mailing list today (the archive hasn't been updated, so I
cannot post a link yet)
Got any ideas?
I'm ready to help testing if needed.
/Bjarke
^ permalink raw reply
* Re: [PATCH 05/10] net: move destructor_arg to the front of sk_buff.
From: Eric Dumazet @ 2012-04-10 15:05 UTC (permalink / raw)
To: Ian Campbell; +Cc: netdev, David Miller, Michael S. Tsirkin, Wei Liu, xen-devel
In-Reply-To: <1334067984-7706-5-git-send-email-ian.campbell@citrix.com>
On Tue, 2012-04-10 at 15:26 +0100, Ian Campbell wrote:
> As of the previous patch we align the end (rather than the start) of the struct
> to a cache line and so, with 32 and 64 byte cache lines and the shinfo size
> increase from the next patch, the first 8 bytes of the struct end up on a
> different cache line to the rest of it so make sure it is something relatively
> unimportant to avoid hitting an extra cache line on hot operations such as
> kfree_skb.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> include/linux/skbuff.h | 15 ++++++++++-----
> net/core/skbuff.c | 5 ++++-
> 2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 0ad6a46..f0ae39c 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -265,6 +265,15 @@ struct ubuf_info {
> * the end of the header data, ie. at skb->end.
> */
> struct skb_shared_info {
> + /* Intermediate layers must ensure that destructor_arg
> + * remains valid until skb destructor */
> + void *destructor_arg;
> +
> + /*
> + * Warning: all fields from here until dataref are cleared in
> + * __alloc_skb()
> + *
> + */
> unsigned char nr_frags;
> __u8 tx_flags;
> unsigned short gso_size;
> @@ -276,14 +285,10 @@ struct skb_shared_info {
> __be32 ip6_frag_id;
>
> /*
> - * Warning : all fields before dataref are cleared in __alloc_skb()
> + * Warning: all fields before dataref are cleared in __alloc_skb()
> */
> atomic_t dataref;
>
> - /* Intermediate layers must ensure that destructor_arg
> - * remains valid until skb destructor */
> - void * destructor_arg;
> -
> /* must be last field, see pskb_expand_head() */
> skb_frag_t frags[MAX_SKB_FRAGS];
> };
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index d4e139e..b8a41d6 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -214,7 +214,10 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
>
> /* make sure we initialize shinfo sequentially */
> shinfo = skb_shinfo(skb);
> - memset(shinfo, 0, offsetof(struct skb_shared_info, dataref));
> +
> + memset(&shinfo->nr_frags, 0,
> + offsetof(struct skb_shared_info, dataref)
> + - offsetof(struct skb_shared_info, nr_frags));
> atomic_set(&shinfo->dataref, 1);
> kmemcheck_annotate_variable(shinfo->destructor_arg);
>
Not sure if we can do the same in build_skb()
^ permalink raw reply
* [patch net-next 0/5] team: add some features and fixes
From: Jiri Pirko @ 2012-04-10 15:15 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet
Jiri Pirko (5):
team: add support for per-port options
team: add bool option type
team: add user_linkup and user_linkup_enabled per-port option
team: ab: walk through port list non-rcu
team: add missed "statics"
drivers/net/team/team.c | 402 +++++++++++++++++++++++------
drivers/net/team/team_mode_activebackup.c | 20 +-
drivers/net/team/team_mode_loadbalance.c | 32 +--
include/linux/if_team.h | 58 +++--
4 files changed, 387 insertions(+), 125 deletions(-)
--
1.7.9.1
^ permalink raw reply
* [patch net-next 1/5] team: add support for per-port options
From: Jiri Pirko @ 2012-04-10 15:15 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet
In-Reply-To: <1334070946-7704-1-git-send-email-jpirko@redhat.com>
This patch allows to create per-port options. That becomes handy for all
sorts of stuff, for example for userspace driven link-state, 802.3ad
implementation and so on.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/team/team.c | 306 +++++++++++++++++++++++------
drivers/net/team/team_mode_activebackup.c | 14 +-
drivers/net/team/team_mode_loadbalance.c | 28 ++--
include/linux/if_team.h | 30 ++--
4 files changed, 278 insertions(+), 100 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index ea96f82..eaf8441 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -81,7 +81,16 @@ EXPORT_SYMBOL(team_port_set_team_mac);
* Options handling
*******************/
-struct team_option *__team_find_option(struct team *team, const char *opt_name)
+struct team_option_inst { /* One for each option instance */
+ struct list_head list;
+ struct team_option *option;
+ struct team_port *port; /* != NULL if per-port */
+ bool changed;
+ bool removed;
+};
+
+static struct team_option *__team_find_option(struct team *team,
+ const char *opt_name)
{
struct team_option *option;
@@ -92,9 +101,121 @@ struct team_option *__team_find_option(struct team *team, const char *opt_name)
return NULL;
}
-int __team_options_register(struct team *team,
- const struct team_option *option,
- size_t option_count)
+static int __team_option_inst_add(struct team *team, struct team_option *option,
+ struct team_port *port)
+{
+ struct team_option_inst *opt_inst;
+
+ opt_inst = kmalloc(sizeof(*opt_inst), GFP_KERNEL);
+ if (!opt_inst)
+ return -ENOMEM;
+ opt_inst->option = option;
+ opt_inst->port = port;
+ opt_inst->changed = true;
+ opt_inst->removed = false;
+ list_add_tail(&opt_inst->list, &team->option_inst_list);
+ return 0;
+}
+
+static void __team_option_inst_del(struct team_option_inst *opt_inst)
+{
+ list_del(&opt_inst->list);
+ kfree(opt_inst);
+}
+
+static void __team_option_inst_del_option(struct team *team,
+ struct team_option *option)
+{
+ struct team_option_inst *opt_inst, *tmp;
+
+ list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
+ if (opt_inst->option == option)
+ __team_option_inst_del(opt_inst);
+ }
+}
+
+static int __team_option_inst_add_option(struct team *team,
+ struct team_option *option)
+{
+ struct team_port *port;
+ int err;
+
+ if (!option->per_port)
+ return __team_option_inst_add(team, option, 0);
+
+ list_for_each_entry(port, &team->port_list, list) {
+ err = __team_option_inst_add(team, option, port);
+ if (err)
+ goto inst_del_option;
+ }
+ return 0;
+
+inst_del_option:
+ __team_option_inst_del_option(team, option);
+ return err;
+}
+
+static void __team_option_inst_mark_removed_option(struct team *team,
+ struct team_option *option)
+{
+ struct team_option_inst *opt_inst;
+
+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
+ if (opt_inst->option == option) {
+ opt_inst->changed = true;
+ opt_inst->removed = true;
+ }
+ }
+}
+
+static void __team_option_inst_del_port(struct team *team,
+ struct team_port *port)
+{
+ struct team_option_inst *opt_inst, *tmp;
+
+ list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
+ if (opt_inst->option->per_port &&
+ opt_inst->port == port)
+ __team_option_inst_del(opt_inst);
+ }
+}
+
+static int __team_option_inst_add_port(struct team *team,
+ struct team_port *port)
+{
+ struct team_option *option;
+ int err;
+
+ list_for_each_entry(option, &team->option_list, list) {
+ if (!option->per_port)
+ continue;
+ err = __team_option_inst_add(team, option, port);
+ if (err)
+ goto inst_del_port;
+ }
+ return 0;
+
+inst_del_port:
+ __team_option_inst_del_port(team, port);
+ return err;
+}
+
+static void __team_option_inst_mark_removed_port(struct team *team,
+ struct team_port *port)
+{
+ struct team_option_inst *opt_inst;
+
+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
+ if (opt_inst->port == port) {
+ opt_inst->changed = true;
+ opt_inst->removed = true;
+ }
+ }
+}
+
+static int __team_options_register(struct team *team,
+ const struct team_option *option,
+ size_t option_count)
{
int i;
struct team_option **dst_opts;
@@ -107,26 +228,32 @@ int __team_options_register(struct team *team,
for (i = 0; i < option_count; i++, option++) {
if (__team_find_option(team, option->name)) {
err = -EEXIST;
- goto rollback;
+ goto alloc_rollback;
}
dst_opts[i] = kmemdup(option, sizeof(*option), GFP_KERNEL);
if (!dst_opts[i]) {
err = -ENOMEM;
- goto rollback;
+ goto alloc_rollback;
}
}
for (i = 0; i < option_count; i++) {
- dst_opts[i]->changed = true;
- dst_opts[i]->removed = false;
+ err = __team_option_inst_add_option(team, dst_opts[i]);
+ if (err)
+ goto inst_rollback;
list_add_tail(&dst_opts[i]->list, &team->option_list);
}
kfree(dst_opts);
return 0;
-rollback:
- for (i = 0; i < option_count; i++)
+inst_rollback:
+ for (i--; i >= 0; i--)
+ __team_option_inst_del_option(team, dst_opts[i]);
+
+ i = option_count - 1;
+alloc_rollback:
+ for (i--; i >= 0; i--)
kfree(dst_opts[i]);
kfree(dst_opts);
@@ -143,10 +270,8 @@ static void __team_options_mark_removed(struct team *team,
struct team_option *del_opt;
del_opt = __team_find_option(team, option->name);
- if (del_opt) {
- del_opt->changed = true;
- del_opt->removed = true;
- }
+ if (del_opt)
+ __team_option_inst_mark_removed_option(team, del_opt);
}
}
@@ -161,6 +286,7 @@ static void __team_options_unregister(struct team *team,
del_opt = __team_find_option(team, option->name);
if (del_opt) {
+ __team_option_inst_del_option(team, del_opt);
list_del(&del_opt->list);
kfree(del_opt);
}
@@ -193,22 +319,42 @@ void team_options_unregister(struct team *team,
}
EXPORT_SYMBOL(team_options_unregister);
-static int team_option_get(struct team *team, struct team_option *option,
- void *arg)
+static int team_option_port_add(struct team *team, struct team_port *port)
{
- return option->getter(team, arg);
+ int err;
+
+ err = __team_option_inst_add_port(team, port);
+ if (err)
+ return err;
+ __team_options_change_check(team);
+ return 0;
}
-static int team_option_set(struct team *team, struct team_option *option,
- void *arg)
+static void team_option_port_del(struct team *team, struct team_port *port)
+{
+ __team_option_inst_mark_removed_port(team, port);
+ __team_options_change_check(team);
+ __team_option_inst_del_port(team, port);
+}
+
+static int team_option_get(struct team *team,
+ struct team_option_inst *opt_inst,
+ struct team_gsetter_ctx *ctx)
+{
+ return opt_inst->option->getter(team, ctx);
+}
+
+static int team_option_set(struct team *team,
+ struct team_option_inst *opt_inst,
+ struct team_gsetter_ctx *ctx)
{
int err;
- err = option->setter(team, arg);
+ err = opt_inst->option->setter(team, ctx);
if (err)
return err;
- option->changed = true;
+ opt_inst->changed = true;
__team_options_change_check(team);
return err;
}
@@ -642,6 +788,13 @@ static int team_port_add(struct team *team, struct net_device *port_dev)
goto err_handler_register;
}
+ err = team_option_port_add(team, port);
+ if (err) {
+ netdev_err(dev, "Device %s failed to add per-port options\n",
+ portname);
+ goto err_option_port_add;
+ }
+
team_port_list_add_port(team, port);
team_adjust_ops(team);
__team_compute_features(team);
@@ -651,6 +804,9 @@ static int team_port_add(struct team *team, struct net_device *port_dev)
return 0;
+err_option_port_add:
+ netdev_rx_handler_unregister(port_dev);
+
err_handler_register:
netdev_set_master(port_dev, NULL);
@@ -690,6 +846,7 @@ static int team_port_del(struct team *team, struct net_device *port_dev)
__team_port_change_check(port, false);
team_port_list_del_port(team, port);
team_adjust_ops(team);
+ team_option_port_del(team, port);
netdev_rx_handler_unregister(port_dev);
netdev_set_master(port_dev, NULL);
vlan_vids_del_by_dev(port_dev, dev);
@@ -712,19 +869,15 @@ static int team_port_del(struct team *team, struct net_device *port_dev)
static const char team_no_mode_kind[] = "*NOMODE*";
-static int team_mode_option_get(struct team *team, void *arg)
+static int team_mode_option_get(struct team *team, struct team_gsetter_ctx *ctx)
{
- const char **str = arg;
-
- *str = team->mode ? team->mode->kind : team_no_mode_kind;
+ ctx->data.str_val = team->mode ? team->mode->kind : team_no_mode_kind;
return 0;
}
-static int team_mode_option_set(struct team *team, void *arg)
+static int team_mode_option_set(struct team *team, struct team_gsetter_ctx *ctx)
{
- const char **str = arg;
-
- return team_change_mode(team, *str);
+ return team_change_mode(team, ctx->data.str_val);
}
static const struct team_option team_options[] = {
@@ -756,6 +909,7 @@ static int team_init(struct net_device *dev)
team_adjust_ops(team);
INIT_LIST_HEAD(&team->option_list);
+ INIT_LIST_HEAD(&team->option_inst_list);
err = team_options_register(team, team_options, ARRAY_SIZE(team_options));
if (err)
goto err_options_register;
@@ -1238,7 +1392,8 @@ static int team_nl_fill_options_get(struct sk_buff *skb,
{
struct nlattr *option_list;
void *hdr;
- struct team_option *option;
+ struct team_option_inst *opt_inst;
+ int err;
hdr = genlmsg_put(skb, pid, seq, &team_nl_family, flags,
TEAM_CMD_OPTIONS_GET);
@@ -1251,50 +1406,61 @@ static int team_nl_fill_options_get(struct sk_buff *skb,
if (!option_list)
return -EMSGSIZE;
- list_for_each_entry(option, &team->option_list, list) {
+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
struct nlattr *option_item;
- long arg;
- struct team_option_binary tbinary;
+ struct team_option *option = opt_inst->option;
+ struct team_gsetter_ctx ctx;
/* Include only changed options if fill all mode is not on */
- if (!fillall && !option->changed)
+ if (!fillall && !opt_inst->changed)
continue;
option_item = nla_nest_start(skb, TEAM_ATTR_ITEM_OPTION);
if (!option_item)
goto nla_put_failure;
if (nla_put_string(skb, TEAM_ATTR_OPTION_NAME, option->name))
goto nla_put_failure;
- if (option->changed) {
+ if (opt_inst->changed) {
if (nla_put_flag(skb, TEAM_ATTR_OPTION_CHANGED))
goto nla_put_failure;
- option->changed = false;
+ opt_inst->changed = false;
}
- if (option->removed &&
+ if (opt_inst->removed &&
nla_put_flag(skb, TEAM_ATTR_OPTION_REMOVED))
goto nla_put_failure;
+ if (opt_inst->port &&
+ nla_put_u32(skb, TEAM_ATTR_OPTION_PORT_IFINDEX,
+ opt_inst->port->dev->ifindex))
+ goto nla_put_failure;
+ ctx.port = opt_inst->port;
switch (option->type) {
case TEAM_OPTION_TYPE_U32:
if (nla_put_u8(skb, TEAM_ATTR_OPTION_TYPE, NLA_U32))
goto nla_put_failure;
- team_option_get(team, option, &arg);
- if (nla_put_u32(skb, TEAM_ATTR_OPTION_DATA, arg))
+ err = team_option_get(team, opt_inst, &ctx);
+ if (err)
+ goto errout;
+ if (nla_put_u32(skb, TEAM_ATTR_OPTION_DATA,
+ ctx.data.u32_val))
goto nla_put_failure;
break;
case TEAM_OPTION_TYPE_STRING:
if (nla_put_u8(skb, TEAM_ATTR_OPTION_TYPE, NLA_STRING))
goto nla_put_failure;
- team_option_get(team, option, &arg);
+ err = team_option_get(team, opt_inst, &ctx);
+ if (err)
+ goto errout;
if (nla_put_string(skb, TEAM_ATTR_OPTION_DATA,
- (char *) arg))
+ ctx.data.str_val))
goto nla_put_failure;
break;
case TEAM_OPTION_TYPE_BINARY:
if (nla_put_u8(skb, TEAM_ATTR_OPTION_TYPE, NLA_BINARY))
goto nla_put_failure;
- arg = (long) &tbinary;
- team_option_get(team, option, &arg);
+ err = team_option_get(team, opt_inst, &ctx);
+ if (err)
+ goto errout;
if (nla_put(skb, TEAM_ATTR_OPTION_DATA,
- tbinary.data_len, tbinary.data))
+ ctx.data.bin_val.len, ctx.data.bin_val.ptr))
goto nla_put_failure;
break;
default:
@@ -1307,8 +1473,10 @@ static int team_nl_fill_options_get(struct sk_buff *skb,
return genlmsg_end(skb, hdr);
nla_put_failure:
+ err = -EMSGSIZE;
+errout:
genlmsg_cancel(skb, hdr);
- return -EMSGSIZE;
+ return err;
}
static int team_nl_fill_options_get_all(struct sk_buff *skb,
@@ -1354,9 +1522,11 @@ static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
}
nla_for_each_nested(nl_option, info->attrs[TEAM_ATTR_LIST_OPTION], i) {
- struct nlattr *mode_attrs[TEAM_ATTR_OPTION_MAX + 1];
+ struct nlattr *opt_attrs[TEAM_ATTR_OPTION_MAX + 1];
+ struct nlattr *attr_port_ifindex;
enum team_option_type opt_type;
- struct team_option *option;
+ int opt_port_ifindex = 0; /* != 0 for per-port options */
+ struct team_option_inst *opt_inst;
char *opt_name;
bool opt_found = false;
@@ -1364,17 +1534,17 @@ static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
err = -EINVAL;
goto team_put;
}
- err = nla_parse_nested(mode_attrs, TEAM_ATTR_OPTION_MAX,
+ err = nla_parse_nested(opt_attrs, TEAM_ATTR_OPTION_MAX,
nl_option, team_nl_option_policy);
if (err)
goto team_put;
- if (!mode_attrs[TEAM_ATTR_OPTION_NAME] ||
- !mode_attrs[TEAM_ATTR_OPTION_TYPE] ||
- !mode_attrs[TEAM_ATTR_OPTION_DATA]) {
+ if (!opt_attrs[TEAM_ATTR_OPTION_NAME] ||
+ !opt_attrs[TEAM_ATTR_OPTION_TYPE] ||
+ !opt_attrs[TEAM_ATTR_OPTION_DATA]) {
err = -EINVAL;
goto team_put;
}
- switch (nla_get_u8(mode_attrs[TEAM_ATTR_OPTION_TYPE])) {
+ switch (nla_get_u8(opt_attrs[TEAM_ATTR_OPTION_TYPE])) {
case NLA_U32:
opt_type = TEAM_OPTION_TYPE_U32;
break;
@@ -1388,39 +1558,47 @@ static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
goto team_put;
}
- opt_name = nla_data(mode_attrs[TEAM_ATTR_OPTION_NAME]);
- list_for_each_entry(option, &team->option_list, list) {
- long arg;
+ opt_name = nla_data(opt_attrs[TEAM_ATTR_OPTION_NAME]);
+ attr_port_ifindex = opt_attrs[TEAM_ATTR_OPTION_PORT_IFINDEX];
+ if (attr_port_ifindex)
+ opt_port_ifindex = nla_get_u32(attr_port_ifindex);
+
+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
+ struct team_option *option = opt_inst->option;
struct nlattr *opt_data_attr;
- struct team_option_binary tbinary;
+ struct team_gsetter_ctx ctx;
int data_len;
+ int tmp_ifindex;
+ tmp_ifindex = opt_inst->port ?
+ opt_inst->port->dev->ifindex : 0;
if (option->type != opt_type ||
- strcmp(option->name, opt_name))
+ strcmp(option->name, opt_name) ||
+ tmp_ifindex != opt_port_ifindex)
continue;
opt_found = true;
- opt_data_attr = mode_attrs[TEAM_ATTR_OPTION_DATA];
+ opt_data_attr = opt_attrs[TEAM_ATTR_OPTION_DATA];
data_len = nla_len(opt_data_attr);
+ ctx.port = opt_inst->port;
switch (opt_type) {
case TEAM_OPTION_TYPE_U32:
- arg = nla_get_u32(opt_data_attr);
+ ctx.data.u32_val = nla_get_u32(opt_data_attr);
break;
case TEAM_OPTION_TYPE_STRING:
if (data_len > TEAM_STRING_MAX_LEN) {
err = -EINVAL;
goto team_put;
}
- arg = (long) nla_data(opt_data_attr);
+ ctx.data.str_val = nla_data(opt_data_attr);
break;
case TEAM_OPTION_TYPE_BINARY:
- tbinary.data_len = data_len;
- tbinary.data = nla_data(opt_data_attr);
- arg = (long) &tbinary;
+ ctx.data.bin_val.len = data_len;
+ ctx.data.bin_val.ptr = nla_data(opt_data_attr);
break;
default:
BUG();
}
- err = team_option_set(team, option, &arg);
+ err = team_option_set(team, opt_inst, &ctx);
if (err)
goto team_put;
}
diff --git a/drivers/net/team/team_mode_activebackup.c b/drivers/net/team/team_mode_activebackup.c
index f4d960e..6cde1ab 100644
--- a/drivers/net/team/team_mode_activebackup.c
+++ b/drivers/net/team/team_mode_activebackup.c
@@ -59,23 +59,21 @@ static void ab_port_leave(struct team *team, struct team_port *port)
RCU_INIT_POINTER(ab_priv(team)->active_port, NULL);
}
-static int ab_active_port_get(struct team *team, void *arg)
+static int ab_active_port_get(struct team *team, struct team_gsetter_ctx *ctx)
{
- u32 *ifindex = arg;
-
- *ifindex = 0;
if (ab_priv(team)->active_port)
- *ifindex = ab_priv(team)->active_port->dev->ifindex;
+ ctx->data.u32_val = ab_priv(team)->active_port->dev->ifindex;
+ else
+ ctx->data.u32_val = 0;
return 0;
}
-static int ab_active_port_set(struct team *team, void *arg)
+static int ab_active_port_set(struct team *team, struct team_gsetter_ctx *ctx)
{
- u32 *ifindex = arg;
struct team_port *port;
list_for_each_entry_rcu(port, &team->port_list, list) {
- if (port->dev->ifindex == *ifindex) {
+ if (port->dev->ifindex == ctx->data.u32_val) {
rcu_assign_pointer(ab_priv(team)->active_port, port);
return 0;
}
diff --git a/drivers/net/team/team_mode_loadbalance.c b/drivers/net/team/team_mode_loadbalance.c
index ed20f39..167cdb4 100644
--- a/drivers/net/team/team_mode_loadbalance.c
+++ b/drivers/net/team/team_mode_loadbalance.c
@@ -52,22 +52,21 @@ drop:
return false;
}
-static int lb_bpf_func_get(struct team *team, void *arg)
+static int lb_bpf_func_get(struct team *team, struct team_gsetter_ctx *ctx)
{
- struct team_option_binary *tbinary = team_optarg_tbinary(arg);
-
- memset(tbinary, 0, sizeof(*tbinary));
- if (!lb_priv(team)->orig_fprog)
+ if (!lb_priv(team)->orig_fprog) {
+ ctx->data.bin_val.len = 0;
+ ctx->data.bin_val.ptr = NULL;
return 0;
-
- tbinary->data_len = lb_priv(team)->orig_fprog->len *
- sizeof(struct sock_filter);
- tbinary->data = lb_priv(team)->orig_fprog->filter;
+ }
+ ctx->data.bin_val.len = lb_priv(team)->orig_fprog->len *
+ sizeof(struct sock_filter);
+ ctx->data.bin_val.ptr = lb_priv(team)->orig_fprog->filter;
return 0;
}
static int __fprog_create(struct sock_fprog **pfprog, u32 data_len,
- void *data)
+ const void *data)
{
struct sock_fprog *fprog;
struct sock_filter *filter = (struct sock_filter *) data;
@@ -93,16 +92,15 @@ static void __fprog_destroy(struct sock_fprog *fprog)
kfree(fprog);
}
-static int lb_bpf_func_set(struct team *team, void *arg)
+static int lb_bpf_func_set(struct team *team, struct team_gsetter_ctx *ctx)
{
- struct team_option_binary *tbinary = team_optarg_tbinary(arg);
struct sk_filter *fp = NULL;
struct sock_fprog *fprog = NULL;
int err;
- if (tbinary->data_len) {
- err = __fprog_create(&fprog, tbinary->data_len,
- tbinary->data);
+ if (ctx->data.bin_val.len) {
+ err = __fprog_create(&fprog, ctx->data.bin_val.len,
+ ctx->data.bin_val.ptr);
if (err)
return err;
err = sk_unattached_filter_create(&fp, fprog);
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index 41163ac..6f27c84 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -71,25 +71,27 @@ enum team_option_type {
TEAM_OPTION_TYPE_BINARY,
};
+struct team_gsetter_ctx {
+ union {
+ u32 u32_val;
+ const char *str_val;
+ struct {
+ const void *ptr;
+ u32 len;
+ } bin_val;
+ } data;
+ struct team_port *port;
+};
+
struct team_option {
struct list_head list;
const char *name;
+ bool per_port;
enum team_option_type type;
- int (*getter)(struct team *team, void *arg);
- int (*setter)(struct team *team, void *arg);
-
- /* Custom gennetlink interface related flags */
- bool changed;
- bool removed;
+ int (*getter)(struct team *team, struct team_gsetter_ctx *ctx);
+ int (*setter)(struct team *team, struct team_gsetter_ctx *ctx);
};
-struct team_option_binary {
- u32 data_len;
- void *data;
-};
-
-#define team_optarg_tbinary(arg) (*((struct team_option_binary **) arg))
-
struct team_mode {
struct list_head list;
const char *kind;
@@ -118,6 +120,7 @@ struct team {
struct list_head port_list;
struct list_head option_list;
+ struct list_head option_inst_list; /* list of option instances */
const struct team_mode *mode;
struct team_mode_ops ops;
@@ -224,6 +227,7 @@ enum {
TEAM_ATTR_OPTION_TYPE, /* u8 */
TEAM_ATTR_OPTION_DATA, /* dynamic */
TEAM_ATTR_OPTION_REMOVED, /* flag */
+ TEAM_ATTR_OPTION_PORT_IFINDEX, /* u32 */ /* for per-port options */
__TEAM_ATTR_OPTION_MAX,
TEAM_ATTR_OPTION_MAX = __TEAM_ATTR_OPTION_MAX - 1,
--
1.7.9.1
^ permalink raw reply related
* [patch net-next 2/5] team: add bool option type
From: Jiri Pirko @ 2012-04-10 15:15 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet
In-Reply-To: <1334070946-7704-1-git-send-email-jpirko@redhat.com>
Add another (hopefully last) option type. Use NLA_FLAG to implement
that.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/team/team.c | 40 +++++++++++++++++++++++++++++-----------
include/linux/if_team.h | 2 ++
2 files changed, 31 insertions(+), 11 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index eaf8441..2645fae 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1463,6 +1463,16 @@ static int team_nl_fill_options_get(struct sk_buff *skb,
ctx.data.bin_val.len, ctx.data.bin_val.ptr))
goto nla_put_failure;
break;
+ case TEAM_OPTION_TYPE_BOOL:
+ if (nla_put_u8(skb, TEAM_ATTR_OPTION_TYPE, NLA_FLAG))
+ goto nla_put_failure;
+ err = team_option_get(team, opt_inst, &ctx);
+ if (err)
+ goto errout;
+ if (ctx.data.bool_val &&
+ nla_put_flag(skb, TEAM_ATTR_OPTION_DATA))
+ goto nla_put_failure;
+ break;
default:
BUG();
}
@@ -1524,6 +1534,7 @@ static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
nla_for_each_nested(nl_option, info->attrs[TEAM_ATTR_LIST_OPTION], i) {
struct nlattr *opt_attrs[TEAM_ATTR_OPTION_MAX + 1];
struct nlattr *attr_port_ifindex;
+ struct nlattr *attr_data;
enum team_option_type opt_type;
int opt_port_ifindex = 0; /* != 0 for per-port options */
struct team_option_inst *opt_inst;
@@ -1539,8 +1550,7 @@ static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
if (err)
goto team_put;
if (!opt_attrs[TEAM_ATTR_OPTION_NAME] ||
- !opt_attrs[TEAM_ATTR_OPTION_TYPE] ||
- !opt_attrs[TEAM_ATTR_OPTION_DATA]) {
+ !opt_attrs[TEAM_ATTR_OPTION_TYPE]) {
err = -EINVAL;
goto team_put;
}
@@ -1554,10 +1564,19 @@ static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
case NLA_BINARY:
opt_type = TEAM_OPTION_TYPE_BINARY;
break;
+ case NLA_FLAG:
+ opt_type = TEAM_OPTION_TYPE_BOOL;
+ break;
default:
goto team_put;
}
+ attr_data = opt_attrs[TEAM_ATTR_OPTION_DATA];
+ if (opt_type != TEAM_OPTION_TYPE_BOOL && !attr_data) {
+ err = -EINVAL;
+ goto team_put;
+ }
+
opt_name = nla_data(opt_attrs[TEAM_ATTR_OPTION_NAME]);
attr_port_ifindex = opt_attrs[TEAM_ATTR_OPTION_PORT_IFINDEX];
if (attr_port_ifindex)
@@ -1565,9 +1584,7 @@ static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
list_for_each_entry(opt_inst, &team->option_inst_list, list) {
struct team_option *option = opt_inst->option;
- struct nlattr *opt_data_attr;
struct team_gsetter_ctx ctx;
- int data_len;
int tmp_ifindex;
tmp_ifindex = opt_inst->port ?
@@ -1577,23 +1594,24 @@ static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
tmp_ifindex != opt_port_ifindex)
continue;
opt_found = true;
- opt_data_attr = opt_attrs[TEAM_ATTR_OPTION_DATA];
- data_len = nla_len(opt_data_attr);
ctx.port = opt_inst->port;
switch (opt_type) {
case TEAM_OPTION_TYPE_U32:
- ctx.data.u32_val = nla_get_u32(opt_data_attr);
+ ctx.data.u32_val = nla_get_u32(attr_data);
break;
case TEAM_OPTION_TYPE_STRING:
- if (data_len > TEAM_STRING_MAX_LEN) {
+ if (nla_len(attr_data) > TEAM_STRING_MAX_LEN) {
err = -EINVAL;
goto team_put;
}
- ctx.data.str_val = nla_data(opt_data_attr);
+ ctx.data.str_val = nla_data(attr_data);
break;
case TEAM_OPTION_TYPE_BINARY:
- ctx.data.bin_val.len = data_len;
- ctx.data.bin_val.ptr = nla_data(opt_data_attr);
+ ctx.data.bin_val.len = nla_len(attr_data);
+ ctx.data.bin_val.ptr = nla_data(attr_data);
+ break;
+ case TEAM_OPTION_TYPE_BOOL:
+ ctx.data.bool_val = attr_data ? true : false;
break;
default:
BUG();
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index 6f27c84..78c84fd 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -69,6 +69,7 @@ enum team_option_type {
TEAM_OPTION_TYPE_U32,
TEAM_OPTION_TYPE_STRING,
TEAM_OPTION_TYPE_BINARY,
+ TEAM_OPTION_TYPE_BOOL,
};
struct team_gsetter_ctx {
@@ -79,6 +80,7 @@ struct team_gsetter_ctx {
const void *ptr;
u32 len;
} bin_val;
+ bool bool_val;
} data;
struct team_port *port;
};
--
1.7.9.1
^ permalink raw reply related
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