* Re: [PATCH net-next 3/3] strparser: Generalize strparser
From: John Fastabend @ 2017-10-19 20:18 UTC (permalink / raw)
To: Tom Herbert, Eric Dumazet
Cc: Tom Herbert, Linux Kernel Network Developers, Rohit Seth
In-Reply-To: <CALx6S3765-5oT6UtSQVts4KwWz3F3ub2vhDWO7YemYZPPM0EjA@mail.gmail.com>
On 10/19/2017 12:51 PM, Tom Herbert wrote:
> On Thu, Oct 19, 2017 at 10:42 AM, Eric Dumazet <eric.dumazet@gmail.com>
> wrote:
>
>> On Fri, 2017-07-28 at 16:22 -0700, Tom Herbert wrote:
>>> Generalize strparser from more than just being used in conjunction
>>> with read_sock. strparser will also be used in the send path with
>>> zero proxy. The primary change is to create strp_process function
>>> that performs the critical processing on skbs. The documentation
>>> is also updated to reflect the new uses.
>>>
>>> Signed-off-by: Tom Herbert <tom@quantonium.net>
>>> ---
>>> Documentation/networking/strparser.txt | 207 +++++++++++++++-------
>>> include/net/strparser.h | 119 +++++++------
>>> net/kcm/kcmproc.c | 34 ++--
>>> net/kcm/kcmsock.c | 38 ++--
>>> net/strparser/strparser.c | 313
>> ++++++++++++++++++++-------------
>>> 5 files changed, 424 insertions(+), 287 deletions(-)
>>
>> Just found this gem :
>>
>> static void strp_msg_timeout(unsigned long arg)
>> {
>> struct strparser *strp = (struct strparser *)arg;
>>
>> /* Message assembly timed out */
>> STRP_STATS_INCR(strp->stats.msg_timeouts);
>> strp->cb.lock(strp);
>> strp->cb.abort_parser(strp, ETIMEDOUT);
>> strp->cb.unlock(strp);
>> }
>>
>> static void strp_sock_lock(struct strparser *strp)
>> {
>> lock_sock(strp->sk);
>> }
>>
>> static void strp_sock_unlock(struct strparser *strp)
>> {
>> release_sock(strp->sk);
>> }
>>
>>
>> A timer runs from BH, and from this interrupt context it is absolutely
>> illegal to call lock_sock() ( and release_sock() )
>>
>> Please fix, thanks !
>>
>
> Nice catch! I'll fix it.
>
Can the lock/release be removed and the strp stopped bit can use atomics?
It looks like sk_error_report can be called without sock lock.
Thanks,
John
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] ipv6: remove from fib tree aged out RTF_CACHE dst
From: Wei Wang @ 2017-10-19 20:19 UTC (permalink / raw)
To: Paolo Abeni
Cc: Linux Kernel Network Developers, David S. Miller, Eric Dumazet,
Hannes Frederic Sowa, Martin KaFai Lau
In-Reply-To: <6440bf87fe967880ff90c1eb732081769f81f4a0.1508421311.git.pabeni@redhat.com>
On Thu, Oct 19, 2017 at 7:07 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> The commit 2b760fcf5cfb ("ipv6: hook up exception table to store
> dst cache") partially reverted the commit 1e2ea8ad37be ("ipv6: set
> dst.obsolete when a cached route has expired").
>
> As a result, RTF_CACHE dst referenced outside the fib tree will
> not be removed until the next sernum change; dst_check() does not
> fail on aged-out dst, and dst->__refcnt can't decrease: the aged
> out dst will stay valid for a potentially unlimited time after the
> timeout expiration.
>
> This change explicitly removes RTF_CACHE dst from the fib tree when
> aged out. The rt6_remove_exception() logic will then obsolete the
> dst and other entities will drop the related reference on next
> dst_check().
>
> pMTU exceptions are not aged-out, and are removed from the exception
> table only when the - usually considerably longer - ip6_rt_mtu_expires
> timeout expires.
>
> v1 -> v2:
> - do not touch dst.obsolete in rt6_remove_exception(), not needed
> v2 -> v3:
> - take care of pMTU exceptions, too
>
> Fixes: 2b760fcf5cfb ("ipv6: hook up exception table to store dst cache")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
Acked-by: Wei Wang <weiwan@google.com>
> net/ipv6/route.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 5c27313803d2..87a15cbd0e8b 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1575,7 +1575,13 @@ static void rt6_age_examine_exception(struct rt6_exception_bucket *bucket,
> {
> struct rt6_info *rt = rt6_ex->rt6i;
>
> - if (atomic_read(&rt->dst.__refcnt) == 1 &&
> + /* we are pruning and obsoleting aged-out and non gateway exceptions
> + * even if others have still references to them, so that on next
> + * dst_check() such references can be dropped.
> + * EXPIRES exceptions - e.g. pmtu-generated ones are pruned when
> + * expired, independently from their aging, as per RFC 8201 section 4
> + */
> + if (!(rt->rt6i_flags & RTF_EXPIRES) &&
> time_after_eq(now, rt->dst.lastuse + gc_args->timeout)) {
> RT6_TRACE("aging clone %p\n", rt);
> rt6_remove_exception(bucket, rt6_ex);
> @@ -1595,6 +1601,10 @@ static void rt6_age_examine_exception(struct rt6_exception_bucket *bucket,
> rt6_remove_exception(bucket, rt6_ex);
> return;
> }
> + } else if (__rt6_check_expired(rt)) {
> + RT6_TRACE("purging expired route %p\n", rt);
> + rt6_remove_exception(bucket, rt6_ex);
> + return;
> }
> gc_args->more++;
> }
> --
> 2.13.6
>
^ permalink raw reply
* RE: [PATCH net-next v2 1/6] devlink: Add permanent config parameter get/set operations
From: Yuval Mintz @ 2017-10-19 20:21 UTC (permalink / raw)
To: Steve Lin, netdev@vger.kernel.org
Cc: Jiri Pirko, davem@davemloft.net, michael.chan@broadcom.com,
linville@tuxdriver.com, gospo@broadcom.com
In-Reply-To: <1508440630-25830-2-git-send-email-steven.lin1@broadcom.com>
> Subject: [PATCH net-next v2 1/6] devlink: Add permanent config parameter
> get/set operations
>
> Add support for permanent config parameter get/set commands. Used
> for parameters held in NVRAM, persistent device configuration.
Given some of the attributes aren't Boolean, what about an API that
allows the user to learn of supported values per option?
Otherwise only way for configuring some of them would be trial & error.
>
> Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
> Acked-by: Andy Gospodarek <gospo@broadcom.com>
> ---
> include/net/devlink.h | 3 +
> include/uapi/linux/devlink.h | 11 ++
> net/core/devlink.c | 234
> +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 248 insertions(+)
>
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index b9654e1..bd64623 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -270,6 +270,9 @@ struct devlink_ops {
> int (*eswitch_inline_mode_set)(struct devlink *devlink, u8
> inline_mode);
> int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
> *p_encap_mode);
> int (*eswitch_encap_mode_set)(struct devlink *devlink, u8
> encap_mode);
> + int (*perm_config_get)(struct devlink *devlink, u32 param, u32
> *value);
> + int (*perm_config_set)(struct devlink *devlink, u32 param, u32
> value,
> + u8 *restart_reqd);
> };
>
> static inline void *devlink_priv(struct devlink *devlink)
> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
> index 0cbca96..47cc584 100644
> --- a/include/uapi/linux/devlink.h
> +++ b/include/uapi/linux/devlink.h
> @@ -70,6 +70,10 @@ enum devlink_command {
> DEVLINK_CMD_DPIPE_HEADERS_GET,
> DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
>
> + /* Permanent (NVRAM) device config get/set */
> + DEVLINK_CMD_PERM_CONFIG_GET,
> + DEVLINK_CMD_PERM_CONFIG_SET,
> +
> /* add new commands above here */
> __DEVLINK_CMD_MAX,
> DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
> @@ -202,6 +206,13 @@ enum devlink_attr {
>
> DEVLINK_ATTR_ESWITCH_ENCAP_MODE, /* u8 */
>
> + /* Permanent Configuration Parameters */
> + DEVLINK_ATTR_PERM_CONFIGS, /* nested */
> + DEVLINK_ATTR_PERM_CONFIG, /* nested */
> + DEVLINK_ATTR_PERM_CONFIG_PARAMETER, /* u32 */
> + DEVLINK_ATTR_PERM_CONFIG_VALUE, /*
> u32 */
> + DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, /* u8 */
> +
> /* add new attributes above here, update the policy in devlink.c */
>
> __DEVLINK_ATTR_MAX,
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 7d430c1..c2cc7c6 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -1566,6 +1566,224 @@ static int
> devlink_nl_cmd_eswitch_set_doit(struct sk_buff *skb,
> return 0;
> }
>
> +static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
> +
> +static int devlink_nl_single_param_get(struct sk_buff *msg,
> + struct devlink *devlink,
> + uint32_t param)
> +{
> + u32 value;
> + int err;
> + const struct devlink_ops *ops = devlink->ops;
> + struct nlattr *param_attr;
> +
> + err = ops->perm_config_get(devlink, param, &value);
> + if (err)
> + return err;
> +
> + param_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
> + nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_PARAMETER,
> param);
> + nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, value);
> + nla_nest_end(msg, param_attr);
> +
> + return 0;
> +}
> +
> +static int devlink_nl_config_get_fill(struct sk_buff *msg,
> + struct devlink *devlink,
> + enum devlink_command cmd,
> + struct genl_info *info)
> +{
> + void *hdr;
> + int err;
> + struct nlattr *attr;
> + int param_count = 0;
> + struct nlattr *cfgparam_attr;
> + int rem;
> + struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
> + u32 param;
> +
> + hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq,
> + &devlink_nl_family, 0, cmd);
> + if (!hdr) {
> + err = -EMSGSIZE;
> + goto nla_msg_failure;
> + }
> +
> + err = devlink_nl_put_handle(msg, devlink);
> + if (err)
> + goto nla_put_failure;
> +
> + if (!info->attrs[DEVLINK_ATTR_PERM_CONFIGS]) {
> + /* No configuration parameters */
> + goto nla_put_failure;
> + }
> +
> + cfgparam_attr = nla_nest_start(msg,
> DEVLINK_ATTR_PERM_CONFIGS);
> +
> + nla_for_each_nested(attr, info-
> >attrs[DEVLINK_ATTR_PERM_CONFIGS],
> + rem) {
Isn't it possible that a response for a single request for multiple ATTRs
wouldn't fit in a single message?
> + err = nla_parse_nested(tb, DEVLINK_ATTR_MAX, attr,
> + devlink_nl_policy, NULL);
> + if (err)
> + goto nla_nest_failure;
> + if (!tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER])
> + continue;
> +
> + param =
> nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER]);
> + err = devlink_nl_single_param_get(msg, devlink, param);
> + if (err)
> + goto nla_nest_failure;
> + param_count++;
Looks like an unused parameter
> + }
> +
> + nla_nest_end(msg, cfgparam_attr);
> +
> + genlmsg_end(msg, hdr);
> + return 0;
> +
> +nla_nest_failure:
> + nla_nest_cancel(msg, cfgparam_attr);
> +nla_put_failure:
> + genlmsg_cancel(msg, hdr);
> +nla_msg_failure:
> + return err;
> +}
> +
> +static int devlink_nl_cmd_perm_config_get_doit(struct sk_buff *skb,
> + struct genl_info *info)
> +{
> + struct devlink *devlink = info->user_ptr[0];
> + struct sk_buff *msg;
> + int err;
> +
> + if (!devlink->ops || !devlink->ops->perm_config_get)
> + return -EOPNOTSUPP;
> +
> + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + err = devlink_nl_config_get_fill(msg, devlink,
> +
> DEVLINK_CMD_PERM_CONFIG_GET, info);
> +
> + if (err) {
> + nlmsg_free(msg);
> + return err;
> + }
> +
> + return genlmsg_reply(msg, info);
> +}
> +
> +static int devlink_nl_single_param_set(struct sk_buff *msg,
> + struct devlink *devlink,
> + u32 param, u32 value)
> +{
> + u32 orig_value;
> + u8 need_restart;
> + int err;
> + const struct devlink_ops *ops = devlink->ops;
> + struct nlattr *cfgparam_attr;
> +
> + /* First get current value of parameter */
> + err = ops->perm_config_get(devlink, param, &orig_value);
> + if (err)
> + return err;
> +
> + /* Now set parameter */
> + err = ops->perm_config_set(devlink, param, value, &need_restart);
> + if (err)
> + return err;
> +
> + cfgparam_attr = nla_nest_start(msg,
> DEVLINK_ATTR_PERM_CONFIG);
> + /* Update restart reqd - if any param needs restart, should be set */
> + if (need_restart)
> + err = nla_put_u8(msg,
> +
> DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, 1);
> +
> + /* Since set was successful, write attr back to msg with orig val */
> + err = nla_put_u32(msg,
> DEVLINK_ATTR_PERM_CONFIG_PARAMETER, param);
> + err = nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE,
> orig_value);
> +
> + nla_nest_end(msg, cfgparam_attr);
> +
> + return 0;
> +}
> +
> +static int devlink_nl_cmd_perm_config_set_doit(struct sk_buff *skb,
> + struct genl_info *info)
> +{
> + struct devlink *devlink = info->user_ptr[0];
> + struct sk_buff *msg;
> + void *hdr;
> + struct nlattr *attr;
> + int rem;
> + int err;
> + u8 restart_reqd = 0;
> + struct nlattr *cfgparam_attr;
> + struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
> + u32 param;
> + u32 value;
> +
> + if (!devlink->ops || !devlink->ops->perm_config_get ||
> + !devlink->ops->perm_config_set)
> + return -EOPNOTSUPP;
> +
> + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq,
> + &devlink_nl_family, 0,
> DEVLINK_CMD_PERM_CONFIG_SET);
> + if (!hdr) {
> + err = -EMSGSIZE;
> + goto nla_msg_failure;
> + }
> +
> + err = devlink_nl_put_handle(msg, devlink);
> + if (err)
> + goto nla_put_failure;
> +
> + cfgparam_attr = nla_nest_start(msg,
> DEVLINK_ATTR_PERM_CONFIGS);
> +
> + nla_for_each_nested(attr, info-
> >attrs[DEVLINK_ATTR_PERM_CONFIGS], rem) {
> + err = nla_parse_nested(tb, DEVLINK_ATTR_MAX, attr,
> + devlink_nl_policy, NULL);
> + if (err)
> + goto nla_nest_failure;
> +
> + if (!tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER] ||
> + !tb[DEVLINK_ATTR_PERM_CONFIG_VALUE])
> + continue;
> +
> + param =
> nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER]);
> + value =
> nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_VALUE]);
> + err = devlink_nl_single_param_set(msg, devlink, param,
> + value);
> + if (err)
> + goto nla_nest_failure;
> + }
> +
> + nla_nest_end(msg, cfgparam_attr);
> +
> + if (restart_reqd) {
Doesn't seem like you're ever setting it.
A leftover from when this was an attribute of the configs instead
of per-config perhaps?
> + err = nla_put_u8(msg,
> DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED,
> + restart_reqd);
> + if (err)
> + goto nla_put_failure;
> + }
> +
> + genlmsg_end(msg, hdr);
> + return genlmsg_reply(msg, info);
> +
> +nla_nest_failure:
> + nla_nest_cancel(msg, cfgparam_attr);
> +nla_put_failure:
> + genlmsg_cancel(msg, hdr);
> +nla_msg_failure:
> + return err;
> +}
> +
> int devlink_dpipe_match_put(struct sk_buff *skb,
> struct devlink_dpipe_match *match)
> {
> @@ -2291,6 +2509,8 @@ static const struct nla_policy
> devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
> [DEVLINK_ATTR_ESWITCH_ENCAP_MODE] = { .type = NLA_U8 },
> [DEVLINK_ATTR_DPIPE_TABLE_NAME] = { .type = NLA_NUL_STRING
> },
> [DEVLINK_ATTR_DPIPE_TABLE_COUNTERS_ENABLED] = { .type =
> NLA_U8 },
> + [DEVLINK_ATTR_PERM_CONFIG_PARAMETER] = { .type = NLA_U32
> },
> + [DEVLINK_ATTR_PERM_CONFIG_VALUE] = { .type = NLA_U32 },
> };
>
> static const struct genl_ops devlink_nl_ops[] = {
> @@ -2451,6 +2671,20 @@ static const struct genl_ops devlink_nl_ops[] = {
> .flags = GENL_ADMIN_PERM,
> .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
> },
> + {
> + .cmd = DEVLINK_CMD_PERM_CONFIG_GET,
> + .doit = devlink_nl_cmd_perm_config_get_doit,
> + .policy = devlink_nl_policy,
> + .flags = GENL_ADMIN_PERM,
> + .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
> + },
> + {
> + .cmd = DEVLINK_CMD_PERM_CONFIG_SET,
> + .doit = devlink_nl_cmd_perm_config_set_doit,
> + .policy = devlink_nl_policy,
> + .flags = GENL_ADMIN_PERM,
> + .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
> + },
> };
>
> static struct genl_family devlink_nl_family __ro_after_init = {
> --
> 2.7.4
^ permalink raw reply
* [PATCH] timer: Provide wrappers safe for use with LOCKDEP
From: Kees Cook @ 2017-10-19 20:28 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Craig Gallek, Eric Dumazet, David S. Miller, netdev, linux-kernel
Under LOCKDEP, the timer lock_class_key (set up in __setup_timer) needs
to be tied to the caller's context, so an inline for timer_setup()
won't work. We do, however, want to keep the inline version around for
argument type checking, though, so this provides macro wrappers in the
LOCKDEP case.
This fixes the case of different timers sharing the same LOCKDEP instance,
and producing a false positive warning:
[ 580.840858] ======================================================
[ 580.842299] WARNING: possible circular locking dependency detected
[ 580.843684] 4.14.0-rc4+ #17 Not tainted
[ 580.844554] ------------------------------------------------------
[ 580.845945] swapper/9/0 is trying to acquire lock:
[ 580.847024] (slock-AF_INET){+.-.}, at: [<ffffffff84ea4c34>] tcp_write_timer+0x24/0xd0
[ 580.848834]
but task is already holding lock:
[ 580.850107] ((timer)#2){+.-.}, at: [<ffffffff846df7c0>] call_timer_fn+0x0/0x300
[ 580.851663]
which lock already depends on the new lock.
[ 580.853439]
the existing dependency chain (in reverse order) is:
[ 580.855311]
-> #1 ((timer)#2){+.-.}:
[ 580.856538] __lock_acquire+0x114d/0x11a0
[ 580.857506] lock_acquire+0xb0/0x1d0
[ 580.858373] del_timer_sync+0x3c/0xb0
[ 580.859260] inet_csk_reqsk_queue_drop+0x7f/0x1b0
...
-> #0 (slock-AF_INET){+.-.}:
[ 580.884980] check_prev_add+0x666/0x700
[ 580.885790] __lock_acquire+0x114d/0x11a0
[ 580.886575] lock_acquire+0xb0/0x1d0
[ 580.887289] _raw_spin_lock+0x2c/0x40
[ 580.888021] tcp_write_timer+0x24/0xd0
...
[ 580.900055] Possible unsafe locking scenario:
[ 580.901043] CPU0 CPU1
[ 580.901797] ---- ----
[ 580.902540] lock((timer)#2);
[ 580.903046] lock(slock-AF_INET);
[ 580.904006] lock((timer)#2);
[ 580.904915] lock(slock-AF_INET);
[ 580.905502]
In this report, del_timer_sync() is from:
inet_csk_reqsk_queue_drop()
reqsk_queue_unlink()
del_timer_sync(&req->rsk_timer)
but tcp_write_timer()'s timer is attached to icsk_retransmit_timer. Both
had the same lock_class_key, since they were using timer_setup(). Switching
to a macro allows for a separate context, avoiding the false positive.
Reported-by: Craig Gallek <cgallek@google.com>
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
This means that all the trees taking conversions are at risk for harmless
LOCKDEP warnings until this is landed. Putting this in v4.14 doesn't make
sense since no one is going to rebase their trees. Also, it depends on
the timer_setup_on_stack() function that was added in -next in timers/core.
Any thoughts on the best way to deal with this?
---
include/linux/timer.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/include/linux/timer.h b/include/linux/timer.h
index 10685c33e679..09950482309b 100644
--- a/include/linux/timer.h
+++ b/include/linux/timer.h
@@ -150,6 +150,7 @@ static inline void init_timer_on_stack_key(struct timer_list *timer,
#define TIMER_DATA_TYPE unsigned long
#define TIMER_FUNC_TYPE void (*)(TIMER_DATA_TYPE)
+#ifndef CONFIG_LOCKDEP
static inline void timer_setup(struct timer_list *timer,
void (*callback)(struct timer_list *),
unsigned int flags)
@@ -165,6 +166,19 @@ static inline void timer_setup_on_stack(struct timer_list *timer,
__setup_timer_on_stack(timer, (TIMER_FUNC_TYPE)callback,
(TIMER_DATA_TYPE)timer, flags);
}
+#else
+/*
+ * Under LOCKDEP, the timer lock_class_key (set up in __init_timer) needs
+ * to be tied to the caller's context, so an inline (above) won't work. We
+ * do want to keep the inline for argument type checking, though.
+ */
+# define timer_setup(timer, callback, flags) \
+ __setup_timer(timer, (TIMER_FUNC_TYPE)callback, \
+ (TIMER_DATA_TYPE)timer, flags)
+# define timer_setup_on_stack(timer, callback, flags) \
+ __setup_timer_on_stack(timer, (TIMER_FUNC_TYPE)callback,\
+ (TIMER_DATA_TYPE)timer, flags)
+#endif
#define from_timer(var, callback_timer, timer_fieldname) \
container_of(callback_timer, typeof(*var), timer_fieldname)
--
2.7.4
--
Kees Cook
Pixel Security
^ permalink raw reply related
* RE: [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
From: Yuval Mintz @ 2017-10-19 20:32 UTC (permalink / raw)
To: Steve Lin, netdev@vger.kernel.org
Cc: Jiri Pirko, davem@davemloft.net, michael.chan@broadcom.com,
linville@tuxdriver.com, gospo@broadcom.com
In-Reply-To: <1508440630-25830-6-git-send-email-steven.lin1@broadcom.com>
> Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF permanent
> config
> parameter. Defines number of MSI-X vectors allocated per VF.
> Value is permanent (stored in NVRAM), so becomes the new default
> value for this device.
Sounds like you're having this enforce the same configuration for all child VFs.
>
> Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
> Acked-by: Andy Gospodarek <gospo@broadcom.com>
> ---
> include/uapi/linux/devlink.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
> index 8ad6c63..ef163b6 100644
> --- a/include/uapi/linux/devlink.h
> +++ b/include/uapi/linux/devlink.h
> @@ -260,6 +260,7 @@ enum devlink_perm_config_param {
> DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
> DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
> DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT,
> + DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF,
> };
>
> #endif /* _UAPI_LINUX_DEVLINK_H_ */
> --
> 2.7.4
^ permalink raw reply
* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Richard Cochran @ 2017-10-19 20:39 UTC (permalink / raw)
To: Jesus Sanchez-Palencia
Cc: Vinicius Costa Gomes, netdev, jhs, xiyou.wangcong, jiri,
intel-wired-lan, andre.guedes, ivan.briano, boon.leong.ong,
Levi Pearson, Henrik Austad
In-Reply-To: <62f3eae4-bf6a-1475-936f-5011c9ff381e@intel.com>
On Wed, Oct 18, 2017 at 03:37:35PM -0700, Jesus Sanchez-Palencia wrote:
> I also did some tests with when you don't set valid launch times, but here using
> your idea from above, so with the driver calculating a valid launch time (i.e.
> current NIC time + X ns, varying X across tests) for packets that didn't have it
> set by the user, and I wasn't too happy with its reliability. It could
> definitely be improved, but it has left me wondering: instead, what about
> documenting that if you enable TXTIME, then you *must* provide a valid Launch
> time for all packets on traffic classes that are affected?
If txtime is enabled, then CBS is pointless because the txtime already
specifies the bandwidth implicitly.
The problem is when one program uses txtime and another uses CBS, then
the CBS user will experience really wrong performance.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net-next v2 2/6] devlink: Adding SR-IOV enablement NVRAM config param
From: Steve Lin @ 2017-10-19 20:40 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Netdev List, Jiri Pirko, David S . Miller, Michael Chan,
John Linville, Andy Gospodarek
In-Reply-To: <20171019193346.GE1978@nanopsycho.orion>
On Thu, Oct 19, 2017 at 3:33 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Oct 19, 2017 at 09:17:06PM CEST, steven.lin1@broadcom.com wrote:
>>Adding DEVLINK_PERM_CONFIG_SRIOV_ENABLED permanent config
>>parameter. If value is 1, SR-IOV is enabled. If value is 0,
>>SR-IOV is disabled on this device. Value is permanent (stored
>>in NVRAM), so becomes the new default value for this device.
>>
>>Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>>Acked-by: Andy Gospodarek <gospo@broadcom.com>
>>---
>> include/uapi/linux/devlink.h | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>>index 47cc584..2640203 100644
>>--- a/include/uapi/linux/devlink.h
>>+++ b/include/uapi/linux/devlink.h
>>@@ -255,4 +255,9 @@ enum devlink_dpipe_header_id {
>> DEVLINK_DPIPE_HEADER_IPV6,
>> };
>>
>>+/* Permanent (NVRAM) config parameters */
>
> We need the decription here in the header as well. Commit message alone
> is no good for this.
>
> Also, there should not be mention of "NVRAM". It is up to the device
> implementation where is stores the value.
>
>
Will do, I'll add descriptions in the header file and reword as
requested. Thanks!
^ permalink raw reply
* Re: [PATCH net-next v2 6/6] bnxt: Add devlink support for config get/set
From: Steve Lin @ 2017-10-19 20:40 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Netdev List, Jiri Pirko, David S . Miller, Michael Chan,
John Linville, Andy Gospodarek
In-Reply-To: <20171019193514.GF1978@nanopsycho.orion>
On Thu, Oct 19, 2017 at 3:35 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Oct 19, 2017 at 09:17:10PM CEST, steven.lin1@broadcom.com wrote:
>>Implements get and set of configuration parameters using new devlink
>>config get/set API.
>
> Please split this patch too. One to introduce the infra, one per each
> config option.
Ok, will do in v3, thanks.
^ permalink raw reply
* Re: [PATCH RFC V1 net-next 0/6] Time based packet transmission
From: Richard Cochran @ 2017-10-19 20:44 UTC (permalink / raw)
To: Jesus Sanchez-Palencia
Cc: Richard Cochran, netdev, linux-kernel, intel-wired-lan,
Andre Guedes, Anna-Maria Gleixner, David Miller, Henrik Austad,
John Stultz, Thomas Gleixner, Vinicius Costa Gomes, Briano, Ivan,
Levi Pearson
In-Reply-To: <743a4550-7344-5e73-bf6d-6ec368263ad9@intel.com>
On Wed, Oct 18, 2017 at 03:18:55PM -0700, Jesus Sanchez-Palencia wrote:
> This is great. Just out of curiosity, were you using vlans on your tests?
No, just raw packets. VLAN tags could be added trivially (in the
program), but that naturally avoids the kernel's VLAN code.
> I might try to reproduce them soon. I would appreciate if you could provide me
> with the scripts, please.
Ok, will do.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH 31/58] isdn/gigaset: Use kzalloc instead of open-coded field zeroing
From: Paul Bolle @ 2017-10-19 20:46 UTC (permalink / raw)
To: Kees Cook, David S. Miller
Cc: Karsten Keil, Johan Hovold, gigaset307x-common, netdev,
Thomas Gleixner, linux-kernel
In-Reply-To: <1508200182-104605-32-git-send-email-keescook@chromium.org>
On Mon, 2017-10-16 at 17:29 -0700, Kees Cook wrote:
> This replaces a kmalloc followed by a bunch of per-field zeroing with a
> single kzalloc call, reducing the lines of code.
Acked-by: Paul Bolle <pebolle@tiscali.nl>
Thanks,
Paul Bolle
^ permalink raw reply
* [PATCH] ethtool: Do not return error code if no changes were attempted.
From: greearb @ 2017-10-19 20:58 UTC (permalink / raw)
To: netdev; +Cc: linville, Ben Greear
From: Ben Greear <greearb@candelatech.com>
This makes it easier to properly handle errors when calling this
from scripts, etc.
Old behaviour:
combined unmodified, ignoring
no channel parameters changed, aborting
current values: tx 0 rx 0 other 1 combined 1
[root@lf0313-6477 ethtool]# echo $?
1
New behaviour:
combined unmodified, ignoring
no channel parameters changed.
current values: tx 0 rx 0 other 1 combined 1
[root@lf0313-6477 ethtool]# echo $?
0
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
ethtool.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index ad18704..224efdb 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1994,12 +1994,12 @@ static int do_schannels(struct cmd_context *ctx)
&changed);
if (!changed) {
- fprintf(stderr, "no channel parameters changed, aborting\n");
+ fprintf(stderr, "no channel parameters changed.\n");
fprintf(stderr, "current values: tx %u rx %u other %u"
" combined %u\n", echannels.rx_count,
echannels.tx_count, echannels.other_count,
echannels.combined_count);
- return 1;
+ return 0;
}
echannels.cmd = ETHTOOL_SCHANNELS;
--
2.7.5
^ permalink raw reply related
* [PATCH v2] ethtool: Do not return error code if no changes were attempted.
From: greearb @ 2017-10-19 21:01 UTC (permalink / raw)
To: netdev; +Cc: linville, Ben Greear
From: Ben Greear <greearb@candelatech.com>
This makes it easier to properly handle errors when calling this
from scripts, etc.
Old behaviour:
$ ethtool -L eth3 combined 1
combined unmodified, ignoring
no channel parameters changed, aborting
current values: tx 0 rx 0 other 1 combined 1
[root@lf0313-6477 ethtool]# echo $?
1
New behaviour:
$ ./ethtool -L eth3 combined 1
combined unmodified, ignoring
no channel parameters changed.
current values: tx 0 rx 0 other 1 combined 1
[root@lf0313-6477 ethtool]# echo $?
0
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
v2: Fix comments, the # ./ethtool .... was removed by git of course.
ethtool.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index ad18704..224efdb 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1994,12 +1994,12 @@ static int do_schannels(struct cmd_context *ctx)
&changed);
if (!changed) {
- fprintf(stderr, "no channel parameters changed, aborting\n");
+ fprintf(stderr, "no channel parameters changed.\n");
fprintf(stderr, "current values: tx %u rx %u other %u"
" combined %u\n", echannels.rx_count,
echannels.tx_count, echannels.other_count,
echannels.combined_count);
- return 1;
+ return 0;
}
echannels.cmd = ETHTOOL_SCHANNELS;
--
2.7.5
^ permalink raw reply related
* Re: [PATCH net-next v3 1/2] ipv6: start fib6 gc on RTF_CACHE dst creation
From: Martin KaFai Lau @ 2017-10-19 21:01 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, David S. Miller, Wei Wang, Eric Dumazet,
Hannes Frederic Sowa
In-Reply-To: <f699f29b3fb7675fdfe5d85ab39e153c147c12f8.1508421311.git.pabeni@redhat.com>
On Thu, Oct 19, 2017 at 02:07:10PM +0000, Paolo Abeni wrote:
> After the commit 2b760fcf5cfb ("ipv6: hook up exception table
> to store dst cache"), the fib6 gc is not started after the
> creation of a RTF_CACHE via a redirect or pmtu update, since
> fib6_add() isn't invoked anymore for such dsts.
>
> We need the fib6 gc to run periodically to clean the RTF_CACHE,
> or the dst will stay there forever.
>
> Fix it by explicitly calling fib6_force_start_gc() on successful
> exception creation. gc_args->more accounting will ensure that
> the gc timer will run for whatever time needed to properly
> clean the table.
>
> v2 -> v3:
> - clarified the commit message
>
> Fixes: 2b760fcf5cfb ("ipv6: hook up exception table to store dst cache")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] ipv6: remove from fib tree aged out RTF_CACHE dst
From: Martin KaFai Lau @ 2017-10-19 21:02 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, David S. Miller, Wei Wang, Eric Dumazet,
Hannes Frederic Sowa
In-Reply-To: <6440bf87fe967880ff90c1eb732081769f81f4a0.1508421311.git.pabeni@redhat.com>
On Thu, Oct 19, 2017 at 02:07:11PM +0000, Paolo Abeni wrote:
> The commit 2b760fcf5cfb ("ipv6: hook up exception table to store
> dst cache") partially reverted the commit 1e2ea8ad37be ("ipv6: set
> dst.obsolete when a cached route has expired").
>
> As a result, RTF_CACHE dst referenced outside the fib tree will
> not be removed until the next sernum change; dst_check() does not
> fail on aged-out dst, and dst->__refcnt can't decrease: the aged
> out dst will stay valid for a potentially unlimited time after the
> timeout expiration.
>
> This change explicitly removes RTF_CACHE dst from the fib tree when
> aged out. The rt6_remove_exception() logic will then obsolete the
> dst and other entities will drop the related reference on next
> dst_check().
>
> pMTU exceptions are not aged-out, and are removed from the exception
> table only when the - usually considerably longer - ip6_rt_mtu_expires
> timeout expires.
>
> v1 -> v2:
> - do not touch dst.obsolete in rt6_remove_exception(), not needed
> v2 -> v3:
> - take care of pMTU exceptions, too
>
> Fixes: 2b760fcf5cfb ("ipv6: hook up exception table to store dst cache")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply
* Re: [PATCH 32/58] isdn/gigaset: Convert timers to use timer_setup()
From: Paul Bolle @ 2017-10-19 21:03 UTC (permalink / raw)
To: Kees Cook, David S. Miller
Cc: Karsten Keil, Johan Hovold, gigaset307x-common, netdev,
Thomas Gleixner, linux-kernel
In-Reply-To: <1508200182-104605-33-git-send-email-keescook@chromium.org>
On Mon, 2017-10-16 at 17:29 -0700, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
Acked-by: Paul Bolle <pebolle@tiscali.nl>
For the record: this patch made me nervous but survived the rigorous testing I
threw at it. (Ie, dialing up using bas_gigaset and downloading almost 20 MB in
just over an hour. Whoot! That's more than good enough to ack this patch.)
There was some cleanup I had in mind to make this patch more straightforward.
But that can wait until someone finds a way to hit an issue with this patch.
We'll see.
Thanks,
Paul Bolle
^ permalink raw reply
* Re: [PATCH] net: rxrpc: mark expected switch fall-throughs
From: David Howells @ 2017-10-19 21:16 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: dhowells, David S. Miller, linux-afs, netdev, linux-kernel
In-Reply-To: <20171019185016.GA18770@embeddedor.com>
Gustavo A. R. Silva <garsilva@embeddedor.com> wrote:
> + /* fall through */
All new comments in rxrpc should begin with a capital letter; I'm switching to
this as I modify the lines with comments on. Fix checkpatch or gcc or
whatever takes -Wimplicit-fallthrough to stop being silly.
> - /* Fall through */
> -
> + /* fall through */
No. Firstly, it should be 'F'; secondly, don't remove the blank line - it's
there for a reason.
> + /* fall through */
Capital 'F'.
> -
> + /* fall through */
Don't remove the blank line. Capital 'F'.
> + /* fall through */
Capital 'F'.
David
^ permalink raw reply
* Re: [PATCH 32/58] isdn/gigaset: Convert timers to use timer_setup()
From: Paul Bolle @ 2017-10-19 21:20 UTC (permalink / raw)
To: Kees Cook, David S. Miller
Cc: Karsten Keil, Johan Hovold, gigaset307x-common, netdev,
Thomas Gleixner, linux-kernel
In-Reply-To: <1508447002.2274.14.camel@tiscali.nl>
On Thu, 2017-10-19 at 23:03 +0200, Paul Bolle wrote:
> On Mon, 2017-10-16 at 17:29 -0700, Kees Cook wrote:
> > In preparation for unconditionally passing the struct timer_list pointer to
> > all timer callbacks, switch to using the new timer_setup() and from_timer()
> > to pass the timer pointer explicitly.
>
> Acked-by: Paul Bolle <pebolle@tiscali.nl>
I have to take this back, sorry!
> For the record: this patch made me nervous but survived the rigorous testing I
> threw at it. (Ie, dialing up using bas_gigaset and downloading almost 20 MB in
> just over an hour. Whoot! That's more than good enough to ack this patch.)
>
> There was some cleanup I had in mind to make this patch more straightforward.
> But that can wait until someone finds a way to hit an issue with this patch.
> We'll see.
That someone turns out to be me, doing "modprobe -r bas_gigaset":
<1>[30143.538135] BUG: unable to handle kernel NULL pointer dereference at 000001e9
<1>[30143.538154] IP: mutex_lock+0x19/0x30
<6>[30143.538157] *pde = 00000000
<5>[30143.538162] Oops: 0002 [#1] SMP
<5>[30143.538165] Modules linked in: bas_gigaset(OE-) gigaset vfat fat uas usb_storage ppp_deflate bsd_comp ppp_synctty ppp_generic slhc capi kernelcapi fuse xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack libcrc32c iptable_mangle iptable_raw iptable_security ebtable_filter ebtables ip6table_filter ip6_tables sunrpc snd_intel8x0 snd_ac97_codec gpio_ich ac97_bus ppdev iTCO_wdt iTCO_vendor_support lpc_ich snd_seq snd_seq_device snd_pcm pcspkr i
2c_i801 thinkpad_acpi
<5>[30143.538228] snd_timer snd irda(C) soundcore parport_pc rfkill parport acpi_cpufreq i915 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops sdhci_pci drm tg3 sdhci mmc_core ata_generic serio_raw yenta_socket ptp pata_acpi pps_core video [last unloaded: gigaset]
<0>[30143.538257] CPU: 0 PID: 22085 Comm: modprobe Tainted: G C OE 4.14.0-0.rc4.1.local0.fc26.i686 #1
<0>[30143.538260] Hardware name: IBM 2525FAG/2525FAG, BIOS 74ET64WW (2.09 ) 12/14/2006
<0>[30143.538263] task: f6671100 task.stack: c77c6000
<5>[30143.538267] EIP: mutex_lock+0x19/0x30
<5>[30143.538269] EFLAGS: 00010246 CPU: 0
<5>[30143.538272] EAX: 00000000 EBX: 000001e9 ECX: 00000001 EDX: f6671100
<5>[30143.538275] ESI: 000001e9 EDI: 011c8f98 EBP: c77c7ef4 ESP: c77c7ef0
<5>[30143.538278] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
<5>[30143.538281] CR0: 80050033 CR2: 000001e9 CR3: 16d2d000 CR4: 000006d0
<0>[30143.538284] Call Trace:
<0>[30143.538300] gigaset_shutdown+0x28/0x130 [gigaset]
<0>[30143.538307] ? find_module_all+0x62/0x80
<0>[30143.538314] bas_gigaset_exit+0x31/0x1077 [bas_gigaset]
<0>[30143.538319] SyS_delete_module+0x19c/0x240
<0>[30143.538325] ? ____fput+0xd/0x10
<0>[30143.538330] do_fast_syscall_32+0x71/0x1a0
<0>[30143.538335] entry_SYSENTER_32+0x4e/0x7c
<5>[30143.538337] EIP: 0xb7fb5cd9
<5>[30143.538339] EFLAGS: 00000206 CPU: 0
<5>[30143.538342] EAX: ffffffda EBX: 011c8fd4 ECX: 00000800 EDX: 011c8fd4
<5>[30143.538345] ESI: 011c8f98 EDI: 011c8f98 EBP: 011c8fd4 ESP: bf8278f8
<5>[30143.538348] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b
<0>[30143.538351] Code: 8e fb ff ff 5d c3 8d b6 00 00 00 00 8d bf 00 00 00 00 3e 8d 74 26 00 55 89 e5 53 89 c3 e8 60 e7 ff ff 64 8b 15 40 c9 82 c7 31 c0 <3e> 0f b1 13 85 c0 74 07 89 d8 e8 b8 ff ff ff 5b 5d c3 90 8d 74
<0>[30143.538399] EIP: mutex_lock+0x19/0x30 SS:ESP: 0068:c77c7ef0
<5>[30143.538402] CR2: 00000000000001e9
<4>[30143.538406] ---[ end trace 3e60af64adfe7e14 ]---
I'll have to ask for some patience so I can find out what's going on. (Very
likely: using urb->context beyond it's lifetime.) Expect something by early
next week. Is that OK with you?
Sorry for the noise,
Paul Bolle
^ permalink raw reply
* Re: [PATCH] net: rxrpc: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2017-10-19 21:23 UTC (permalink / raw)
To: David Howells; +Cc: David S. Miller, linux-afs, netdev, linux-kernel
In-Reply-To: <32524.1508447816@warthog.procyon.org.uk>
Quoting David Howells <dhowells@redhat.com>:
> Gustavo A. R. Silva <garsilva@embeddedor.com> wrote:
>
>> + /* fall through */
>
> All new comments in rxrpc should begin with a capital letter; I'm
> switching to
> this as I modify the lines with comments on. Fix checkpatch or gcc or
> whatever takes -Wimplicit-fallthrough to stop being silly.
>
>> - /* Fall through */
>> -
>> + /* fall through */
>
> No. Firstly, it should be 'F'; secondly, don't remove the blank line - it's
> there for a reason.
>
What is the reason?
>> + /* fall through */
>
> Capital 'F'.
>
>> -
>> + /* fall through */
>
> Don't remove the blank line. Capital 'F'.
>
>> + /* fall through */
>
> Capital 'F'.
>
> David
Thanks
--
Gustavo A. R. Silva
^ permalink raw reply
* [PATCH] net: sched: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2017-10-19 21:28 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller
Cc: netdev, linux-kernel, Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify if the actual intention of the code is to fall through.
net/sched/sch_cbq.c | 1 +
net/sched/sch_drr.c | 1 +
net/sched/sch_fq_codel.c | 1 +
net/sched/sch_hfsc.c | 1 +
net/sched/sch_htb.c | 1 +
net/sched/sch_multiq.c | 1 +
net/sched/sch_prio.c | 1 +
net/sched/sch_qfq.c | 1 +
net/sched/sch_sfb.c | 1 +
net/sched/sch_sfq.c | 1 +
10 files changed, 10 insertions(+)
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index c3b92d6..6361be7 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -255,6 +255,7 @@ cbq_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
case TC_ACT_STOLEN:
case TC_ACT_TRAP:
*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
+ /* fall through */
case TC_ACT_SHOT:
return NULL;
case TC_ACT_RECLASSIFY:
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index 753dc7a..5bbcef3 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -321,6 +321,7 @@ static struct drr_class *drr_classify(struct sk_buff *skb, struct Qdisc *sch,
case TC_ACT_STOLEN:
case TC_ACT_TRAP:
*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
+ /* fall through */
case TC_ACT_SHOT:
return NULL;
}
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index 3c40ede..0305d79 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -105,6 +105,7 @@ static unsigned int fq_codel_classify(struct sk_buff *skb, struct Qdisc *sch,
case TC_ACT_QUEUED:
case TC_ACT_TRAP:
*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
+ /* fall through */
case TC_ACT_SHOT:
return 0;
}
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index a692184..d04068a 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1144,6 +1144,7 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
case TC_ACT_STOLEN:
case TC_ACT_TRAP:
*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
+ /* fall through */
case TC_ACT_SHOT:
return NULL;
}
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 57be73c..fa03807 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -244,6 +244,7 @@ static struct htb_class *htb_classify(struct sk_buff *skb, struct Qdisc *sch,
case TC_ACT_STOLEN:
case TC_ACT_TRAP:
*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
+ /* fall through */
case TC_ACT_SHOT:
return NULL;
}
diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c
index 31e0a28..0122163 100644
--- a/net/sched/sch_multiq.c
+++ b/net/sched/sch_multiq.c
@@ -54,6 +54,7 @@ multiq_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
case TC_ACT_QUEUED:
case TC_ACT_TRAP:
*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
+ /* fall through */
case TC_ACT_SHOT:
return NULL;
}
diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
index 95fad34..2c79559 100644
--- a/net/sched/sch_prio.c
+++ b/net/sched/sch_prio.c
@@ -50,6 +50,7 @@ prio_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
case TC_ACT_QUEUED:
case TC_ACT_TRAP:
*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
+ /* fall through */
case TC_ACT_SHOT:
return NULL;
}
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 8694c7b..6962b37 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -709,6 +709,7 @@ static struct qfq_class *qfq_classify(struct sk_buff *skb, struct Qdisc *sch,
case TC_ACT_STOLEN:
case TC_ACT_TRAP:
*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
+ /* fall through */
case TC_ACT_SHOT:
return NULL;
}
diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c
index 487d375..0678deb 100644
--- a/net/sched/sch_sfb.c
+++ b/net/sched/sch_sfb.c
@@ -268,6 +268,7 @@ static bool sfb_classify(struct sk_buff *skb, struct tcf_proto *fl,
case TC_ACT_QUEUED:
case TC_ACT_TRAP:
*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
+ /* fall through */
case TC_ACT_SHOT:
return false;
}
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 123a53a..6d4b59d 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -189,6 +189,7 @@ static unsigned int sfq_classify(struct sk_buff *skb, struct Qdisc *sch,
case TC_ACT_QUEUED:
case TC_ACT_TRAP:
*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
+ /* fall through */
case TC_ACT_SHOT:
return 0;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 32/58] isdn/gigaset: Convert timers to use timer_setup()
From: Thomas Gleixner @ 2017-10-19 21:31 UTC (permalink / raw)
To: Paul Bolle
Cc: Kees Cook, David S. Miller, Karsten Keil, Johan Hovold,
gigaset307x-common, netdev, linux-kernel
In-Reply-To: <1508448028.2274.21.camel@tiscali.nl>
On Thu, 19 Oct 2017, Paul Bolle wrote:
> On Thu, 2017-10-19 at 23:03 +0200, Paul Bolle wrote:
> > On Mon, 2017-10-16 at 17:29 -0700, Kees Cook wrote:
> > > In preparation for unconditionally passing the struct timer_list pointer to
> > > all timer callbacks, switch to using the new timer_setup() and from_timer()
> > > to pass the timer pointer explicitly.
> >
> > Acked-by: Paul Bolle <pebolle@tiscali.nl>
>
> I have to take this back, sorry!
>
> > For the record: this patch made me nervous but survived the rigorous testing I
> > threw at it. (Ie, dialing up using bas_gigaset and downloading almost 20 MB in
> > just over an hour. Whoot! That's more than good enough to ack this patch.)
> >
> > There was some cleanup I had in mind to make this patch more straightforward.
> > But that can wait until someone finds a way to hit an issue with this patch.
> > We'll see.
>
> That someone turns out to be me, doing "modprobe -r bas_gigaset":
>
> <1>[30143.538135] BUG: unable to handle kernel NULL pointer dereference at 000001e9
> <1>[30143.538154] IP: mutex_lock+0x19/0x30
> <0>[30143.538300] gigaset_shutdown+0x28/0x130 [gigaset]
> <0>[30143.538307] ? find_module_all+0x62/0x80
> <0>[30143.538314] bas_gigaset_exit+0x31/0x1077 [bas_gigaset]
bas_gigaset_exit()
{
for (i = 0; i < driver->minors; i++) {
if (gigaset_shutdown(driver->cs + i) < 0)
gigaset_shutdown(cs)
{
mutex_lock(&cs->mutex); <-------- Explodes here
So driver->cs + i is invalid. No idea how that might be related to that
timer conversion patch, but ....
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH 32/58] isdn/gigaset: Convert timers to use timer_setup()
From: Kees Cook @ 2017-10-19 21:31 UTC (permalink / raw)
To: Paul Bolle
Cc: David S. Miller, Karsten Keil, Johan Hovold, gigaset307x-common,
Network Development, Thomas Gleixner, LKML
In-Reply-To: <1508448028.2274.21.camel@tiscali.nl>
On Thu, Oct 19, 2017 at 2:20 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
> On Thu, 2017-10-19 at 23:03 +0200, Paul Bolle wrote:
>> On Mon, 2017-10-16 at 17:29 -0700, Kees Cook wrote:
>> > In preparation for unconditionally passing the struct timer_list pointer to
>> > all timer callbacks, switch to using the new timer_setup() and from_timer()
>> > to pass the timer pointer explicitly.
>>
>> Acked-by: Paul Bolle <pebolle@tiscali.nl>
>
> I have to take this back, sorry!
>
>> For the record: this patch made me nervous but survived the rigorous testing I
>> threw at it. (Ie, dialing up using bas_gigaset and downloading almost 20 MB in
>> just over an hour. Whoot! That's more than good enough to ack this patch.)
>>
>> There was some cleanup I had in mind to make this patch more straightforward.
>> But that can wait until someone finds a way to hit an issue with this patch.
>> We'll see.
>
> That someone turns out to be me, doing "modprobe -r bas_gigaset":
>
> <1>[30143.538135] BUG: unable to handle kernel NULL pointer dereference at 000001e9
> <1>[30143.538154] IP: mutex_lock+0x19/0x30
> <6>[30143.538157] *pde = 00000000
> <5>[30143.538162] Oops: 0002 [#1] SMP
> <5>[30143.538165] Modules linked in: bas_gigaset(OE-) gigaset vfat fat uas usb_storage ppp_deflate bsd_comp ppp_synctty ppp_generic slhc capi kernelcapi fuse xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack libcrc32c iptable_mangle iptable_raw iptable_security ebtable_filter ebtables ip6table_filter ip6_tables sunrpc snd_intel8x0 snd_ac97_codec gpio_ich ac97_bus ppdev iTCO_wdt iTCO_vendor_support lpc_ich snd_seq snd_seq_device snd_pcm pcspkr i2c_i801 thinkpad_acpi
> <5>[30143.538228] snd_timer snd irda(C) soundcore parport_pc rfkill parport acpi_cpufreq i915 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops sdhci_pci drm tg3 sdhci mmc_core ata_generic serio_raw yenta_socket ptp pata_acpi pps_core video [last unloaded: gigaset]
> <0>[30143.538257] CPU: 0 PID: 22085 Comm: modprobe Tainted: G C OE 4.14.0-0.rc4.1.local0.fc26.i686 #1
> <0>[30143.538260] Hardware name: IBM 2525FAG/2525FAG, BIOS 74ET64WW (2.09 ) 12/14/2006
> <0>[30143.538263] task: f6671100 task.stack: c77c6000
> <5>[30143.538267] EIP: mutex_lock+0x19/0x30
> <5>[30143.538269] EFLAGS: 00010246 CPU: 0
> <5>[30143.538272] EAX: 00000000 EBX: 000001e9 ECX: 00000001 EDX: f6671100
> <5>[30143.538275] ESI: 000001e9 EDI: 011c8f98 EBP: c77c7ef4 ESP: c77c7ef0
> <5>[30143.538278] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> <5>[30143.538281] CR0: 80050033 CR2: 000001e9 CR3: 16d2d000 CR4: 000006d0
> <0>[30143.538284] Call Trace:
> <0>[30143.538300] gigaset_shutdown+0x28/0x130 [gigaset]
> <0>[30143.538307] ? find_module_all+0x62/0x80
> <0>[30143.538314] bas_gigaset_exit+0x31/0x1077 [bas_gigaset]
> <0>[30143.538319] SyS_delete_module+0x19c/0x240
> <0>[30143.538325] ? ____fput+0xd/0x10
> <0>[30143.538330] do_fast_syscall_32+0x71/0x1a0
> <0>[30143.538335] entry_SYSENTER_32+0x4e/0x7c
> <5>[30143.538337] EIP: 0xb7fb5cd9
> <5>[30143.538339] EFLAGS: 00000206 CPU: 0
> <5>[30143.538342] EAX: ffffffda EBX: 011c8fd4 ECX: 00000800 EDX: 011c8fd4
> <5>[30143.538345] ESI: 011c8f98 EDI: 011c8f98 EBP: 011c8fd4 ESP: bf8278f8
> <5>[30143.538348] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b
> <0>[30143.538351] Code: 8e fb ff ff 5d c3 8d b6 00 00 00 00 8d bf 00 00 00 00 3e 8d 74 26 00 55 89 e5 53 89 c3 e8 60 e7 ff ff 64 8b 15 40 c9 82 c7 31 c0 <3e> 0f b1 13 85 c0 74 07 89 d8 e8 b8 ff ff ff 5b 5d c3 90 8d 74
> <0>[30143.538399] EIP: mutex_lock+0x19/0x30 SS:ESP: 0068:c77c7ef0
> <5>[30143.538402] CR2: 00000000000001e9
> <4>[30143.538406] ---[ end trace 3e60af64adfe7e14 ]---
>
> I'll have to ask for some patience so I can find out what's going on. (Very
> likely: using urb->context beyond it's lifetime.) Expect something by early
Eek, thanks for finding this.
> next week. Is that OK with you?
Totally fine, I'm in no specific rush. I've just been mostly trying to
get as many conversions done as possible to get them in front of the
right people for review.
> Sorry for the noise,
What I did in many other non-trivial conversions was just add an
explicit pointer back, since that's operationally identical to what
struct timer_list was storing in its .data field.
i.e.
add:
struct cardstate *cs;
to struct bas_cardstate, and then use this on timer entry:
struct bas_cardstate *ucs = from_timer(ucs, t, $timer...);
struct cardstate *cs = ucs->cs;
and this at init:
spin_lock_init(&ucs->lock);
+ ucs->cs = cs;
- setup_timer(&ucs->timer_ctrl, req_timeout, (unsigned long) cs);
- setup_timer(&ucs->timer_atrdy, atrdy_timeout, (unsigned long) cs);
- setup_timer(&ucs->timer_cmd_in, cmd_in_timeout, (unsigned long) cs);
- setup_timer(&ucs->timer_int_in, int_in_resubmit, (unsigned long) cs);
+ timer_setup(&ucs->timer_ctrl, req_timeout, 0);
+ timer_setup(&ucs->timer_atrdy, atrdy_timeout, 0);
+ timer_setup(&ucs->timer_cmd_in, cmd_in_timeout, 0);
+ timer_setup(&ucs->timer_int_in, int_in_resubmit, 0);
which will avoid the urb entirely.
Do you want me to send an alternative patch?
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
From: Jiri Pirko @ 2017-10-19 21:39 UTC (permalink / raw)
To: Yuval Mintz
Cc: Steve Lin, netdev@vger.kernel.org, Jiri Pirko,
davem@davemloft.net, michael.chan@broadcom.com,
linville@tuxdriver.com, gospo@broadcom.com
In-Reply-To: <AM0PR0502MB368347E87E48F7EB04AF1513BF420@AM0PR0502MB3683.eurprd05.prod.outlook.com>
Thu, Oct 19, 2017 at 10:32:21PM CEST, yuvalm@mellanox.com wrote:
>> Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF permanent
>> config
>> parameter. Defines number of MSI-X vectors allocated per VF.
>> Value is permanent (stored in NVRAM), so becomes the new default
>> value for this device.
>
>Sounds like you're having this enforce the same configuration for all child VFs.
Yeah, this sounds like per-port config.
>
>>
>> Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>> Acked-by: Andy Gospodarek <gospo@broadcom.com>
>> ---
>> include/uapi/linux/devlink.h | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>> index 8ad6c63..ef163b6 100644
>> --- a/include/uapi/linux/devlink.h
>> +++ b/include/uapi/linux/devlink.h
>> @@ -260,6 +260,7 @@ enum devlink_perm_config_param {
>> DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
>> DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
>> DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT,
>> + DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF,
>> };
>>
>> #endif /* _UAPI_LINUX_DEVLINK_H_ */
>> --
>> 2.7.4
>
^ permalink raw reply
* Re: [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
From: Jiri Pirko @ 2017-10-19 21:43 UTC (permalink / raw)
To: Yuval Mintz
Cc: Steve Lin, netdev@vger.kernel.org, Jiri Pirko,
davem@davemloft.net, michael.chan@broadcom.com,
linville@tuxdriver.com, gospo@broadcom.com
In-Reply-To: <20171019213955.GG1978@nanopsycho.orion>
Thu, Oct 19, 2017 at 11:39:55PM CEST, jiri@resnulli.us wrote:
>Thu, Oct 19, 2017 at 10:32:21PM CEST, yuvalm@mellanox.com wrote:
>>> Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF permanent
>>> config
>>> parameter. Defines number of MSI-X vectors allocated per VF.
>>> Value is permanent (stored in NVRAM), so becomes the new default
>>> value for this device.
>>
>>Sounds like you're having this enforce the same configuration for all child VFs.
>
>Yeah, this sounds like per-port config.
This opens old but lately silent discussion about introducing new port
types for different things. Like VF, dsa CPU port or dsa inter-chip
ports.
>
>
>>
>>>
>>> Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>>> Acked-by: Andy Gospodarek <gospo@broadcom.com>
>>> ---
>>> include/uapi/linux/devlink.h | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>>> index 8ad6c63..ef163b6 100644
>>> --- a/include/uapi/linux/devlink.h
>>> +++ b/include/uapi/linux/devlink.h
>>> @@ -260,6 +260,7 @@ enum devlink_perm_config_param {
>>> DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
>>> DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
>>> DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT,
>>> + DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF,
>>> };
>>>
>>> #endif /* _UAPI_LINUX_DEVLINK_H_ */
>>> --
>>> 2.7.4
>>
^ permalink raw reply
* Re: [PATCH] net: rxrpc: mark expected switch fall-throughs
From: David Howells @ 2017-10-19 21:44 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: dhowells, David S. Miller, linux-afs, netdev, linux-kernel
In-Reply-To: <20171019162323.Horde.9-ghoep6chyagc5A5TqQ8s8@gator4166.hostgator.com>
Gustavo A. R. Silva <garsilva@embeddedor.com> wrote:
> > No. Firstly, it should be 'F'; secondly, don't remove the blank line - it's
> > there for a reason.
> >
>
> What is the reason?
Visual separation.
David
^ permalink raw reply
* Re: [PATCH] net: rxrpc: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2017-10-19 21:50 UTC (permalink / raw)
To: David Howells; +Cc: David S. Miller, linux-afs, netdev, linux-kernel
In-Reply-To: <3101.1508449499@warthog.procyon.org.uk>
Quoting David Howells <dhowells@redhat.com>:
>> What is the reason?
>
> Visual separation.
>
Thanks for clarifying.
--
Gustavo A. R. Silva
^ 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