* Re: [net-next PATCH v1 7/7] macvlan: add FDB bridge ops and new macvlan mode
From: Michael S. Tsirkin @ 2012-04-10 15:30 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: <4F84511D.7060902@intel.com>
On Tue, Apr 10, 2012 at 08:26:21AM -0700, John Fastabend wrote:
> On 4/10/2012 7:35 AM, Michael S. Tsirkin wrote:
> > 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.
> >
>
> how about MACVLAN_MODE_FDBFLAG?
The idea being that no one figures out what this means so
no one will make any wrong assumptions? ;)
--
MST
^ permalink raw reply
* Re: [net-next PATCH v1 7/7] macvlan: add FDB bridge ops and new macvlan mode
From: John Fastabend @ 2012-04-10 15:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: roprabhu, stephen.hemminger, davem, hadi, bhutchings,
jeffrey.t.kirsher, netdev, gregory.v.rose, krkumar2, sri
In-Reply-To: <20120410143306.GC19556@redhat.com>
On 4/10/2012 7:33 AM, Michael S. Tsirkin wrote:
> 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.
>
If people really _need_/_want_ this then I guess we can
add another flag. I don't think we should to tie this into
the FDB bits creating an interface with strange side effects
is probably a poor design. Much better IMHO to have an
explicit bit if and when this is needed.
.John
^ permalink raw reply
* [PATCH] iproute: show metrics as an unsigned value
From: Jorge Boncompte [DTI2] @ 2012-04-10 15:20 UTC (permalink / raw)
To: netdev; +Cc: shemminger, Jorge Boncompte [DTI2]
From: "Jorge Boncompte [DTI2]" <jorge@dti2.net>
Avoids showing negative metrics.
Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net>
---
ip/iproute.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/ip/iproute.c b/ip/iproute.c
index c97f979..2d15c01 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -404,7 +404,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
abuf, sizeof(abuf)));
}
if (tb[RTA_PRIORITY])
- fprintf(fp, " metric %d ", *(__u32*)RTA_DATA(tb[RTA_PRIORITY]));
+ fprintf(fp, " metric %u ", *(__u32*)RTA_DATA(tb[RTA_PRIORITY]));
if (r->rtm_flags & RTNH_F_DEAD)
fprintf(fp, "dead ");
if (r->rtm_flags & RTNH_F_ONLINK)
--
1.7.8.3
^ permalink raw reply related
* Re: [net-next PATCH v1 7/7] macvlan: add FDB bridge ops and new macvlan mode
From: John Fastabend @ 2012-04-10 15:26 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: roprabhu, stephen.hemminger, davem, hadi, bhutchings,
jeffrey.t.kirsher, netdev, gregory.v.rose, krkumar2, sri
In-Reply-To: <20120410143500.GD19556@redhat.com>
On 4/10/2012 7:35 AM, Michael S. Tsirkin wrote:
> 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.
>
how about MACVLAN_MODE_FDBFLAG?
^ permalink raw reply
* Re: [PATCH 05/10] net: move destructor_arg to the front of sk_buff.
From: Ian Campbell @ 2012-04-10 15:19 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev@vger.kernel.org, David Miller, Michael S. Tsirkin,
Wei Liu (Intern), xen-devel@lists.xen.org
In-Reply-To: <1334070301.5300.65.camel@edumazet-glaptop>
On Tue, 2012-04-10 at 16:05 +0100, Eric Dumazet wrote:
> 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()
I don't think there's any chance of there being a destructor_arg to
preserve in that case?
Regardless of that though I think for consistency it would be worth
pulling the common shinfo init out into a helper and using it in both
places.
I'll make that change.
Ian.
>
>
^ permalink raw reply
* [patch net-next 5/5] team: add missed "statics"
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>
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/team/team.c | 2 +-
drivers/net/team/team_mode_activebackup.c | 4 ++--
drivers/net/team/team_mode_loadbalance.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index e639abe..153a62d 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -65,7 +65,7 @@ static int __set_port_mac(struct net_device *port_dev,
return dev_set_mac_address(port_dev, &addr);
}
-int team_port_set_orig_mac(struct team_port *port)
+static int team_port_set_orig_mac(struct team_port *port)
{
return __set_port_mac(port->dev, port->orig.dev_addr);
}
diff --git a/drivers/net/team/team_mode_activebackup.c b/drivers/net/team/team_mode_activebackup.c
index a715c40..fd6bd03 100644
--- a/drivers/net/team/team_mode_activebackup.c
+++ b/drivers/net/team/team_mode_activebackup.c
@@ -90,12 +90,12 @@ static const struct team_option ab_options[] = {
},
};
-int ab_init(struct team *team)
+static int ab_init(struct team *team)
{
return team_options_register(team, ab_options, ARRAY_SIZE(ab_options));
}
-void ab_exit(struct team *team)
+static void ab_exit(struct team *team)
{
team_options_unregister(team, ab_options, ARRAY_SIZE(ab_options));
}
diff --git a/drivers/net/team/team_mode_loadbalance.c b/drivers/net/team/team_mode_loadbalance.c
index 167cdb4..2b506b2 100644
--- a/drivers/net/team/team_mode_loadbalance.c
+++ b/drivers/net/team/team_mode_loadbalance.c
@@ -130,13 +130,13 @@ static const struct team_option lb_options[] = {
},
};
-int lb_init(struct team *team)
+static int lb_init(struct team *team)
{
return team_options_register(team, lb_options,
ARRAY_SIZE(lb_options));
}
-void lb_exit(struct team *team)
+static void lb_exit(struct team *team)
{
team_options_unregister(team, lb_options,
ARRAY_SIZE(lb_options));
--
1.7.9.1
^ permalink raw reply related
* [patch net-next 4/5] team: ab: walk through port list non-rcu
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>
Since team->lock is being held, _rcu variant make no sense.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/team/team_mode_activebackup.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/team/team_mode_activebackup.c b/drivers/net/team/team_mode_activebackup.c
index 6cde1ab..a715c40 100644
--- a/drivers/net/team/team_mode_activebackup.c
+++ b/drivers/net/team/team_mode_activebackup.c
@@ -72,7 +72,7 @@ static int ab_active_port_set(struct team *team, struct team_gsetter_ctx *ctx)
{
struct team_port *port;
- list_for_each_entry_rcu(port, &team->port_list, list) {
+ list_for_each_entry(port, &team->port_list, list) {
if (port->dev->ifindex == ctx->data.u32_val) {
rcu_assign_pointer(ab_priv(team)->active_port, port);
return 0;
--
1.7.9.1
^ permalink raw reply related
* [patch net-next 3/5] team: add user_linkup and user_linkup_enabled per-port option
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>
Allows userspace to setup linkup for ports. Default is to take linkup
directly from ethtool state.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/team/team.c | 72 +++++++++++++++++++++++++++++++++++++++++------
include/linux/if_team.h | 26 ++++++++++++-----
2 files changed, 81 insertions(+), 17 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 2645fae..e639abe 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -76,6 +76,11 @@ int team_port_set_team_mac(struct team_port *port)
}
EXPORT_SYMBOL(team_port_set_team_mac);
+static void team_refresh_port_linkup(struct team_port *port)
+{
+ port->linkup = port->user.linkup_enabled ? port->user.linkup :
+ port->state.linkup;
+}
/*******************
* Options handling
@@ -880,6 +885,40 @@ static int team_mode_option_set(struct team *team, struct team_gsetter_ctx *ctx)
return team_change_mode(team, ctx->data.str_val);
}
+static int team_user_linkup_option_get(struct team *team,
+ struct team_gsetter_ctx *ctx)
+{
+ ctx->data.bool_val = ctx->port->user.linkup;
+ return 0;
+}
+
+static int team_user_linkup_option_set(struct team *team,
+ struct team_gsetter_ctx *ctx)
+{
+ ctx->port->user.linkup = ctx->data.bool_val;
+ team_refresh_port_linkup(ctx->port);
+ return 0;
+}
+
+static int team_user_linkup_en_option_get(struct team *team,
+ struct team_gsetter_ctx *ctx)
+{
+ struct team_port *port = ctx->port;
+
+ ctx->data.bool_val = port->user.linkup_enabled;
+ return 0;
+}
+
+static int team_user_linkup_en_option_set(struct team *team,
+ struct team_gsetter_ctx *ctx)
+{
+ struct team_port *port = ctx->port;
+
+ port->user.linkup_enabled = ctx->data.bool_val;
+ team_refresh_port_linkup(ctx->port);
+ return 0;
+}
+
static const struct team_option team_options[] = {
{
.name = "mode",
@@ -887,6 +926,20 @@ static const struct team_option team_options[] = {
.getter = team_mode_option_get,
.setter = team_mode_option_set,
},
+ {
+ .name = "user_linkup",
+ .type = TEAM_OPTION_TYPE_BOOL,
+ .per_port = true,
+ .getter = team_user_linkup_option_get,
+ .setter = team_user_linkup_option_set,
+ },
+ {
+ .name = "user_linkup_enabled",
+ .type = TEAM_OPTION_TYPE_BOOL,
+ .per_port = true,
+ .getter = team_user_linkup_en_option_get,
+ .setter = team_user_linkup_en_option_set,
+ },
};
static int team_init(struct net_device *dev)
@@ -1670,10 +1723,10 @@ static int team_nl_fill_port_list_get(struct sk_buff *skb,
}
if ((port->removed &&
nla_put_flag(skb, TEAM_ATTR_PORT_REMOVED)) ||
- (port->linkup &&
+ (port->state.linkup &&
nla_put_flag(skb, TEAM_ATTR_PORT_LINKUP)) ||
- nla_put_u32(skb, TEAM_ATTR_PORT_SPEED, port->speed) ||
- nla_put_u8(skb, TEAM_ATTR_PORT_DUPLEX, port->duplex))
+ nla_put_u32(skb, TEAM_ATTR_PORT_SPEED, port->state.speed) ||
+ nla_put_u8(skb, TEAM_ATTR_PORT_DUPLEX, port->state.duplex))
goto nla_put_failure;
nla_nest_end(skb, port_item);
}
@@ -1833,23 +1886,24 @@ static void __team_port_change_check(struct team_port *port, bool linkup)
{
int err;
- if (!port->removed && port->linkup == linkup)
+ if (!port->removed && port->state.linkup == linkup)
return;
port->changed = true;
- port->linkup = linkup;
+ port->state.linkup = linkup;
+ team_refresh_port_linkup(port);
if (linkup) {
struct ethtool_cmd ecmd;
err = __ethtool_get_settings(port->dev, &ecmd);
if (!err) {
- port->speed = ethtool_cmd_speed(&ecmd);
- port->duplex = ecmd.duplex;
+ port->state.speed = ethtool_cmd_speed(&ecmd);
+ port->state.duplex = ecmd.duplex;
goto send_event;
}
}
- port->speed = 0;
- port->duplex = 0;
+ port->state.speed = 0;
+ port->state.duplex = 0;
send_event:
err = team_nl_send_event_port_list_get(port->team);
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index 78c84fd..5fd5ab1 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -33,6 +33,24 @@ struct team_port {
struct team *team;
int index;
+ bool linkup; /* either state.linkup or user.linkup */
+
+ struct {
+ bool linkup;
+ u32 speed;
+ u8 duplex;
+ } state;
+
+ /* Values set by userspace */
+ struct {
+ bool linkup;
+ bool linkup_enabled;
+ } user;
+
+ /* Custom gennetlink interface related flags */
+ bool changed;
+ bool removed;
+
/*
* A place for storing original values of the device before it
* become a port.
@@ -42,14 +60,6 @@ struct team_port {
unsigned int mtu;
} orig;
- bool linkup;
- u32 speed;
- u8 duplex;
-
- /* Custom gennetlink interface related flags */
- bool changed;
- bool removed;
-
struct rcu_head rcu;
};
--
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
* [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 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
* 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
* 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 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
* 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 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 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 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 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 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: 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: [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-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
* [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
* [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
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