* [PATCH] net: netfilter: Merge assignment with return
From: Arushi Singhal @ 2018-03-28 19:09 UTC (permalink / raw)
To: pablo
Cc: outreachy-kernel, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, netfilter-devel, coreteam, netdev, linux-kernel
Merge assignment with return statement to directly return the value.
Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
---
net/netfilter/nf_conntrack_netlink.c | 5 ++---
net/netfilter/xt_hashlimit.c | 3 +--
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index dd177eb..bfa8b7f 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1497,9 +1497,8 @@ ctnetlink_setup_nat(struct nf_conn *ct, const struct nlattr * const cda[])
if (ret < 0)
return ret;
- ret = ctnetlink_parse_nat_setup(ct, NF_NAT_MANIP_SRC,
- cda[CTA_NAT_SRC]);
- return ret;
+ return ctnetlink_parse_nat_setup(ct, NF_NAT_MANIP_SRC,
+ cda[CTA_NAT_SRC]);
#else
if (!cda[CTA_NAT_DST] && !cda[CTA_NAT_SRC])
return 0;
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 66f5aca..9e8ba2e 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -534,8 +534,7 @@ static u64 user2rate_bytes(u32 user)
u64 r;
r = user ? U32_MAX / user : U32_MAX;
- r = (r - 1) << XT_HASHLIMIT_BYTE_SHIFT;
- return r;
+ return (r - 1) << XT_HASHLIMIT_BYTE_SHIFT;
}
static void rateinfo_recalc(struct dsthash_ent *dh, unsigned long now,
--
2.7.4
^ permalink raw reply related
* Re: [PATCH bpf-next]: add sock_ops R/W access to ipv4 tos
From: Daniel Borkmann @ 2018-03-28 19:09 UTC (permalink / raw)
To: Nikita Shirokov, Lawrence Brakmo, ast@kernel.org,
netdev@vger.kernel.org
Cc: Kernel Team
In-Reply-To: <DM6PR15MB23807AF16244A1173A717D85D5A30@DM6PR15MB2380.namprd15.prod.outlook.com>
On 03/28/2018 07:41 PM, Nikita Shirokov wrote:
>>> On 03/26/2018 05:36 PM, Nikita V. Shirokov wrote:
>>> bpf: Add sock_ops R/W access to ipv4 tos
>>>
>>> Sample usage for tos:
>>>
>>> bpf_getsockopt(skops, SOL_IP, IP_TOS, &v, sizeof(v))
>>>
>>> where skops is a pointer to the ctx (struct bpf_sock_ops).
>>>
>>> Signed-off-by: Nikita V. Shirokov <tehnerd@fb.com>
>>> ---
>>> net/core/filter.c | 35 +++++++++++++++++++++++++++++++++++
>>> 1 file changed, 35 insertions(+)
>>>
>>> diff --git a/net/core/filter.c b/net/core/filter.c
>>> index 00c711c..afd8255 100644
>>> --- a/net/core/filter.c
>>> +++ b/net/core/filter.c
>>> @@ -3462,6 +3462,27 @@ BPF_CALL_5(bpf_setsockopt, struct bpf_sock_ops_kern *, bpf_sock,
>>> ret = -EINVAL;
>>> }
>>> #ifdef CONFIG_INET
>>> + } else if (level == SOL_IP) {
>>> + if (optlen != sizeof(int) || sk->sk_family != AF_INET)
>>> + return -EINVAL;
>>> +
>>> + val = *((int *)optval);
>>> + /* Only some options are supported */
>>> + switch (optname) {
>>> + case IP_TOS:
>>> + if (val < -1 || val > 0xff) {
>>> + ret = -EINVAL;
>>> + } else {
>>> + struct inet_sock *inet = inet_sk(sk);
>>> +
>>> + if (val == -1)
>>> + val = 0;
>>> + inet->tos = val;
>>
>> Should this not have the exact same semantics given the helper resembles
>> the normal setsockopt? do_ip_setsockopt() does the following when setting
>> IP_TOS:
>>
>> case IP_TOS: /* This sets both TOS and Precedence */
>> if (sk->sk_type == SOCK_STREAM) {
>> val &= ~INET_ECN_MASK;
>> val |= inet->tos & INET_ECN_MASK;
>> }
>> if (inet->tos != val) {
>> inet->tos = val;
>> sk->sk_priority = rt_tos2priority(val);
>> sk_dst_reset(sk);
>> }
>> break;
>>
>> E.g. why we don't need to set sk->sk_priority as well or reset the dst
>> entry here?
>
> it feels like initially (w/ commit for IP_TOS in ip_sockglue.c) there were some usecase in mind
> where reflection of tos to prio was needed + some policy based routing (thats why dst_reset).
> but e.g. for ipv6 (IPV6_TCLASS, same as TOS but in ipv6 world) we do just this - set new tclass value
> and call it the day. in my opinion this aproach is more flexible (e.g. we have separate
> bpf_setsockopt for SOL_PRIORITY) as it did only what we want (i can imagine few usecases
> where we want just to change TOS w/o changing priority)
Ok, fair point, that way the behavior is exactly the same as in v6 case.
Applied to bpf-next, thanks Nikita!
^ permalink raw reply
* Re: [PATCH net-next 2/2] sfp/phylink: move module EEPROM ethtool access into netdev core ethtool
From: Russell King - ARM Linux @ 2018-03-28 19:11 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, Thomas Petazzoni, Andrew Lunn, David S. Miller, open list,
Antoine Tenart, Yan Markman, Stefan Chulski, Maxime Chevallier,
Miquel Raynal, Marcin Wojtas
In-Reply-To: <20180328190339.31433-3-f.fainelli@gmail.com>
On Wed, Mar 28, 2018 at 12:03:39PM -0700, Florian Fainelli wrote:
> From: Russell King <rmk+kernel@armlinux.org.uk>
>
> Provide a pointer to the SFP bus in struct net_device, so that the
> ethtool module EEPROM methods can access the SFP directly, rather
> than needing every user to provide a hook for it.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
This probably ought to have your sign-off too as you're passing the
patch along rather than me submitting it directly. DCO v1.1 (c)
seems to apply to this situation.
> ---
> drivers/net/ethernet/marvell/mvneta.c | 18 ------------------
> drivers/net/phy/phylink.c | 28 ----------------------------
> drivers/net/phy/sfp-bus.c | 6 ++----
> include/linux/netdevice.h | 3 +++
> include/linux/phylink.h | 3 ---
> net/core/ethtool.c | 7 +++++++
> 6 files changed, 12 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index cd09bde55596..25ced96750bf 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -4075,22 +4075,6 @@ static int mvneta_ethtool_set_wol(struct net_device *dev,
> return ret;
> }
>
> -static int mvneta_ethtool_get_module_info(struct net_device *dev,
> - struct ethtool_modinfo *modinfo)
> -{
> - struct mvneta_port *pp = netdev_priv(dev);
> -
> - return phylink_ethtool_get_module_info(pp->phylink, modinfo);
> -}
> -
> -static int mvneta_ethtool_get_module_eeprom(struct net_device *dev,
> - struct ethtool_eeprom *ee, u8 *buf)
> -{
> - struct mvneta_port *pp = netdev_priv(dev);
> -
> - return phylink_ethtool_get_module_eeprom(pp->phylink, ee, buf);
> -}
> -
> static int mvneta_ethtool_get_eee(struct net_device *dev,
> struct ethtool_eee *eee)
> {
> @@ -4165,8 +4149,6 @@ static const struct ethtool_ops mvneta_eth_tool_ops = {
> .set_link_ksettings = mvneta_ethtool_set_link_ksettings,
> .get_wol = mvneta_ethtool_get_wol,
> .set_wol = mvneta_ethtool_set_wol,
> - .get_module_info = mvneta_ethtool_get_module_info,
> - .get_module_eeprom = mvneta_ethtool_get_module_eeprom,
> .get_eee = mvneta_ethtool_get_eee,
> .set_eee = mvneta_ethtool_set_eee,
> };
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 9b1e4721ea3a..c582b2d7546c 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -1250,34 +1250,6 @@ int phylink_ethtool_set_pauseparam(struct phylink *pl,
> }
> EXPORT_SYMBOL_GPL(phylink_ethtool_set_pauseparam);
>
> -int phylink_ethtool_get_module_info(struct phylink *pl,
> - struct ethtool_modinfo *modinfo)
> -{
> - int ret = -EOPNOTSUPP;
> -
> - WARN_ON(!lockdep_rtnl_is_held());
> -
> - if (pl->sfp_bus)
> - ret = sfp_get_module_info(pl->sfp_bus, modinfo);
> -
> - return ret;
> -}
> -EXPORT_SYMBOL_GPL(phylink_ethtool_get_module_info);
> -
> -int phylink_ethtool_get_module_eeprom(struct phylink *pl,
> - struct ethtool_eeprom *ee, u8 *buf)
> -{
> - int ret = -EOPNOTSUPP;
> -
> - WARN_ON(!lockdep_rtnl_is_held());
> -
> - if (pl->sfp_bus)
> - ret = sfp_get_module_eeprom(pl->sfp_bus, ee, buf);
> -
> - return ret;
> -}
> -EXPORT_SYMBOL_GPL(phylink_ethtool_get_module_eeprom);
> -
> /**
> * phylink_ethtool_get_eee_err() - read the energy efficient ethernet error
> * counter
> diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
> index 3d4ff5d0d2a6..0381da78d228 100644
> --- a/drivers/net/phy/sfp-bus.c
> +++ b/drivers/net/phy/sfp-bus.c
> @@ -342,6 +342,7 @@ static int sfp_register_bus(struct sfp_bus *bus)
> }
> if (bus->started)
> bus->socket_ops->start(bus->sfp);
> + bus->netdev->sfp_bus = bus;
> bus->registered = true;
> return 0;
> }
> @@ -356,6 +357,7 @@ static void sfp_unregister_bus(struct sfp_bus *bus)
> if (bus->phydev && ops && ops->disconnect_phy)
> ops->disconnect_phy(bus->upstream);
> }
> + bus->netdev->sfp_bus = NULL;
> bus->registered = false;
> }
>
> @@ -371,8 +373,6 @@ static void sfp_unregister_bus(struct sfp_bus *bus)
> */
> int sfp_get_module_info(struct sfp_bus *bus, struct ethtool_modinfo *modinfo)
> {
> - if (!bus->registered)
> - return -ENOIOCTLCMD;
> return bus->socket_ops->module_info(bus->sfp, modinfo);
> }
> EXPORT_SYMBOL_GPL(sfp_get_module_info);
> @@ -391,8 +391,6 @@ EXPORT_SYMBOL_GPL(sfp_get_module_info);
> int sfp_get_module_eeprom(struct sfp_bus *bus, struct ethtool_eeprom *ee,
> u8 *data)
> {
> - if (!bus->registered)
> - return -ENOIOCTLCMD;
> return bus->socket_ops->module_eeprom(bus->sfp, ee, data);
> }
> EXPORT_SYMBOL_GPL(sfp_get_module_eeprom);
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 2a2d9cf50aa2..53f0cd64676b 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -58,6 +58,7 @@ struct device;
> struct phy_device;
> struct dsa_port;
>
> +struct sfp_bus;
> /* 802.11 specific */
> struct wireless_dev;
> /* 802.15.4 specific */
> @@ -1662,6 +1663,7 @@ enum netdev_priv_flags {
> * @priomap: XXX: need comments on this one
> * @phydev: Physical device may attach itself
> * for hardware timestamping
> + * @sfp_bus: attached &struct sfp_bus structure.
> *
> * @qdisc_tx_busylock: lockdep class annotating Qdisc->busylock spinlock
> * @qdisc_running_key: lockdep class annotating Qdisc->running seqcount
> @@ -1945,6 +1947,7 @@ struct net_device {
> struct netprio_map __rcu *priomap;
> #endif
> struct phy_device *phydev;
> + struct sfp_bus *sfp_bus;
> struct lock_class_key *qdisc_tx_busylock;
> struct lock_class_key *qdisc_running_key;
> bool proto_down;
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index f29a40947de9..67cd518d66d1 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -217,9 +217,6 @@ void phylink_ethtool_get_pauseparam(struct phylink *,
> struct ethtool_pauseparam *);
> int phylink_ethtool_set_pauseparam(struct phylink *,
> struct ethtool_pauseparam *);
> -int phylink_ethtool_get_module_info(struct phylink *, struct ethtool_modinfo *);
> -int phylink_ethtool_get_module_eeprom(struct phylink *,
> - struct ethtool_eeprom *, u8 *);
> int phylink_get_eee_err(struct phylink *);
> int phylink_ethtool_get_eee(struct phylink *, struct ethtool_eee *);
> int phylink_ethtool_set_eee(struct phylink *, struct ethtool_eee *);
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index bb6e498c6e3d..eb55252ca1fb 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -22,6 +22,7 @@
> #include <linux/bitops.h>
> #include <linux/uaccess.h>
> #include <linux/vmalloc.h>
> +#include <linux/sfp.h>
> #include <linux/slab.h>
> #include <linux/rtnetlink.h>
> #include <linux/sched/signal.h>
> @@ -2245,6 +2246,9 @@ static int __ethtool_get_module_info(struct net_device *dev,
> const struct ethtool_ops *ops = dev->ethtool_ops;
> struct phy_device *phydev = dev->phydev;
>
> + if (dev->sfp_bus)
> + return sfp_get_module_info(dev->sfp_bus, modinfo);
> +
> if (phydev && phydev->drv && phydev->drv->module_info)
> return phydev->drv->module_info(phydev, modinfo);
>
> @@ -2279,6 +2283,9 @@ static int __ethtool_get_module_eeprom(struct net_device *dev,
> const struct ethtool_ops *ops = dev->ethtool_ops;
> struct phy_device *phydev = dev->phydev;
>
> + if (dev->sfp_bus)
> + return sfp_get_module_eeprom(dev->sfp_bus, ee, data);
> +
> if (phydev && phydev->drv && phydev->drv->module_eeprom)
> return phydev->drv->module_eeprom(phydev, ee, data);
>
> --
> 2.14.1
>
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* Re: [PATCH net-next 1/2] ipv6: do not set routes if disable_ipv6 has been enabled
From: David Ahern @ 2018-03-28 19:12 UTC (permalink / raw)
To: Lorenzo Bianconi, davem; +Cc: netdev
In-Reply-To: <3635f139e31e1c28803ed6d76743c8d8da2c2d3d.1522166051.git.lorenzo.bianconi@redhat.com>
On 3/27/18 11:11 AM, Lorenzo Bianconi wrote:
> Do not allow to set ipv6 routes from userspace if disable_ipv6 has been
> enabled. The issue can be triggered using the following reproducer:
>
> - sysctl net.ipv6.conf.all.disable_ipv6=1
> - ip -6 route add a:b:c:d::/64 dev em1
> - ip -6 route show
> a:b:c:d::/64 dev em1 metric 1024 pref medium
>
> Fix it checking disable_ipv6 value in ip6_route_info_create routine
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
> net/ipv6/route.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 1d0eaa69874d..672fd7fdb037 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -2917,6 +2917,11 @@ static struct rt6_info *ip6_route_info_create(struct fib6_config *cfg,
> if (!dev)
> goto out;
>
> + if (idev->cnf.disable_ipv6) {
> + err = -EACCES;
you need an extack message telling the user that IPv6 is disabled on the
nexthop device.
> + goto out;
> + }
> +
> if (!(dev->flags & IFF_UP)) {
> NL_SET_ERR_MSG(extack, "Nexthop device is not up");
> err = -ENETDOWN;
>
^ permalink raw reply
* Re: [PATCH net-next 2/2] sfp/phylink: move module EEPROM ethtool access into netdev core ethtool
From: Florian Fainelli @ 2018-03-28 19:12 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: netdev, Thomas Petazzoni, Andrew Lunn, David S. Miller, open list,
Antoine Tenart, Yan Markman, Stefan Chulski, Maxime Chevallier,
Miquel Raynal, Marcin Wojtas
In-Reply-To: <20180328191151.GS10980@n2100.armlinux.org.uk>
On 03/28/2018 12:11 PM, Russell King - ARM Linux wrote:
> On Wed, Mar 28, 2018 at 12:03:39PM -0700, Florian Fainelli wrote:
>> From: Russell King <rmk+kernel@armlinux.org.uk>
>>
>> Provide a pointer to the SFP bus in struct net_device, so that the
>> ethtool module EEPROM methods can access the SFP directly, rather
>> than needing every user to provide a hook for it.
>>
>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>
> This probably ought to have your sign-off too as you're passing the
> patch along rather than me submitting it directly. DCO v1.1 (c)
> seems to apply to this situation.
Indeed:
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v7 bpf-next 06/10] tracepoint: compute num_args at build time
From: Mathieu Desnoyers @ 2018-03-28 19:22 UTC (permalink / raw)
To: rostedt
Cc: Alexei Starovoitov, David S. Miller, Daniel Borkmann,
Linus Torvalds, Peter Zijlstra, netdev, kernel-team, linux-api,
Josh Poimboeuf
In-Reply-To: <20180328145431.687643bc@gandalf.local.home>
----- On Mar 28, 2018, at 2:54 PM, rostedt rostedt@goodmis.org wrote:
> On Wed, 28 Mar 2018 11:19:34 -0700
> Alexei Starovoitov <ast@fb.com> wrote:
>
>> On 3/28/18 11:10 AM, Steven Rostedt wrote:
>> > On Wed, 28 Mar 2018 11:03:24 -0700
>> > Alexei Starovoitov <ast@fb.com> wrote:
>> >
>> >> I can live with this overhead if Mathieu insists,
>> >> but I prefer to keep it in 'struct tracepoint'.
>> >>
>> >> Thoughts?
>> >
>> > I'm fine with keeping it as is. We could probably use it for future
>> > enhancements in perf and ftrace.
>> >
>> > Perhaps, we should just add a:
>> >
>> > #ifdef CONFIG_BPF_EVENTS
>> >
>> > Around the use cases of num_args.
>>
>> it sounds like a good idea, but implementation wise
>> it will be ifdef CONFIG_BPF_EVENTS around u32 num_args;
>> in struct tracepoint and similar double definition of
>> DEFINE_TRACE_FN. One that uses num_args to init
>> struct tracepoint and one that doesn't ?
>> Feels like serious uglification of already macros heavy code.
>> Also what it will address?
>
> 32bit bloat ;-)
>
> But I agree, it's not worth uglifying it.
>
> -- Steve
>
> > cache hot/cold argument clearly doesn't apply.
In the current situation I'm fine with adding this extra field
to struct tracepoint. However, we should keep in mind to move
all non-required cache-cold fields to a separate section at
some point. Clearly just this single field won't make a difference
due to other fields and padding.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v7 bpf-next 06/10] tracepoint: compute num_args at build time
From: Alexei Starovoitov @ 2018-03-28 19:25 UTC (permalink / raw)
To: Mathieu Desnoyers, rostedt
Cc: David S. Miller, Daniel Borkmann, Linus Torvalds, Peter Zijlstra,
netdev, kernel-team, linux-api, Josh Poimboeuf
In-Reply-To: <842190155.225.1522264944386.JavaMail.zimbra@efficios.com>
On 3/28/18 12:22 PM, Mathieu Desnoyers wrote:
>
> ----- On Mar 28, 2018, at 2:54 PM, rostedt rostedt@goodmis.org wrote:
>
>> On Wed, 28 Mar 2018 11:19:34 -0700
>> Alexei Starovoitov <ast@fb.com> wrote:
>>
>>> On 3/28/18 11:10 AM, Steven Rostedt wrote:
>>>> On Wed, 28 Mar 2018 11:03:24 -0700
>>>> Alexei Starovoitov <ast@fb.com> wrote:
>>>>
>>>>> I can live with this overhead if Mathieu insists,
>>>>> but I prefer to keep it in 'struct tracepoint'.
>>>>>
>>>>> Thoughts?
>>>>
>>>> I'm fine with keeping it as is. We could probably use it for future
>>>> enhancements in perf and ftrace.
>>>>
>>>> Perhaps, we should just add a:
>>>>
>>>> #ifdef CONFIG_BPF_EVENTS
>>>>
>>>> Around the use cases of num_args.
>>>
>>> it sounds like a good idea, but implementation wise
>>> it will be ifdef CONFIG_BPF_EVENTS around u32 num_args;
>>> in struct tracepoint and similar double definition of
>>> DEFINE_TRACE_FN. One that uses num_args to init
>>> struct tracepoint and one that doesn't ?
>>> Feels like serious uglification of already macros heavy code.
>>> Also what it will address?
>>
>> 32bit bloat ;-)
>>
>> But I agree, it's not worth uglifying it.
>>
>> -- Steve
>>
>>> cache hot/cold argument clearly doesn't apply.
>
> In the current situation I'm fine with adding this extra field
> to struct tracepoint. However, we should keep in mind to move
> all non-required cache-cold fields to a separate section at
> some point. Clearly just this single field won't make a difference
> due to other fields and padding.
Submitted v8 where num_args is moved to bpf side.
Please ack it.
^ permalink raw reply
* Re: [PATCH v7 bpf-next 06/10] tracepoint: compute num_args at build time
From: Steven Rostedt @ 2018-03-28 19:32 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Alexei Starovoitov, David S. Miller, Daniel Borkmann,
Linus Torvalds, Peter Zijlstra, netdev, kernel-team, linux-api,
Josh Poimboeuf
In-Reply-To: <842190155.225.1522264944386.JavaMail.zimbra@efficios.com>
On Wed, 28 Mar 2018 15:22:24 -0400 (EDT)
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> > > cache hot/cold argument clearly doesn't apply.
>
> In the current situation I'm fine with adding this extra field
> to struct tracepoint. However, we should keep in mind to move
> all non-required cache-cold fields to a separate section at
> some point. Clearly just this single field won't make a difference
> due to other fields and padding.
funcs is the only part of the tracepoint structure that needs hot
cache. Thus, instead of trying to keep the tracepoint structure small
for cache reasons, pull funcs out of the tracepoint structure.
What about this patch?
Of course, this just adds another 8 bytes per tracepoint :-/ But it
keeps the functions away from the tracepoint structures. We could even
add a section for them to be together, but I'm not sure that will help
much more that just letting the linker place them, as these function
pointers of the same system will probably be grouped together.
-- Steve
diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index 35db8dd48c4c..8d100163e9af 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -32,7 +32,7 @@ struct tracepoint {
struct static_key key;
int (*regfunc)(void);
void (*unregfunc)(void);
- struct tracepoint_func __rcu *funcs;
+ struct tracepoint_func **funcs;
u32 num_args;
};
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index c92f4adbc0d7..b55282202f71 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -129,7 +129,7 @@ extern void syscall_unregfunc(void);
* as "(void *, void)". The DECLARE_TRACE_NOARGS() will pass in just
* "void *data", where as the DECLARE_TRACE() will pass in "void *data, proto".
*/
-#define __DO_TRACE(tp, proto, args, cond, rcucheck) \
+#define __DO_TRACE(name, proto, args, cond, rcucheck) \
do { \
struct tracepoint_func *it_func_ptr; \
void *it_func; \
@@ -140,7 +140,7 @@ extern void syscall_unregfunc(void);
if (rcucheck) \
rcu_irq_enter_irqson(); \
rcu_read_lock_sched_notrace(); \
- it_func_ptr = rcu_dereference_sched((tp)->funcs); \
+ it_func_ptr = rcu_dereference_sched(__trace_##name##_funcs); \
if (it_func_ptr) { \
do { \
it_func = (it_func_ptr)->func; \
@@ -158,7 +158,7 @@ extern void syscall_unregfunc(void);
static inline void trace_##name##_rcuidle(proto) \
{ \
if (static_key_false(&__tracepoint_##name.key)) \
- __DO_TRACE(&__tracepoint_##name, \
+ __DO_TRACE(name, \
TP_PROTO(data_proto), \
TP_ARGS(data_args), \
TP_CONDITION(cond), 1); \
@@ -181,10 +181,11 @@ extern void syscall_unregfunc(void);
*/
#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
extern struct tracepoint __tracepoint_##name; \
+ extern struct tracepoint_func __rcu *__trace_##name##_funcs; \
static inline void trace_##name(proto) \
{ \
if (static_key_false(&__tracepoint_##name.key)) \
- __DO_TRACE(&__tracepoint_##name, \
+ __DO_TRACE(name, \
TP_PROTO(data_proto), \
TP_ARGS(data_args), \
TP_CONDITION(cond), 0); \
@@ -233,9 +234,11 @@ extern void syscall_unregfunc(void);
#define DEFINE_TRACE_FN(name, reg, unreg, num_args) \
static const char __tpstrtab_##name[] \
__attribute__((section("__tracepoints_strings"))) = #name; \
+ struct tracepoint_func __rcu *__trace_##name##_funcs; \
struct tracepoint __tracepoint_##name \
__attribute__((section("__tracepoints"))) = \
- { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL, num_args };\
+ { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, \
+ &__trace_##name##_funcs, num_args }; \
static struct tracepoint * const __tracepoint_ptr_##name __used \
__attribute__((section("__tracepoints_ptrs"))) = \
&__tracepoint_##name;
@@ -244,9 +247,12 @@ extern void syscall_unregfunc(void);
DEFINE_TRACE_FN(name, NULL, NULL, num_args);
#define EXPORT_TRACEPOINT_SYMBOL_GPL(name) \
- EXPORT_SYMBOL_GPL(__tracepoint_##name)
+ EXPORT_SYMBOL_GPL(__tracepoint_##name); \
+ EXPORT_SYMBOL_GPL(__trace_##name##_funcs)
+
#define EXPORT_TRACEPOINT_SYMBOL(name) \
- EXPORT_SYMBOL(__tracepoint_##name)
+ EXPORT_SYMBOL(__tracepoint_##name); \
+ EXPORT_SYMBOL(__trace_##name##_funcs)
#else /* !TRACEPOINTS_ENABLED */
#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index a02bc09d765a..47b884809f22 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -809,6 +809,7 @@ static void free_synth_tracepoint(struct tracepoint *tp)
if (!tp)
return;
+ kfree(tp->funcs);
kfree(tp->name);
kfree(tp);
}
@@ -827,6 +828,13 @@ static struct tracepoint *alloc_synth_tracepoint(char *name)
return ERR_PTR(-ENOMEM);
}
+ tp->funcs = kzalloc(sizeof(*tp->funcs), GFP_KERNEL);
+ if (!tp->funcs) {
+ kfree(tp->name);
+ kfree(tp);
+ return ERR_PTR(-ENOMEM);
+ }
+
return tp;
}
@@ -846,7 +854,7 @@ static inline void trace_synth(struct synth_event *event, u64 *var_ref_vals,
if (!(cpu_online(raw_smp_processor_id())))
return;
- probe_func_ptr = rcu_dereference_sched((tp)->funcs);
+ probe_func_ptr = rcu_dereference_sched(*(tp)->funcs);
if (probe_func_ptr) {
do {
probe_func = probe_func_ptr->func;
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 671b13457387..638a35f77841 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -203,7 +203,7 @@ static int tracepoint_add_func(struct tracepoint *tp,
return ret;
}
- tp_funcs = rcu_dereference_protected(tp->funcs,
+ tp_funcs = rcu_dereference_protected(*tp->funcs,
lockdep_is_held(&tracepoints_mutex));
old = func_add(&tp_funcs, func, prio);
if (IS_ERR(old)) {
@@ -217,7 +217,7 @@ static int tracepoint_add_func(struct tracepoint *tp,
* a pointer to it. This array is referenced by __DO_TRACE from
* include/linux/tracepoint.h using rcu_dereference_sched().
*/
- rcu_assign_pointer(tp->funcs, tp_funcs);
+ rcu_assign_pointer(*tp->funcs, tp_funcs);
if (!static_key_enabled(&tp->key))
static_key_slow_inc(&tp->key);
release_probes(old);
@@ -235,7 +235,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
{
struct tracepoint_func *old, *tp_funcs;
- tp_funcs = rcu_dereference_protected(tp->funcs,
+ tp_funcs = rcu_dereference_protected(*tp->funcs,
lockdep_is_held(&tracepoints_mutex));
old = func_remove(&tp_funcs, func);
if (IS_ERR(old)) {
@@ -251,7 +251,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
if (static_key_enabled(&tp->key))
static_key_slow_dec(&tp->key);
}
- rcu_assign_pointer(tp->funcs, tp_funcs);
+ rcu_assign_pointer(*tp->funcs, tp_funcs);
release_probes(old);
return 0;
}
@@ -398,7 +398,7 @@ static void tp_module_going_check_quiescent(struct tracepoint * const *begin,
if (!begin)
return;
for (iter = begin; iter < end; iter++)
- WARN_ON_ONCE((*iter)->funcs);
+ WARN_ON_ONCE(*(*iter)->funcs);
}
static int tracepoint_module_coming(struct module *mod)
^ permalink raw reply related
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics
From: Andrew Lunn @ 2018-03-28 19:33 UTC (permalink / raw)
To: Florian Fainelli; +Cc: David Miller, netdev
In-Reply-To: <70d16316-df99-0664-a574-18edaa3ec313@gmail.com>
On Wed, Mar 28, 2018 at 11:17:19AM -0700, Florian Fainelli wrote:
> On 03/27/2018 02:59 PM, Andrew Lunn wrote:
> > Count the numbers of various ATU and VTU violation statistics and
> > return them as part of the ethtool -S statistics.
> >
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> > drivers/net/dsa/mv88e6xxx/chip.c | 50 ++++++++++++++++++++++++++++-----
> > drivers/net/dsa/mv88e6xxx/chip.h | 13 ++++++---
> > drivers/net/dsa/mv88e6xxx/global1_atu.c | 12 +++++---
> > drivers/net/dsa/mv88e6xxx/global1_vtu.c | 8 ++++--
> > drivers/net/dsa/mv88e6xxx/serdes.c | 15 ++++++----
> > drivers/net/dsa/mv88e6xxx/serdes.h | 8 +++---
> > 6 files changed, 78 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> > index 9a5d786b4885..186021f98c5d 100644
> > --- a/drivers/net/dsa/mv88e6xxx/chip.c
> > +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> > @@ -723,6 +723,24 @@ static int mv88e6320_stats_get_strings(struct mv88e6xxx_chip *chip,
> > STATS_TYPE_BANK0 | STATS_TYPE_BANK1);
> > }
> >
> > +static const uint8_t *mv88e6xxx_atu_vtu_stats_strings[] = {
>
> Why not const char *?
The ethtool call passes i uint8_t *data to receive the copy into. I'm
keeping it consistent.
> > +static void mv88e6xxx_atu_vtu_get_strings(uint8_t *data)
> > +{
> > + int i;
>
> unsigned int i?
I could do, but it seems unlikely it will overflow 31 bits.
> > +
> > + for (i = 0; i < ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); i++)
> > + strlcpy(data + i * ETH_GSTRING_LEN,
> > + mv88e6xxx_atu_vtu_stats_strings[i],
> > + ETH_GSTRING_LEN);
> > +}
> > +
> > static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
> > uint8_t *data)
> > {
> > @@ -736,9 +754,12 @@ static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
> >
> > if (chip->info->ops->serdes_get_strings) {
> > data += count * ETH_GSTRING_LEN;
> > - chip->info->ops->serdes_get_strings(chip, port, data);
> > + count = chip->info->ops->serdes_get_strings(chip, port, data);
> > }
> >
> > + data += count * ETH_GSTRING_LEN;
> > + mv88e6xxx_atu_vtu_get_strings(data);
> > +
> > mutex_unlock(&chip->reg_lock);
> > }
> >
> > @@ -783,10 +804,13 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
> > if (chip->info->ops->serdes_get_sset_count)
> > serdes_count = chip->info->ops->serdes_get_sset_count(chip,
> > port);
> > - if (serdes_count < 0)
> > + if (serdes_count < 0) {
> > count = serdes_count;
> > - else
> > - count += serdes_count;
> > + goto out;
> > + }
> > + count += serdes_count;
> > + count += ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings);
> > +
> > out:
> > mutex_unlock(&chip->reg_lock);
> >
> > @@ -841,6 +865,16 @@ static int mv88e6390_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
> > 0);
> > }
> >
> > +static void mv88e6xxx_atu_vtu_get_stats(struct mv88e6xxx_chip *chip, int port,
> > + uint64_t *data)
> > +{
> > + *data++ = chip->ports[port].atu_member_violation;
> > + *data++ = chip->ports[port].atu_miss_violation;
> > + *data++ = chip->ports[port].atu_full_violation;
> > + *data++ = chip->ports[port].vtu_member_violation;
> > + *data++ = chip->ports[port].vtu_miss_violation;
>
> This looks fine, but I suppose you could just have an u64 pointer which
> is initialized to point to atu_member_violation, and then just do
> pointer arithmetics to iterate, this would avoid possibly missing that
> function in case new ATU/VTU violations are handled in the future?
KISS. This works and is obvious.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: Make VTU miss violations less spammy
From: Andrew Lunn @ 2018-03-28 19:34 UTC (permalink / raw)
To: Florian Fainelli; +Cc: David Miller, netdev
In-Reply-To: <f9252f0b-c78c-f377-e52f-896dca5e5a07@gmail.com>
On Wed, Mar 28, 2018 at 11:11:07AM -0700, Florian Fainelli wrote:
> On 03/27/2018 02:59 PM, Andrew Lunn wrote:
> > VTU miss violations can happen under normal conditions. Don't spam the
> > kernel log. The statistics counter will indicate it is happening, if
> > anybody is interested.
> >
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
>
> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
>
> > ---
> > drivers/net/dsa/mv88e6xxx/global1_vtu.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
> > index 2cbaf946e7ed..e0f1b4f6e29f 100644
> > --- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c
> > +++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
> > @@ -547,11 +547,9 @@ static irqreturn_t mv88e6xxx_g1_vtu_prob_irq_thread_fn(int irq, void *dev_id)
> > chip->ports[spid].vtu_member_violation++;
> > }
> >
> > - if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) {
> > - dev_err_ratelimited(chip->dev, "VTU miss violation for vid %d, source port %d\n",
> > - entry.vid, spid);
>
> Why not keep it as a dev_dbg() message?
O.K, will do.
Andrew
^ permalink raw reply
* Re: [PATCH v8 bpf-next 6/9] bpf: introduce BPF_RAW_TRACEPOINT
From: Steven Rostedt @ 2018-03-28 19:34 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, mathieu.desnoyers, netdev,
kernel-team, linux-api
In-Reply-To: <20180328190540.370956-7-ast@kernel.org>
On Wed, 28 Mar 2018 12:05:37 -0700
Alexei Starovoitov <ast@kernel.org> wrote:
> +++ b/include/linux/tracepoint-defs.h
> @@ -35,4 +35,10 @@ struct tracepoint {
> struct tracepoint_func __rcu *funcs;
> };
>
> +struct bpf_raw_event_map {
> + struct tracepoint *tp;
> + void *bpf_func;
> + u32 num_args;
> +} __aligned(32);
> +
If you prefer v7, I'm fine with that. For cache issues, I can pull out
the funcs from the tracepoint structure like I posted.
Mathieu, your thoughts?
-- Steve
^ permalink raw reply
* Re: [PATCH v7 bpf-next 06/10] tracepoint: compute num_args at build time
From: Steven Rostedt @ 2018-03-28 19:38 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Alexei Starovoitov, David S. Miller, Daniel Borkmann,
Linus Torvalds, Peter Zijlstra, netdev, kernel-team, linux-api,
Josh Poimboeuf
In-Reply-To: <20180328153220.06a132f4@gandalf.local.home>
On Wed, 28 Mar 2018 15:32:20 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> -#define __DO_TRACE(tp, proto, args, cond, rcucheck) \
> +#define __DO_TRACE(name, proto, args, cond, rcucheck) \
> do { \
> struct tracepoint_func *it_func_ptr; \
> void *it_func; \
> @@ -140,7 +140,7 @@ extern void syscall_unregfunc(void);
> if (rcucheck) \
> rcu_irq_enter_irqson(); \
> rcu_read_lock_sched_notrace(); \
> - it_func_ptr = rcu_dereference_sched((tp)->funcs); \
> + it_func_ptr = rcu_dereference_sched(__trace_##name##_funcs); \
What we lose in data size, we may make up for in text (which is even
more important). This will remove a dereference in the hot path.
I'll make a few builds and run size on the vmlinux images to see how
this pans out.
-- Steve
> if (it_func_ptr) { \
> do { \
^ permalink raw reply
* Re: [PATCH v8 bpf-next 6/9] bpf: introduce BPF_RAW_TRACEPOINT
From: Alexei Starovoitov @ 2018-03-28 19:38 UTC (permalink / raw)
To: Steven Rostedt, Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, mathieu.desnoyers, netdev,
kernel-team, linux-api
In-Reply-To: <20180328153435.5e733957@gandalf.local.home>
On 3/28/18 12:34 PM, Steven Rostedt wrote:
> On Wed, 28 Mar 2018 12:05:37 -0700
> Alexei Starovoitov <ast@kernel.org> wrote:
>
>> +++ b/include/linux/tracepoint-defs.h
>> @@ -35,4 +35,10 @@ struct tracepoint {
>> struct tracepoint_func __rcu *funcs;
>> };
>>
>> +struct bpf_raw_event_map {
>> + struct tracepoint *tp;
>> + void *bpf_func;
>> + u32 num_args;
>> +} __aligned(32);
>> +
>
> If you prefer v7, I'm fine with that. For cache issues, I can pull out
> the funcs from the tracepoint structure like I posted.
I very much prefer to land this v8 as-is and optimize later.
I still have bpfilter/microkernel patches to finish which were
practically ready two weeks ago and got delayed but this set.
^ permalink raw reply
* Re: [PATCH v8 bpf-next 6/9] bpf: introduce BPF_RAW_TRACEPOINT
From: Steven Rostedt @ 2018-03-28 19:40 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, davem, daniel, torvalds, peterz,
mathieu.desnoyers, netdev, kernel-team, linux-api
In-Reply-To: <2a0f6ac2-b13e-b8ad-31b4-a42909a34efa@fb.com>
On Wed, 28 Mar 2018 12:38:48 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> On 3/28/18 12:34 PM, Steven Rostedt wrote:
> > On Wed, 28 Mar 2018 12:05:37 -0700
> > Alexei Starovoitov <ast@kernel.org> wrote:
> >
> >> +++ b/include/linux/tracepoint-defs.h
> >> @@ -35,4 +35,10 @@ struct tracepoint {
> >> struct tracepoint_func __rcu *funcs;
> >> };
> >>
> >> +struct bpf_raw_event_map {
> >> + struct tracepoint *tp;
> >> + void *bpf_func;
> >> + u32 num_args;
> >> +} __aligned(32);
> >> +
> >
> > If you prefer v7, I'm fine with that. For cache issues, I can pull out
> > the funcs from the tracepoint structure like I posted.
>
> I very much prefer to land this v8 as-is and optimize later.
>
> I still have bpfilter/microkernel patches to finish which were
> practically ready two weeks ago and got delayed but this set.
Then by all means, you have my Ack. We can optimize later.
For the whole series... (v7 or v8)
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Thanks!
-- Steve
^ permalink raw reply
* Re: [PATCH v7 bpf-next 06/10] tracepoint: compute num_args at build time
From: Mathieu Desnoyers @ 2018-03-28 19:47 UTC (permalink / raw)
To: rostedt
Cc: Alexei Starovoitov, David S. Miller, Daniel Borkmann,
Linus Torvalds, Peter Zijlstra, netdev, kernel-team, linux-api,
Josh Poimboeuf
In-Reply-To: <20180328153819.654c1600@gandalf.local.home>
----- On Mar 28, 2018, at 3:38 PM, rostedt rostedt@goodmis.org wrote:
> On Wed, 28 Mar 2018 15:32:20 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
>> -#define __DO_TRACE(tp, proto, args, cond, rcucheck) \
>> +#define __DO_TRACE(name, proto, args, cond, rcucheck) \
>> do { \
>> struct tracepoint_func *it_func_ptr; \
>> void *it_func; \
>> @@ -140,7 +140,7 @@ extern void syscall_unregfunc(void);
>> if (rcucheck) \
>> rcu_irq_enter_irqson(); \
>> rcu_read_lock_sched_notrace(); \
>> - it_func_ptr = rcu_dereference_sched((tp)->funcs); \
>> + it_func_ptr = rcu_dereference_sched(__trace_##name##_funcs); \
>
> What we lose in data size, we may make up for in text (which is even
> more important). This will remove a dereference in the hot path.
>
> I'll make a few builds and run size on the vmlinux images to see how
> this pans out.
I like the approach. If it passes testing, I think it's a valuable improvement
to lessen cache footprint of tracepoint when tracing is active.
Thanks!
Mathieu
>
> -- Steve
>
>
>> if (it_func_ptr) { \
> > do { \
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v8 bpf-next 6/9] bpf: introduce BPF_RAW_TRACEPOINT
From: Mathieu Desnoyers @ 2018-03-28 19:48 UTC (permalink / raw)
To: rostedt
Cc: Alexei Starovoitov, Alexei Starovoitov, David S. Miller,
Daniel Borkmann, Linus Torvalds, Peter Zijlstra, netdev,
kernel-team, linux-api
In-Reply-To: <20180328154027.569206ba@gandalf.local.home>
----- On Mar 28, 2018, at 3:40 PM, rostedt rostedt@goodmis.org wrote:
> On Wed, 28 Mar 2018 12:38:48 -0700
> Alexei Starovoitov <ast@fb.com> wrote:
>
>> On 3/28/18 12:34 PM, Steven Rostedt wrote:
>> > On Wed, 28 Mar 2018 12:05:37 -0700
>> > Alexei Starovoitov <ast@kernel.org> wrote:
>> >
>> >> +++ b/include/linux/tracepoint-defs.h
>> >> @@ -35,4 +35,10 @@ struct tracepoint {
>> >> struct tracepoint_func __rcu *funcs;
>> >> };
>> >>
>> >> +struct bpf_raw_event_map {
>> >> + struct tracepoint *tp;
>> >> + void *bpf_func;
>> >> + u32 num_args;
>> >> +} __aligned(32);
>> >> +
>> >
>> > If you prefer v7, I'm fine with that. For cache issues, I can pull out
>> > the funcs from the tracepoint structure like I posted.
>>
>> I very much prefer to land this v8 as-is and optimize later.
>>
>> I still have bpfilter/microkernel patches to finish which were
>> practically ready two weeks ago and got delayed but this set.
>
> Then by all means, you have my Ack. We can optimize later.
>
> For the whole series... (v7 or v8)
>
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
I'm fine with either v7 or v8 as well.
Thanks,
Mathieu
>
> Thanks!
>
> -- Steve
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* [bpf-next PATCH v3 0/4] bpf, sockmap BPF_F_INGRESS support
From: John Fastabend @ 2018-03-28 19:49 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, davem
This series adds the BPF_F_INGRESS flag support to the redirect APIs.
Bringing the sockmap API in-line with the cls_bpf redirect APIs.
We add it to both variants of sockmap programs, the first patch adds
support for tx ulp hooks and the third patch adds support for the recv
skb hooks. Patches two and four add tests for the corresponding
ingress redirect hooks.
Follow on patches can address busy polling support, but next series
from me will move the sockmap sample program into selftests.
v2: added static to function definition caught by kbuild bot
v3: fixed an error branch with missing mem_uncharge
in recvmsg op moved receive_queue check outside of RCU region
---
John Fastabend (4):
bpf: sockmap redirect ingress support
bpf: sockmap, add BPF_F_INGRESS tests
bpf: sockmap, BPF_F_INGRESS flag for BPF_SK_SKB_STREAM_VERDICT:
bpf: sockmap, more BPF_SK_SKB_STREAM_VERDICT tests
include/linux/filter.h | 2
include/net/sock.h | 1
kernel/bpf/sockmap.c | 290 ++++++++++++++++++++++++++++++++++++---
net/core/filter.c | 4 -
net/ipv4/tcp.c | 10 +
samples/sockmap/sockmap_kern.c | 62 +++++++-
samples/sockmap/sockmap_test.sh | 40 +++++
samples/sockmap/sockmap_user.c | 58 ++++++++
8 files changed, 430 insertions(+), 37 deletions(-)
^ permalink raw reply
* [bpf-next PATCH v3 1/4] bpf: sockmap redirect ingress support
From: John Fastabend @ 2018-03-28 19:49 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, davem
In-Reply-To: <20180328194710.16072.99278.stgit@john-Precision-Tower-5810>
Add support for the BPF_F_INGRESS flag in sk_msg redirect helper.
To do this add a scatterlist ring for receiving socks to check
before calling into regular recvmsg call path. Additionally, because
the poll wakeup logic only checked the skb recv queue we need to
add a hook in TCP stack (similar to write side) so that we have
a way to wake up polling socks when a scatterlist is redirected
to that sock.
After this all that is needed is for the redirect helper to
push the scatterlist into the psock receive queue.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/linux/filter.h | 1
include/net/sock.h | 1
kernel/bpf/sockmap.c | 198 +++++++++++++++++++++++++++++++++++++++++++++++-
net/core/filter.c | 2
net/ipv4/tcp.c | 10 ++
5 files changed, 207 insertions(+), 5 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 109d05c..d0e207f 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -521,6 +521,7 @@ struct sk_msg_buff {
__u32 key;
__u32 flags;
struct bpf_map *map;
+ struct list_head list;
};
/* Compute the linear packet data range [data, data_end) which
diff --git a/include/net/sock.h b/include/net/sock.h
index 7093111..b8ff435 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1085,6 +1085,7 @@ struct proto {
#endif
bool (*stream_memory_free)(const struct sock *sk);
+ bool (*stream_memory_read)(const struct sock *sk);
/* Memory pressure */
void (*enter_memory_pressure)(struct sock *sk);
void (*leave_memory_pressure)(struct sock *sk);
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 69c5bcc..402e154 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -41,6 +41,8 @@
#include <linux/mm.h>
#include <net/strparser.h>
#include <net/tcp.h>
+#include <linux/ptr_ring.h>
+#include <net/inet_common.h>
#define SOCK_CREATE_FLAG_MASK \
(BPF_F_NUMA_NODE | BPF_F_RDONLY | BPF_F_WRONLY)
@@ -82,6 +84,7 @@ struct smap_psock {
int sg_size;
int eval;
struct sk_msg_buff *cork;
+ struct list_head ingress;
struct strparser strp;
struct bpf_prog *bpf_tx_msg;
@@ -103,6 +106,8 @@ struct smap_psock {
};
static void smap_release_sock(struct smap_psock *psock, struct sock *sock);
+static int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
+ int nonblock, int flags, int *addr_len);
static int bpf_tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size);
static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
int offset, size_t size, int flags);
@@ -112,6 +117,21 @@ static inline struct smap_psock *smap_psock_sk(const struct sock *sk)
return rcu_dereference_sk_user_data(sk);
}
+static bool bpf_tcp_stream_read(const struct sock *sk)
+{
+ struct smap_psock *psock;
+ bool empty = true;
+
+ rcu_read_lock();
+ psock = smap_psock_sk(sk);
+ if (unlikely(!psock))
+ goto out;
+ empty = list_empty(&psock->ingress);
+out:
+ rcu_read_unlock();
+ return !empty;
+}
+
static struct proto tcp_bpf_proto;
static int bpf_tcp_init(struct sock *sk)
{
@@ -135,6 +155,8 @@ static int bpf_tcp_init(struct sock *sk)
if (psock->bpf_tx_msg) {
tcp_bpf_proto.sendmsg = bpf_tcp_sendmsg;
tcp_bpf_proto.sendpage = bpf_tcp_sendpage;
+ tcp_bpf_proto.recvmsg = bpf_tcp_recvmsg;
+ tcp_bpf_proto.stream_memory_read = bpf_tcp_stream_read;
}
sk->sk_prot = &tcp_bpf_proto;
@@ -170,6 +192,7 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
{
void (*close_fun)(struct sock *sk, long timeout);
struct smap_psock_map_entry *e, *tmp;
+ struct sk_msg_buff *md, *mtmp;
struct smap_psock *psock;
struct sock *osk;
@@ -188,6 +211,12 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
close_fun = psock->save_close;
write_lock_bh(&sk->sk_callback_lock);
+ list_for_each_entry_safe(md, mtmp, &psock->ingress, list) {
+ list_del(&md->list);
+ free_start_sg(psock->sock, md);
+ kfree(md);
+ }
+
list_for_each_entry_safe(e, tmp, &psock->maps, list) {
osk = cmpxchg(e->entry, sk, NULL);
if (osk == sk) {
@@ -468,6 +497,72 @@ static unsigned int smap_do_tx_msg(struct sock *sk,
return _rc;
}
+static int bpf_tcp_ingress(struct sock *sk, int apply_bytes,
+ struct smap_psock *psock,
+ struct sk_msg_buff *md, int flags)
+{
+ bool apply = apply_bytes;
+ size_t size, copied = 0;
+ struct sk_msg_buff *r;
+ int err = 0, i;
+
+ r = kzalloc(sizeof(struct sk_msg_buff), __GFP_NOWARN | GFP_KERNEL);
+ if (unlikely(!r))
+ return -ENOMEM;
+
+ lock_sock(sk);
+ r->sg_start = md->sg_start;
+ i = md->sg_start;
+
+ do {
+ r->sg_data[i] = md->sg_data[i];
+
+ size = (apply && apply_bytes < md->sg_data[i].length) ?
+ apply_bytes : md->sg_data[i].length;
+
+ if (!sk_wmem_schedule(sk, size)) {
+ if (!copied)
+ err = -ENOMEM;
+ break;
+ }
+
+ sk_mem_charge(sk, size);
+ r->sg_data[i].length = size;
+ md->sg_data[i].length -= size;
+ md->sg_data[i].offset += size;
+ copied += size;
+
+ if (md->sg_data[i].length) {
+ get_page(sg_page(&r->sg_data[i]));
+ r->sg_end = (i + 1) == MAX_SKB_FRAGS ? 0 : i + 1;
+ } else {
+ i++;
+ if (i == MAX_SKB_FRAGS)
+ i = 0;
+ r->sg_end = i;
+ }
+
+ if (apply) {
+ apply_bytes -= size;
+ if (!apply_bytes)
+ break;
+ }
+ } while (i != md->sg_end);
+
+ md->sg_start = i;
+
+ if (!err) {
+ list_add_tail(&r->list, &psock->ingress);
+ sk->sk_data_ready(sk);
+ } else {
+ free_start_sg(sk, r);
+ kfree(r);
+ }
+
+ release_sock(sk);
+ return err;
+}
+
static int bpf_tcp_sendmsg_do_redirect(struct sock *sk, int send,
struct sk_msg_buff *md,
int flags)
@@ -475,6 +570,7 @@ static int bpf_tcp_sendmsg_do_redirect(struct sock *sk, int send,
struct smap_psock *psock;
struct scatterlist *sg;
int i, err, free = 0;
+ bool ingress = !!(md->flags & BPF_F_INGRESS);
sg = md->sg_data;
@@ -487,9 +583,14 @@ static int bpf_tcp_sendmsg_do_redirect(struct sock *sk, int send,
goto out_rcu;
rcu_read_unlock();
- lock_sock(sk);
- err = bpf_tcp_push(sk, send, md, flags, false);
- release_sock(sk);
+
+ if (ingress) {
+ err = bpf_tcp_ingress(sk, send, psock, md, flags);
+ } else {
+ lock_sock(sk);
+ err = bpf_tcp_push(sk, send, md, flags, false);
+ release_sock(sk);
+ }
smap_release_sock(psock, sk);
if (unlikely(err))
goto out;
@@ -623,6 +724,89 @@ static int bpf_exec_tx_verdict(struct smap_psock *psock,
return err;
}
+static int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
+ int nonblock, int flags, int *addr_len)
+{
+ struct iov_iter *iter = &msg->msg_iter;
+ struct smap_psock *psock;
+ int copied = 0;
+
+ if (unlikely(flags & MSG_ERRQUEUE))
+ return inet_recv_error(sk, msg, len, addr_len);
+
+ rcu_read_lock();
+ psock = smap_psock_sk(sk);
+ if (unlikely(!psock))
+ goto out;
+
+ if (unlikely(!refcount_inc_not_zero(&psock->refcnt)))
+ goto out;
+ rcu_read_unlock();
+
+ if (!skb_queue_empty(&sk->sk_receive_queue))
+ return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
+
+ lock_sock(sk);
+ while (copied != len) {
+ struct scatterlist *sg;
+ struct sk_msg_buff *md;
+ int i;
+
+ md = list_first_entry_or_null(&psock->ingress,
+ struct sk_msg_buff, list);
+ if (unlikely(!md))
+ break;
+ i = md->sg_start;
+ do {
+ struct page *page;
+ int n, copy;
+
+ sg = &md->sg_data[i];
+ copy = sg->length;
+ page = sg_page(sg);
+
+ if (copied + copy > len)
+ copy = len - copied;
+
+ n = copy_page_to_iter(page, sg->offset, copy, iter);
+ if (n != copy) {
+ md->sg_start = i;
+ release_sock(sk);
+ smap_release_sock(psock, sk);
+ return -EFAULT;
+ }
+
+ copied += copy;
+ sg->offset += copy;
+ sg->length -= copy;
+ sk_mem_uncharge(sk, copy);
+
+ if (!sg->length) {
+ i++;
+ if (i == MAX_SKB_FRAGS)
+ i = 0;
+ put_page(page);
+ }
+ if (copied == len)
+ break;
+ } while (i != md->sg_end);
+ md->sg_start = i;
+
+ if (!sg->length && md->sg_start == md->sg_end) {
+ list_del(&md->list);
+ kfree(md);
+ }
+ }
+
+ release_sock(sk);
+ smap_release_sock(psock, sk);
+ return copied;
+out:
+ rcu_read_unlock();
+ return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
+}
+
+
static int bpf_tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
{
int flags = msg->msg_flags | MSG_NO_SHARED_FRAGS;
@@ -1107,6 +1291,7 @@ static void sock_map_remove_complete(struct bpf_stab *stab)
static void smap_gc_work(struct work_struct *w)
{
struct smap_psock_map_entry *e, *tmp;
+ struct sk_msg_buff *md, *mtmp;
struct smap_psock *psock;
psock = container_of(w, struct smap_psock, gc_work);
@@ -1131,6 +1316,12 @@ static void smap_gc_work(struct work_struct *w)
kfree(psock->cork);
}
+ list_for_each_entry_safe(md, mtmp, &psock->ingress, list) {
+ list_del(&md->list);
+ free_start_sg(psock->sock, md);
+ kfree(md);
+ }
+
list_for_each_entry_safe(e, tmp, &psock->maps, list) {
list_del(&e->list);
kfree(e);
@@ -1160,6 +1351,7 @@ static struct smap_psock *smap_init_psock(struct sock *sock,
INIT_WORK(&psock->tx_work, smap_tx_work);
INIT_WORK(&psock->gc_work, smap_gc_work);
INIT_LIST_HEAD(&psock->maps);
+ INIT_LIST_HEAD(&psock->ingress);
refcount_set(&psock->refcnt, 1);
rcu_assign_sk_user_data(sock, psock);
diff --git a/net/core/filter.c b/net/core/filter.c
index 00c711c..11b1f16 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1894,7 +1894,7 @@ struct sock *do_sk_redirect_map(struct sk_buff *skb)
struct bpf_map *, map, u32, key, u64, flags)
{
/* If user passes invalid input drop the packet. */
- if (unlikely(flags))
+ if (unlikely(flags & ~(BPF_F_INGRESS)))
return SK_DROP;
msg->key = key;
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 0c31be3..bccc4c2 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -485,6 +485,14 @@ static void tcp_tx_timestamp(struct sock *sk, u16 tsflags)
}
}
+static inline bool tcp_stream_is_readable(const struct tcp_sock *tp,
+ int target, struct sock *sk)
+{
+ return (tp->rcv_nxt - tp->copied_seq >= target) ||
+ (sk->sk_prot->stream_memory_read ?
+ sk->sk_prot->stream_memory_read(sk) : false);
+}
+
/*
* Wait for a TCP event.
*
@@ -554,7 +562,7 @@ __poll_t tcp_poll(struct file *file, struct socket *sock, poll_table *wait)
tp->urg_data)
target++;
- if (tp->rcv_nxt - tp->copied_seq >= target)
+ if (tcp_stream_is_readable(tp, target, sk))
mask |= EPOLLIN | EPOLLRDNORM;
if (!(sk->sk_shutdown & SEND_SHUTDOWN)) {
^ permalink raw reply related
* [bpf-next PATCH v3 2/4] bpf: sockmap, add BPF_F_INGRESS tests
From: John Fastabend @ 2018-03-28 19:49 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, davem
In-Reply-To: <20180328194710.16072.99278.stgit@john-Precision-Tower-5810>
Add a set of tests to verify ingress flag in redirect helpers
works correctly with various msg sizes.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
samples/sockmap/sockmap_kern.c | 41 +++++++++++++++++++++++++++++----------
samples/sockmap/sockmap_test.sh | 22 ++++++++++++++++++++-
samples/sockmap/sockmap_user.c | 35 +++++++++++++++++++++++++++++++++
3 files changed, 87 insertions(+), 11 deletions(-)
diff --git a/samples/sockmap/sockmap_kern.c b/samples/sockmap/sockmap_kern.c
index 9ad5ba7..ca28722 100644
--- a/samples/sockmap/sockmap_kern.c
+++ b/samples/sockmap/sockmap_kern.c
@@ -54,7 +54,7 @@ struct bpf_map_def SEC("maps") sock_map_redir = {
.type = BPF_MAP_TYPE_SOCKMAP,
.key_size = sizeof(int),
.value_size = sizeof(int),
- .max_entries = 1,
+ .max_entries = 20,
};
struct bpf_map_def SEC("maps") sock_apply_bytes = {
@@ -78,6 +78,13 @@ struct bpf_map_def SEC("maps") sock_pull_bytes = {
.max_entries = 2
};
+struct bpf_map_def SEC("maps") sock_redir_flags = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .max_entries = 1
+};
+
SEC("sk_skb1")
int bpf_prog1(struct __sk_buff *skb)
@@ -197,8 +204,9 @@ int bpf_prog5(struct sk_msg_md *msg)
SEC("sk_msg3")
int bpf_prog6(struct sk_msg_md *msg)
{
- int *bytes, zero = 0, one = 1;
- int *start, *end;
+ int *bytes, zero = 0, one = 1, key = 0;
+ int *start, *end, *f;
+ __u64 flags = 0;
bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
if (bytes)
@@ -210,15 +218,22 @@ int bpf_prog6(struct sk_msg_md *msg)
end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
if (start && end)
bpf_msg_pull_data(msg, *start, *end, 0);
- return bpf_msg_redirect_map(msg, &sock_map_redir, zero, 0);
+ f = bpf_map_lookup_elem(&sock_redir_flags, &zero);
+ if (f && *f) {
+ key = 2;
+ flags = *f;
+ }
+ return bpf_msg_redirect_map(msg, &sock_map_redir, key, flags);
}
SEC("sk_msg4")
int bpf_prog7(struct sk_msg_md *msg)
{
- int err1 = 0, err2 = 0, zero = 0, one = 1;
- int *bytes, *start, *end, len1, len2;
+ int err1 = 0, err2 = 0, zero = 0, one = 1, key = 0;
+ int *f, *bytes, *start, *end, len1, len2;
+ __u64 flags = 0;
+ int err;
bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
if (bytes)
err1 = bpf_msg_apply_bytes(msg, *bytes);
@@ -229,7 +244,6 @@ int bpf_prog7(struct sk_msg_md *msg)
start = bpf_map_lookup_elem(&sock_pull_bytes, &zero);
end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
if (start && end) {
- int err;
bpf_printk("sk_msg2: pull(%i:%i)\n",
start ? *start : 0, end ? *end : 0);
@@ -241,9 +255,16 @@ int bpf_prog7(struct sk_msg_md *msg)
bpf_printk("sk_msg2: length update %i->%i\n",
len1, len2);
}
- bpf_printk("sk_msg3: redirect(%iB) err1=%i err2=%i\n",
- len1, err1, err2);
- return bpf_msg_redirect_map(msg, &sock_map_redir, zero, 0);
+ f = bpf_map_lookup_elem(&sock_redir_flags, &zero);
+ if (f && *f) {
+ key = 2;
+ flags = *f;
+ }
+ bpf_printk("sk_msg3: redirect(%iB) flags=%i err=%i\n",
+ len1, flags, err1 ? err1 : err2);
+ err = bpf_msg_redirect_map(msg, &sock_map_redir, key, flags);
+ bpf_printk("sk_msg3: err %i\n", err);
+ return err;
}
SEC("sk_msg5")
diff --git a/samples/sockmap/sockmap_test.sh b/samples/sockmap/sockmap_test.sh
index 6d8cc40..13b205f 100755
--- a/samples/sockmap/sockmap_test.sh
+++ b/samples/sockmap/sockmap_test.sh
@@ -1,5 +1,5 @@
#Test a bunch of positive cases to verify basic functionality
-for prog in "--txmsg" "--txmsg_redir" "--txmsg_drop"; do
+for prog in "--txmsg_redir --txmsg_ingress" "--txmsg" "--txmsg_redir" "--txmsg_redir --txmsg_ingress" "--txmsg_drop"; do
for t in "sendmsg" "sendpage"; do
for r in 1 10 100; do
for i in 1 10 100; do
@@ -100,6 +100,16 @@ for t in "sendmsg" "sendpage"; do
sleep 2
done
+prog="--txmsg_redir --txmsg_apply 1 --txmsg_ingress"
+
+for t in "sendmsg" "sendpage"; do
+ TEST="./sockmap --cgroup /mnt/cgroup2/ -t $t -r $r -i $i -l $l $prog"
+ echo $TEST
+ $TEST
+ sleep 2
+done
+
+
# Test apply and redirect with larger value than send
r=1
i=8
@@ -113,6 +123,16 @@ for t in "sendmsg" "sendpage"; do
sleep 2
done
+prog="--txmsg_redir --txmsg_apply 2048 --txmsg_ingress"
+
+for t in "sendmsg" "sendpage"; do
+ TEST="./sockmap --cgroup /mnt/cgroup2/ -t $t -r $r -i $i -l $l $prog"
+ echo $TEST
+ $TEST
+ sleep 2
+done
+
+
# Test apply and redirect with apply that never reaches limit
r=1024
i=1
diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
index 07aa237..f7503f4 100644
--- a/samples/sockmap/sockmap_user.c
+++ b/samples/sockmap/sockmap_user.c
@@ -64,6 +64,7 @@
int txmsg_cork;
int txmsg_start;
int txmsg_end;
+int txmsg_ingress;
static const struct option long_options[] = {
{"help", no_argument, NULL, 'h' },
@@ -83,6 +84,7 @@
{"txmsg_cork", required_argument, NULL, 'k'},
{"txmsg_start", required_argument, NULL, 's'},
{"txmsg_end", required_argument, NULL, 'e'},
+ {"txmsg_ingress", no_argument, &txmsg_ingress, 1 },
{0, 0, NULL, 0 }
};
@@ -793,6 +795,39 @@ int main(int argc, char **argv)
return err;
}
}
+
+ if (txmsg_ingress) {
+ int in = BPF_F_INGRESS;
+
+ i = 0;
+ err = bpf_map_update_elem(map_fd[6], &i, &in, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (txmsg_ingress): %d (%s)\n",
+ err, strerror(errno));
+ }
+ i = 1;
+ err = bpf_map_update_elem(map_fd[1], &i, &p1, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (p1 txmsg): %d (%s)\n",
+ err, strerror(errno));
+ }
+ err = bpf_map_update_elem(map_fd[2], &i, &p1, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (p1 redir): %d (%s)\n",
+ err, strerror(errno));
+ }
+
+ i = 2;
+ err = bpf_map_update_elem(map_fd[2], &i, &p2, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (p2 txmsg): %d (%s)\n",
+ err, strerror(errno));
+ }
+ }
}
if (txmsg_drop)
^ permalink raw reply related
* [bpf-next PATCH v3 3/4] bpf: sockmap, BPF_F_INGRESS flag for BPF_SK_SKB_STREAM_VERDICT:
From: John Fastabend @ 2018-03-28 19:49 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, davem
In-Reply-To: <20180328194710.16072.99278.stgit@john-Precision-Tower-5810>
Add support for the BPF_F_INGRESS flag in skb redirect helper. To
do this convert skb into a scatterlist and push into ingress queue.
This is the same logic that is used in the sk_msg redirect helper
so it should feel familiar.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/linux/filter.h | 1 +
kernel/bpf/sockmap.c | 94 +++++++++++++++++++++++++++++++++++++++---------
net/core/filter.c | 2 +
3 files changed, 78 insertions(+), 19 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index d0e207f..7de778a 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -521,6 +521,7 @@ struct sk_msg_buff {
__u32 key;
__u32 flags;
struct bpf_map *map;
+ struct sk_buff *skb;
struct list_head list;
};
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 402e154..9192fdb 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -785,7 +785,8 @@ static int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
i++;
if (i == MAX_SKB_FRAGS)
i = 0;
- put_page(page);
+ if (!md->skb)
+ put_page(page);
}
if (copied == len)
break;
@@ -794,6 +795,8 @@ static int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
if (!sg->length && md->sg_start == md->sg_end) {
list_del(&md->list);
+ if (md->skb)
+ consume_skb(md->skb);
kfree(md);
}
}
@@ -1045,27 +1048,72 @@ static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb)
__SK_DROP;
}
+static int smap_do_ingress(struct smap_psock *psock, struct sk_buff *skb)
+{
+ struct sock *sk = psock->sock;
+ int copied = 0, num_sg;
+ struct sk_msg_buff *r;
+
+ r = kzalloc(sizeof(struct sk_msg_buff), __GFP_NOWARN | GFP_ATOMIC);
+ if (unlikely(!r))
+ return -EAGAIN;
+
+ if (!sk_rmem_schedule(sk, skb, skb->len)) {
+ kfree(r);
+ return -EAGAIN;
+ }
+
+ sg_init_table(r->sg_data, MAX_SKB_FRAGS);
+ num_sg = skb_to_sgvec(skb, r->sg_data, 0, skb->len);
+ if (unlikely(num_sg < 0)) {
+ kfree(r);
+ return num_sg;
+ }
+ sk_mem_charge(sk, skb->len);
+ copied = skb->len;
+ r->sg_start = 0;
+ r->sg_end = num_sg == MAX_SKB_FRAGS ? 0 : num_sg;
+ r->skb = skb;
+ list_add_tail(&r->list, &psock->ingress);
+ sk->sk_data_ready(sk);
+ return copied;
+}
+
static void smap_do_verdict(struct smap_psock *psock, struct sk_buff *skb)
{
+ struct smap_psock *peer;
struct sock *sk;
+ __u32 in;
int rc;
rc = smap_verdict_func(psock, skb);
switch (rc) {
case __SK_REDIRECT:
sk = do_sk_redirect_map(skb);
- if (likely(sk)) {
- struct smap_psock *peer = smap_psock_sk(sk);
-
- if (likely(peer &&
- test_bit(SMAP_TX_RUNNING, &peer->state) &&
- !sock_flag(sk, SOCK_DEAD) &&
- sock_writeable(sk))) {
- skb_set_owner_w(skb, sk);
- skb_queue_tail(&peer->rxqueue, skb);
- schedule_work(&peer->tx_work);
- break;
- }
+ if (!sk) {
+ kfree_skb(skb);
+ break;
+ }
+
+ peer = smap_psock_sk(sk);
+ in = (TCP_SKB_CB(skb)->bpf.flags) & BPF_F_INGRESS;
+
+ if (unlikely(!peer || sock_flag(sk, SOCK_DEAD) ||
+ !test_bit(SMAP_TX_RUNNING, &peer->state))) {
+ kfree_skb(skb);
+ break;
+ }
+
+ if (!in && sock_writeable(sk)) {
+ skb_set_owner_w(skb, sk);
+ skb_queue_tail(&peer->rxqueue, skb);
+ schedule_work(&peer->tx_work);
+ break;
+ } else if (in &&
+ atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf) {
+ skb_queue_tail(&peer->rxqueue, skb);
+ schedule_work(&peer->tx_work);
+ break;
}
/* Fall through and free skb otherwise */
case __SK_DROP:
@@ -1127,15 +1175,23 @@ static void smap_tx_work(struct work_struct *w)
}
while ((skb = skb_dequeue(&psock->rxqueue))) {
+ __u32 flags;
+
rem = skb->len;
off = 0;
start:
+ flags = (TCP_SKB_CB(skb)->bpf.flags) & BPF_F_INGRESS;
do {
- if (likely(psock->sock->sk_socket))
- n = skb_send_sock_locked(psock->sock,
- skb, off, rem);
- else
+ if (likely(psock->sock->sk_socket)) {
+ if (flags)
+ n = smap_do_ingress(psock, skb);
+ else
+ n = skb_send_sock_locked(psock->sock,
+ skb, off, rem);
+ } else {
n = -EINVAL;
+ }
+
if (n <= 0) {
if (n == -EAGAIN) {
/* Retry when space is available */
@@ -1153,7 +1209,9 @@ static void smap_tx_work(struct work_struct *w)
rem -= n;
off += n;
} while (rem);
- kfree_skb(skb);
+
+ if (!flags)
+ kfree_skb(skb);
}
out:
release_sock(psock->sock);
diff --git a/net/core/filter.c b/net/core/filter.c
index 11b1f16..b46916d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1855,7 +1855,7 @@ int skb_do_redirect(struct sk_buff *skb)
struct tcp_skb_cb *tcb = TCP_SKB_CB(skb);
/* If user passes invalid input drop the packet. */
- if (unlikely(flags))
+ if (unlikely(flags & ~(BPF_F_INGRESS)))
return SK_DROP;
tcb->bpf.key = key;
^ permalink raw reply related
* [bpf-next PATCH v3 4/4] bpf: sockmap, more BPF_SK_SKB_STREAM_VERDICT tests
From: John Fastabend @ 2018-03-28 19:49 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, davem
In-Reply-To: <20180328194710.16072.99278.stgit@john-Precision-Tower-5810>
Add BPF_SK_SKB_STREAM_VERDICT tests for ingress hook. While
we do this also bring stream tests in-line with MSG based
testing.
A map for skb options is added for userland to push options
at BPF programs.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
samples/sockmap/sockmap_kern.c | 21 ++++++++++++++++++---
samples/sockmap/sockmap_test.sh | 20 +++++++++++++++++++-
samples/sockmap/sockmap_user.c | 23 +++++++++++++++++++++++
3 files changed, 60 insertions(+), 4 deletions(-)
diff --git a/samples/sockmap/sockmap_kern.c b/samples/sockmap/sockmap_kern.c
index ca28722..9ff8bc5 100644
--- a/samples/sockmap/sockmap_kern.c
+++ b/samples/sockmap/sockmap_kern.c
@@ -85,6 +85,12 @@ struct bpf_map_def SEC("maps") sock_redir_flags = {
.max_entries = 1
};
+struct bpf_map_def SEC("maps") sock_skb_opts = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .max_entries = 1
+};
SEC("sk_skb1")
int bpf_prog1(struct __sk_buff *skb)
@@ -97,15 +103,24 @@ int bpf_prog2(struct __sk_buff *skb)
{
__u32 lport = skb->local_port;
__u32 rport = skb->remote_port;
- int ret = 0;
+ int len, *f, ret, zero = 0;
+ __u64 flags = 0;
if (lport == 10000)
ret = 10;
else
ret = 1;
- bpf_printk("sockmap: %d -> %d @ %d\n", lport, bpf_ntohl(rport), ret);
- return bpf_sk_redirect_map(skb, &sock_map, ret, 0);
+ len = (__u32)skb->data_end - (__u32)skb->data;
+ f = bpf_map_lookup_elem(&sock_skb_opts, &zero);
+ if (f && *f) {
+ ret = 3;
+ flags = *f;
+ }
+
+ bpf_printk("sk_skb2: redirect(%iB) flags=%i\n",
+ len, flags);
+ return bpf_sk_redirect_map(skb, &sock_map, ret, flags);
}
SEC("sockops")
diff --git a/samples/sockmap/sockmap_test.sh b/samples/sockmap/sockmap_test.sh
index 13b205f..ace75f0 100755
--- a/samples/sockmap/sockmap_test.sh
+++ b/samples/sockmap/sockmap_test.sh
@@ -1,5 +1,5 @@
#Test a bunch of positive cases to verify basic functionality
-for prog in "--txmsg_redir --txmsg_ingress" "--txmsg" "--txmsg_redir" "--txmsg_redir --txmsg_ingress" "--txmsg_drop"; do
+for prog in "--txmsg_redir --txmsg_skb" "--txmsg_redir --txmsg_ingress" "--txmsg" "--txmsg_redir" "--txmsg_redir --txmsg_ingress" "--txmsg_drop"; do
for t in "sendmsg" "sendpage"; do
for r in 1 10 100; do
for i in 1 10 100; do
@@ -109,6 +109,15 @@ for t in "sendmsg" "sendpage"; do
sleep 2
done
+prog="--txmsg_redir --txmsg_apply 1 --txmsg_skb"
+
+for t in "sendmsg" "sendpage"; do
+ TEST="./sockmap --cgroup /mnt/cgroup2/ -t $t -r $r -i $i -l $l $prog"
+ echo $TEST
+ $TEST
+ sleep 2
+done
+
# Test apply and redirect with larger value than send
r=1
@@ -132,6 +141,15 @@ for t in "sendmsg" "sendpage"; do
sleep 2
done
+prog="--txmsg_redir --txmsg_apply 2048 --txmsg_skb"
+
+for t in "sendmsg" "sendpage"; do
+ TEST="./sockmap --cgroup /mnt/cgroup2/ -t $t -r $r -i $i -l $l $prog"
+ echo $TEST
+ $TEST
+ sleep 2
+done
+
# Test apply and redirect with apply that never reaches limit
r=1024
diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
index f7503f4..6f23349 100644
--- a/samples/sockmap/sockmap_user.c
+++ b/samples/sockmap/sockmap_user.c
@@ -65,6 +65,7 @@
int txmsg_start;
int txmsg_end;
int txmsg_ingress;
+int txmsg_skb;
static const struct option long_options[] = {
{"help", no_argument, NULL, 'h' },
@@ -85,6 +86,7 @@
{"txmsg_start", required_argument, NULL, 's'},
{"txmsg_end", required_argument, NULL, 'e'},
{"txmsg_ingress", no_argument, &txmsg_ingress, 1 },
+ {"txmsg_skb", no_argument, &txmsg_skb, 1 },
{0, 0, NULL, 0 }
};
@@ -828,6 +830,27 @@ int main(int argc, char **argv)
err, strerror(errno));
}
}
+
+ if (txmsg_skb) {
+ int skb_fd = (test == SENDMSG || test == SENDPAGE) ? p2 : p1;
+ int ingress = BPF_F_INGRESS;
+
+ i = 0;
+ err = bpf_map_update_elem(map_fd[7], &i, &ingress, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (txmsg_ingress): %d (%s)\n",
+ err, strerror(errno));
+ }
+
+ i = 3;
+ err = bpf_map_update_elem(map_fd[0], &i, &skb_fd, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (c1 sockmap): %d (%s)\n",
+ err, strerror(errno));
+ }
+ }
}
if (txmsg_drop)
^ permalink raw reply related
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics
From: Florian Fainelli @ 2018-03-28 19:57 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, netdev
In-Reply-To: <20180328193304.GB20749@lunn.ch>
On 03/28/2018 12:33 PM, Andrew Lunn wrote:
> On Wed, Mar 28, 2018 at 11:17:19AM -0700, Florian Fainelli wrote:
>> On 03/27/2018 02:59 PM, Andrew Lunn wrote:
>>> Count the numbers of various ATU and VTU violation statistics and
>>> return them as part of the ethtool -S statistics.
>>>
>>> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
>>> ---
>>> drivers/net/dsa/mv88e6xxx/chip.c | 50 ++++++++++++++++++++++++++++-----
>>> drivers/net/dsa/mv88e6xxx/chip.h | 13 ++++++---
>>> drivers/net/dsa/mv88e6xxx/global1_atu.c | 12 +++++---
>>> drivers/net/dsa/mv88e6xxx/global1_vtu.c | 8 ++++--
>>> drivers/net/dsa/mv88e6xxx/serdes.c | 15 ++++++----
>>> drivers/net/dsa/mv88e6xxx/serdes.h | 8 +++---
>>> 6 files changed, 78 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
>>> index 9a5d786b4885..186021f98c5d 100644
>>> --- a/drivers/net/dsa/mv88e6xxx/chip.c
>>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
>>> @@ -723,6 +723,24 @@ static int mv88e6320_stats_get_strings(struct mv88e6xxx_chip *chip,
>>> STATS_TYPE_BANK0 | STATS_TYPE_BANK1);
>>> }
>>>
>>> +static const uint8_t *mv88e6xxx_atu_vtu_stats_strings[] = {
>>
>> Why not const char *?
>
> The ethtool call passes i uint8_t *data to receive the copy into. I'm
> keeping it consistent.
Fair enough.
>
>>> +static void mv88e6xxx_atu_vtu_get_strings(uint8_t *data)
>>> +{
>>> + int i;
>>
>> unsigned int i?
>
> I could do, but it seems unlikely it will overflow 31 bits.
The size cannot be negative, so unsigned int would seem like a natural
choice.
>
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); i++)
>>> + strlcpy(data + i * ETH_GSTRING_LEN,
>>> + mv88e6xxx_atu_vtu_stats_strings[i],
>>> + ETH_GSTRING_LEN);
>>> +}
>>> +
>>> static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
>>> uint8_t *data)
>>> {
>>> @@ -736,9 +754,12 @@ static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
>>>
>>> if (chip->info->ops->serdes_get_strings) {
>>> data += count * ETH_GSTRING_LEN;
>>> - chip->info->ops->serdes_get_strings(chip, port, data);
>>> + count = chip->info->ops->serdes_get_strings(chip, port, data);
>>> }
>>>
>>> + data += count * ETH_GSTRING_LEN;
>>> + mv88e6xxx_atu_vtu_get_strings(data);
>>> +
>>> mutex_unlock(&chip->reg_lock);
>>> }
>>>
>>> @@ -783,10 +804,13 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
>>> if (chip->info->ops->serdes_get_sset_count)
>>> serdes_count = chip->info->ops->serdes_get_sset_count(chip,
>>> port);
>>> - if (serdes_count < 0)
>>> + if (serdes_count < 0) {
>>> count = serdes_count;
>>> - else
>>> - count += serdes_count;
>>> + goto out;
>>> + }
>>> + count += serdes_count;
>>> + count += ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings);
>>> +
>>> out:
>>> mutex_unlock(&chip->reg_lock);
>>>
>>> @@ -841,6 +865,16 @@ static int mv88e6390_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
>>> 0);
>>> }
>>>
>>> +static void mv88e6xxx_atu_vtu_get_stats(struct mv88e6xxx_chip *chip, int port,
>>> + uint64_t *data)
>>> +{
>>> + *data++ = chip->ports[port].atu_member_violation;
>>> + *data++ = chip->ports[port].atu_miss_violation;
>>> + *data++ = chip->ports[port].atu_full_violation;
>>> + *data++ = chip->ports[port].vtu_member_violation;
>>> + *data++ = chip->ports[port].vtu_miss_violation;
>>
>> This looks fine, but I suppose you could just have an u64 pointer which
>> is initialized to point to atu_member_violation, and then just do
>> pointer arithmetics to iterate, this would avoid possibly missing that
>> function in case new ATU/VTU violations are handled in the future?
>
> KISS. This works and is obvious.
Fair enough.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: systemport: Remove adaptive TX coalescing
From: Tal Gilboa @ 2018-03-28 20:51 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: davem, jaedon.shin, pgynther, opendmb, Michael Chan, gospo,
saeedm
In-Reply-To: <20180327194707.31857-2-f.fainelli@gmail.com>
On 3/27/2018 10:47 PM, Florian Fainelli wrote:
> Adaptive TX coalescing is not currently giving us any advantages and
> ends up making the CPU spin more frequently until TX completion. Deny
> and disable adaptive TX coalescing for now and rely on static
> configuration, we can always add it back later.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/broadcom/bcmsysport.c | 61 ++++--------------------------
> drivers/net/ethernet/broadcom/bcmsysport.h | 1 -
> 2 files changed, 8 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
> index 4e26f606a7f2..1e52bb7d822e 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> @@ -15,7 +15,6 @@
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/netdevice.h>
> -#include <linux/net_dim.h>
> #include <linux/etherdevice.h>
> #include <linux/platform_device.h>
> #include <linux/of.h>
> @@ -588,7 +587,8 @@ static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv)
> rdma_writel(priv, reg, RDMA_MBDONE_INTR);
> }
>
> -static void bcm_sysport_set_tx_coalesce(struct bcm_sysport_tx_ring *ring)
> +static void bcm_sysport_set_tx_coalesce(struct bcm_sysport_tx_ring *ring,
> + struct ethtool_coalesce *ec)
> {
> struct bcm_sysport_priv *priv = ring->priv;
> u32 reg;
> @@ -596,8 +596,8 @@ static void bcm_sysport_set_tx_coalesce(struct bcm_sysport_tx_ring *ring)
> reg = tdma_readl(priv, TDMA_DESC_RING_INTR_CONTROL(ring->index));
> reg &= ~(RING_INTR_THRESH_MASK |
> RING_TIMEOUT_MASK << RING_TIMEOUT_SHIFT);
> - reg |= ring->dim.coal_pkts;
> - reg |= DIV_ROUND_UP(ring->dim.coal_usecs * 1000, 8192) <<
> + reg |= ec->tx_max_coalesced_frames;
> + reg |= DIV_ROUND_UP(ec->tx_coalesce_usecs * 1000, 8192) <<
> RING_TIMEOUT_SHIFT;
> tdma_writel(priv, reg, TDMA_DESC_RING_INTR_CONTROL(ring->index));
> }
> @@ -606,18 +606,12 @@ static int bcm_sysport_get_coalesce(struct net_device *dev,
> struct ethtool_coalesce *ec)
> {
> struct bcm_sysport_priv *priv = netdev_priv(dev);
> - struct bcm_sysport_tx_ring *ring;
> - unsigned int i;
> u32 reg;
>
> reg = tdma_readl(priv, TDMA_DESC_RING_INTR_CONTROL(0));
>
> ec->tx_coalesce_usecs = (reg >> RING_TIMEOUT_SHIFT) * 8192 / 1000;
> ec->tx_max_coalesced_frames = reg & RING_INTR_THRESH_MASK;
> - for (i = 0; i < dev->num_tx_queues; i++) {
> - ring = &priv->tx_rings[i];
> - ec->use_adaptive_tx_coalesce |= ring->dim.use_dim;
> - }
>
> reg = rdma_readl(priv, RDMA_MBDONE_INTR);
>
> @@ -632,7 +626,6 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
> struct ethtool_coalesce *ec)
> {
> struct bcm_sysport_priv *priv = netdev_priv(dev);
> - struct bcm_sysport_tx_ring *ring;
> unsigned int i;
>
> /* Base system clock is 125Mhz, DMA timeout is this reference clock
> @@ -646,20 +639,12 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
> return -EINVAL;
>
> if ((ec->tx_coalesce_usecs == 0 && ec->tx_max_coalesced_frames == 0) ||
> - (ec->rx_coalesce_usecs == 0 && ec->rx_max_coalesced_frames == 0))
> + (ec->rx_coalesce_usecs == 0 && ec->rx_max_coalesced_frames == 0) ||
> + ec->use_adaptive_tx_coalesce)
> return -EINVAL;
>
> - for (i = 0; i < dev->num_tx_queues; i++) {
> - ring = &priv->tx_rings[i];
> - ring->dim.coal_pkts = ec->tx_max_coalesced_frames;
> - ring->dim.coal_usecs = ec->tx_coalesce_usecs;
> - if (!ec->use_adaptive_tx_coalesce && ring->dim.use_dim) {
> - ring->dim.coal_pkts = 1;
> - ring->dim.coal_usecs = 0;
> - }
> - ring->dim.use_dim = ec->use_adaptive_tx_coalesce;
> - bcm_sysport_set_tx_coalesce(ring);
> - }
> + for (i = 0; i < dev->num_tx_queues; i++)
> + bcm_sysport_set_tx_coalesce(&priv->tx_rings[i], ec);
>
> priv->dim.coal_usecs = ec->rx_coalesce_usecs;
> priv->dim.coal_pkts = ec->rx_max_coalesced_frames;
> @@ -940,8 +925,6 @@ static unsigned int __bcm_sysport_tx_reclaim(struct bcm_sysport_priv *priv,
> ring->packets += pkts_compl;
> ring->bytes += bytes_compl;
> u64_stats_update_end(&priv->syncp);
> - ring->dim.packets = pkts_compl;
> - ring->dim.bytes = bytes_compl;
>
> ring->c_index = c_index;
>
> @@ -987,7 +970,6 @@ static int bcm_sysport_tx_poll(struct napi_struct *napi, int budget)
> {
> struct bcm_sysport_tx_ring *ring =
> container_of(napi, struct bcm_sysport_tx_ring, napi);
> - struct net_dim_sample dim_sample;
> unsigned int work_done = 0;
>
> work_done = bcm_sysport_tx_reclaim(ring->priv, ring);
> @@ -1004,12 +986,6 @@ static int bcm_sysport_tx_poll(struct napi_struct *napi, int budget)
> return 0;
> }
>
> - if (ring->dim.use_dim) {
> - net_dim_sample(ring->dim.event_ctr, ring->dim.packets,
> - ring->dim.bytes, &dim_sample);
> - net_dim(&ring->dim.dim, dim_sample);
> - }
> -
> return budget;
> }
>
> @@ -1089,23 +1065,6 @@ static void bcm_sysport_dim_work(struct work_struct *work)
> dim->state = NET_DIM_START_MEASURE;
> }
>
> -static void bcm_sysport_dim_tx_work(struct work_struct *work)
> -{
> - struct net_dim *dim = container_of(work, struct net_dim, work);
> - struct bcm_sysport_net_dim *ndim =
> - container_of(dim, struct bcm_sysport_net_dim, dim);
> - struct bcm_sysport_tx_ring *ring =
> - container_of(ndim, struct bcm_sysport_tx_ring, dim);
> - struct net_dim_cq_moder cur_profile =
> - net_dim_get_profile(dim->mode, dim->profile_ix);
> -
> - ring->dim.coal_usecs = cur_profile.usec;
> - ring->dim.coal_pkts = cur_profile.pkts;
> -
> - bcm_sysport_set_tx_coalesce(ring);
> - dim->state = NET_DIM_START_MEASURE;
> -}
> -
> /* RX and misc interrupt routine */
> static irqreturn_t bcm_sysport_rx_isr(int irq, void *dev_id)
> {
> @@ -1152,7 +1111,6 @@ static irqreturn_t bcm_sysport_rx_isr(int irq, void *dev_id)
> continue;
>
> txr = &priv->tx_rings[ring];
> - txr->dim.event_ctr++;
>
> if (likely(napi_schedule_prep(&txr->napi))) {
> intrl2_0_mask_set(priv, ring_bit);
> @@ -1185,7 +1143,6 @@ static irqreturn_t bcm_sysport_tx_isr(int irq, void *dev_id)
> continue;
>
> txr = &priv->tx_rings[ring];
> - txr->dim.event_ctr++;
>
> if (likely(napi_schedule_prep(&txr->napi))) {
> intrl2_1_mask_set(priv, BIT(ring));
> @@ -1551,7 +1508,6 @@ static int bcm_sysport_init_tx_ring(struct bcm_sysport_priv *priv,
> reg |= (1 << index);
> tdma_writel(priv, reg, TDMA_TIER1_ARB_0_QUEUE_EN);
>
> - bcm_sysport_init_dim(&ring->dim, bcm_sysport_dim_tx_work);
> napi_enable(&ring->napi);
>
> netif_dbg(priv, hw, priv->netdev,
> @@ -1582,7 +1538,6 @@ static void bcm_sysport_fini_tx_ring(struct bcm_sysport_priv *priv,
> return;
>
> napi_disable(&ring->napi);
> - cancel_work_sync(&ring->dim.dim.work);
> netif_napi_del(&ring->napi);
>
> bcm_sysport_tx_clean(priv, ring);
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
> index e1c97d4a82b4..57e18ef8f206 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.h
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.h
> @@ -723,7 +723,6 @@ struct bcm_sysport_tx_ring {
> struct bcm_sysport_priv *priv; /* private context backpointer */
> unsigned long packets; /* packets statistics */
> unsigned long bytes; /* bytes statistics */
> - struct bcm_sysport_net_dim dim; /* Net DIM context */
> unsigned int switch_queue; /* switch port queue number */
> unsigned int switch_port; /* switch port queue number */
> bool inspect; /* inspect switch port and queue */
>
Reviewed-by: Tal Gilboa <talgi@mellanox.com>
^ permalink raw reply
* [PATCH iproute2-next 1/1] tc: enable json output for actions
From: Roman Mashak @ 2018-03-28 20:59 UTC (permalink / raw)
To: dsahern; +Cc: netdev, stephen, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
tc/m_action.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/tc/m_action.c b/tc/m_action.c
index 6c3049c7db88..8891659ae15a 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -364,6 +364,7 @@ tc_print_action(FILE *f, const struct rtattr *arg, unsigned short tot_acts)
if (tab_flush && NULL != tb[0] && NULL == tb[1])
return tc_print_action_flush(f, tb[0]);
+ open_json_object(NULL);
open_json_array(PRINT_JSON, "actions");
for (i = 0; i <= tot_acts; i++) {
if (tb[i]) {
@@ -379,6 +380,7 @@ tc_print_action(FILE *f, const struct rtattr *arg, unsigned short tot_acts)
}
close_json_array(PRINT_JSON, NULL);
+ close_json_object();
return 0;
}
@@ -405,7 +407,10 @@ int print_action(const struct sockaddr_nl *who,
if (tb[TCA_ROOT_COUNT])
tot_acts = RTA_DATA(tb[TCA_ROOT_COUNT]);
- fprintf(fp, "total acts %d\n", tot_acts ? *tot_acts:0);
+ open_json_object(NULL);
+ print_uint(PRINT_ANY, "total acts", "total acts %u",
+ tot_acts ? *tot_acts : 0);
+ close_json_object();
if (tb[TCA_ACT_TAB] == NULL) {
if (n->nlmsg_type != RTM_GETACTION)
fprintf(stderr, "print_action: NULL kind\n");
@@ -531,10 +536,16 @@ static int tc_action_gd(int cmd, unsigned int flags,
return 1;
}
- if (cmd == RTM_GETACTION && print_action(NULL, ans, stdout) < 0) {
- fprintf(stderr, "Dump terminated\n");
- free(ans);
- return 1;
+ if (cmd == RTM_GETACTION) {
+ new_json_obj(json);
+ ret = print_action(NULL, ans, stdout);
+ if (ret < 0) {
+ fprintf(stderr, "Dump terminated\n");
+ free(ans);
+ delete_json_obj();
+ return 1;
+ }
+ delete_json_obj();
}
free(ans);
@@ -675,7 +686,9 @@ static int tc_act_list_or_flush(int *argc_p, char ***argv_p, int event)
perror("Cannot send dump request");
return 1;
}
+ new_json_obj(json);
ret = rtnl_dump_filter(&rth, print_action, stdout);
+ delete_json_obj();
}
if (event == RTM_DELACTION) {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] sunrpc: remove incorrect HMAC request initialization
From: J . Bruce Fields @ 2018-03-28 21:12 UTC (permalink / raw)
To: Eric Biggers
Cc: Trond Myklebust, Anna Schumaker, Jeff Layton, linux-nfs, netdev,
linux-crypto, Herbert Xu, Michael Young, stable
In-Reply-To: <20180328175722.193355-1-ebiggers@google.com>
Applying, thanks!--b.
On Wed, Mar 28, 2018 at 10:57:22AM -0700, Eric Biggers wrote:
> make_checksum_hmac_md5() is allocating an HMAC transform and doing
> crypto API calls in the following order:
>
> crypto_ahash_init()
> crypto_ahash_setkey()
> crypto_ahash_digest()
>
> This is wrong because it makes no sense to init() the request before a
> key has been set, given that the initial state depends on the key. And
> digest() is short for init() + update() + final(), so in this case
> there's no need to explicitly call init() at all.
>
> Before commit 9fa68f620041 ("crypto: hash - prevent using keyed hashes
> without setting key") the extra init() had no real effect, at least for
> the software HMAC implementation. (There are also hardware drivers that
> implement HMAC-MD5, and it's not immediately obvious how gracefully they
> handle init() before setkey().) But now the crypto API detects this
> incorrect initialization and returns -ENOKEY. This is breaking NFS
> mounts in some cases.
>
> Fix it by removing the incorrect call to crypto_ahash_init().
>
> Reported-by: Michael Young <m.a.young@durham.ac.uk>
> Fixes: 9fa68f620041 ("crypto: hash - prevent using keyed hashes without setting key")
> Fixes: fffdaef2eb4a ("gss_krb5: Add support for rc4-hmac encryption")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> net/sunrpc/auth_gss/gss_krb5_crypto.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> index 12649c9fedab..8654494b4d0a 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_crypto.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> @@ -237,9 +237,6 @@ make_checksum_hmac_md5(struct krb5_ctx *kctx, char *header, int hdrlen,
>
> ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP, NULL, NULL);
>
> - err = crypto_ahash_init(req);
> - if (err)
> - goto out;
> err = crypto_ahash_setkey(hmac_md5, cksumkey, kctx->gk5e->keylength);
> if (err)
> goto out;
> --
> 2.17.0.rc1.321.gba9d0f2565-goog
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox