* Re: [PATCH] iwlwifi: mvm: only send LEDS_CMD when the FW supports it
From: Linus Torvalds @ 2017-09-07 16:13 UTC (permalink / raw)
To: Kalle Valo
Cc: Luca Coelho, Linux Wireless List, Johannes Berg,
Linux Kernel Mailing List, Andrew Morton, Network Development,
David Miller, Emmanuel Grumbach, Luca Coelho
In-Reply-To: <877exan0nq.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>
On Thu, Sep 7, 2017 at 5:39 AM, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>
> Linus, do you want to apply this directly or should we take it via the
> normal route (wireless-drivers -> net)? If your prefer the latter when
> I'm planning to submit this to Dave in a day or two and expecting it to
> get to your tree in about a week, depending of course what is Dave's
> schedule.
Since we have a workaround for the problem, let's just go through the
regular channels. As long as I get the fix through David before the
merge window closes, I'm happy.
Linus
^ permalink raw reply
* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Richard Cochran @ 2017-09-07 15:53 UTC (permalink / raw)
To: Henrik Austad
Cc: Vinicius Costa Gomes, netdev, jhs, xiyou.wangcong, jiri,
intel-wired-lan, andre.guedes, ivan.briano,
jesus.sanchez-palencia, boon.leong.ong
In-Reply-To: <20170907152751.GA9064@sisyphus.home.austad.us>
On Thu, Sep 07, 2017 at 05:27:51PM +0200, Henrik Austad wrote:
> On Thu, Sep 07, 2017 at 02:40:18PM +0200, Richard Cochran wrote:
> And if you want to this driver to act as a bridge, how do you accomodate
> change in network requirements? (i.e. how does this work with switchdev?)
To my understanding, this Qdisc idea provides QoS for the host's
transmitted traffic, and nothing more.
> - Or am I overthinking this?
Being able to configure the external ports of a switchdev is probably
a nice feature, but that is another story. (But maybe I misunderstood
the authors' intent!)
> If you have more than 1 application in userspace that wants to send data
> using this scheduler, how do you ensure fair transmission of frames? (both
> how much bandwidth they use,
There are many ways to handle this, and we shouldn't put any of that
policy into the kernel. For example, there might be a monolithic
application with configurable threads, or an allocation server that
grants bandwidth to applications via IPC, or a multiplexing stream
server like jack, pulse, etc, and so on...
> but also ordering of frames from each application)
Not sure what you mean by this.
> Do you expect all of this to be handled in userspace?
Yes, I do.
Thanks,
Richard
^ permalink raw reply
* [PATCH] net: ethernet: ti: netcp_core: no need in netif_napi_del
From: Ivan Khoronzhuk @ 2017-09-07 15:32 UTC (permalink / raw)
To: w-kwok2, m-karicheri2, netdev
Cc: linux-kernel, grygorii.strashko, Ivan Khoronzhuk
Don't remove rx_napi specifically just before free_netdev(),
it's supposed to be done in it and is confusing w/o tx_napi deletion.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
Based on net-next/master
drivers/net/ethernet/ti/netcp_core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index eb96a69..437d362 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -2145,7 +2145,6 @@ static void netcp_delete_interface(struct netcp_device *netcp_device,
of_node_put(netcp->node_interface);
unregister_netdev(ndev);
- netif_napi_del(&netcp->rx_napi);
free_netdev(ndev);
}
--
2.7.4
^ permalink raw reply related
* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Henrik Austad @ 2017-09-07 15:27 UTC (permalink / raw)
To: Richard Cochran
Cc: Vinicius Costa Gomes, netdev, jhs, xiyou.wangcong, jiri,
intel-wired-lan, andre.guedes, ivan.briano,
jesus.sanchez-palencia, boon.leong.ong
In-Reply-To: <20170907124018.deinzo3c4ice3q7n@localhost>
[-- Attachment #1: Type: text/plain, Size: 3054 bytes --]
On Thu, Sep 07, 2017 at 02:40:18PM +0200, Richard Cochran wrote:
> On Thu, Sep 07, 2017 at 07:34:11AM +0200, Henrik Austad wrote:
> > Also, does this mean that when you create the qdisc, you have locked the
> > bandwidth for the scheduler? Meaning, if I later want to add another
> > stream that requires more bandwidth, I have to close all active streams,
> > reconfigure the qdisc and then restart?
>
> No, just allocate enough bandwidth to accomodate all of the expected
> streams. The streams can start and stop at will.
Sure, that'll work.
And if you want to this driver to act as a bridge, how do you accomodate
change in network requirements? (i.e. how does this work with switchdev?)
- Or am I overthinking this?
> > So my understanding of all of this is that you configure the *total*
> > bandwith for each class when you load the qdisc and then let userspace
> > handle the rest. Is this correct?
>
> Nothing wrong with that.
Didn't mean to say it was wrong, just making sure I've understood the
concept.
> > In my view, it would be nice if the qdisc had some notion about streams so
> > that you could create a stream, feed frames to it and let the driver pace
> > them out. (The fewer you queue, the shorter the delay). This will also
> > allow you to enforce per-stream bandwidth restrictions. I don't see how you
> > can do this here unless you want to do this in userspace.
> >
> > Do you have any plans for adding support for multiplexing streams? If you
> > have multiple streams, how do you enforce that one stream does not eat into
> > the bandwidth of another stream? AFAIK, this is something the network must
> > enforce, but I see no option of doing som here.
>
> Please, lets keep this simple.
Simple is always good
> Today we have exactly zero user space
> applications using this kind of bandwidth reservation. The case of
> wanting the kernel to police individual stream usage does not exist,
> and probably never will.
That we have *zero* userspace applications today is probably related to the
fact that we have exacatly *zero* drivers in the kernel that talks TSN :)
To rephrase a bit, what I'm worried about:
If you have more than 1 application in userspace that wants to send data
using this scheduler, how do you ensure fair transmission of frames? (both
how much bandwidth they use, but also ordering of frames from each
application) Do you expect all of this to be handled in userspace?
> For serious TSN use cases, the bandwidth needed by each system and
> indeed the entire network will be engineered, and we can reasonably
> expect applications to cooperate in this regard.
yes.. that'll happen ;)
> Thanks,
> Richard
Don't get me wrong, I think it is great that others are working on this!
I'm just trying to fully understand the thought that have gone into this
and how it is inteded to be used.
I'll get busy testing the code and wrapping my head around the different
parameters.
--
Henrik Austad
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH 0/2] 9p: Fixes for hard-to-hit bugs
From: Latchesar Ionkov @ 2017-09-07 14:49 UTC (permalink / raw)
To: Tuomas Tynkkynen
Cc: Al Viro, V9FS Developers, Eric Van Hensbergen, Ron Minnich,
David S. Miller, Linux Kernel, netdev, linux-fsdevel
In-Reply-To: <20170906145908.8082-1-tuomas@tuxera.com>
Acked-by: Latchesar Ionkov <lucho@ionkov.net>
On Wed, Sep 6, 2017 at 8:59 AM, Tuomas Tynkkynen <tuomas@tuxera.com> wrote:
> These two patches fix two hard-to-hit (but really annoying) bugs in 9p.
> The first one was posted earlier in February (with one R-b), the second
> is a new one.
>
> Both of these have had soaking in NixOS distribution kernels for
> a couple of months with no ill effects.
>
> Tuomas Tynkkynen (2):
> fs/9p: Compare qid.path in v9fs_test_inode
> net/9p: Switch to wait_event_killable()
>
> fs/9p/vfs_inode.c | 3 +++
> fs/9p/vfs_inode_dotl.c | 3 +++
> net/9p/client.c | 3 +--
> net/9p/trans_virtio.c | 13 ++++++-------
> net/9p/trans_xen.c | 4 ++--
> 5 files changed, 15 insertions(+), 11 deletions(-)
>
> --
> 2.13.0
>
^ permalink raw reply
* Re: [PATCH] rtlwifi: btcoex: 23b 1ant: fix duplicated code for different branches
From: Gustavo A. R. Silva @ 2017-09-07 14:35 UTC (permalink / raw)
To: Larry Finger, Chaoming Li, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <ae09e952-112e-9616-735a-b86ba01ad398@lwfinger.net>
Hi Larry,
On 08/30/2017 11:48 PM, Larry Finger wrote:
> On 08/30/2017 08:42 AM, Gustavo A. R. Silva wrote:
>> Refactor code in order to avoid identical code for different branches.
>>
>> This issue was detected with the help of Coccinelle.
>>
>> Addresses-Coverity-ID: 1226788
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>> This issue was reported by Coverity and it was tested by compilation
>> only.
>> I'm suspicious this may be a copy/paste error. Please, verify.
>>
>> .../net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c | 10
>> ++--------
>> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> This change is not correct. When bt_link_info->sco_exist is true, the
> call should be
>
> halbtc8723b1ant_limited_rx(btcoexist,
> NORMAL_EXEC, true,
> false, 0x5);
>
> NACK
>
> I will push the correct patch.
>
Great. Good to know.
Thanks
--
Gustavo A. R. Silva
^ permalink raw reply
* Re: [V2 PATCH net-next 2/2] xdp: catch invalid XDP_REDIRECT API usage
From: Daniel Borkmann @ 2017-09-07 14:32 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, David S. Miller
Cc: Daniel Borkmann, John Fastabend, Andy Gospodarek,
alexei.starovoitov
In-Reply-To: <59B15403.2090109@iogearbox.net>
On 09/07/2017 04:13 PM, Daniel Borkmann wrote:
[...]
> + uint64_t addr = (unsigned long)prog;
And of course: s/uint64_t/u64/. Lack of context switch. ;-)
^ permalink raw reply
* RE: [PATCH net 0/3] lan78xx: Fixes to lan78xx driver
From: Woojung.Huh @ 2017-09-07 14:26 UTC (permalink / raw)
To: andrew; +Cc: Nisar.Sayed, davem, UNGLinuxDriver, netdev
In-Reply-To: <20170907141957.GL11248@lunn.ch>
> > > > This series of patches are for lan78xx driver.
> > > >
> > > > These patches fixes potential issues associated with lan78xx driver
> > >
> > > Hi Nisar
> > >
> > > So this is version 2? Please include v2 in the subject line.
> > >
> > > Also, briefly list what is different from the previous version.
> > Hi Andrew,
> >
> > Because Nisar dropped of non-mdio patch in the series,
> > I suggested to treat as new patch than version 2.
> > In this case, it is still considered as version 2 than new series?
>
> Hi Woojung
>
> So it is not a clear hard rule here. But we have seen these patches
> before, and they have been modified based on my comments. So i would
> say these are version 2. But a new series would also be O.K. What
> really matters is that the cover note explains what is going on. That
> some of the patches in the previous version have been dropped and will
> be posted later, and what changes have been made to the remaining
> patches.
Andrew,
Thanks for detail explanation and agree that cover page has more description.
Thanks.
Woojung
^ permalink raw reply
* [PATCH iproute2 3/4] devlink: Update devlink UAPI file
From: Arkadi Sharshevsky @ 2017-09-07 14:26 UTC (permalink / raw)
To: netdev; +Cc: davem, stephen, jiri, mlxsw, andrew, Arkadi Sharshevsky,
Jiri Pirko
In-Reply-To: <1504794403-45690-1-git-send-email-arkadis@mellanox.com>
Update devlink UAPI file.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/linux/devlink.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/include/linux/devlink.h b/include/linux/devlink.h
index 7644005..a62695e 100644
--- a/include/linux/devlink.h
+++ b/include/linux/devlink.h
@@ -226,4 +226,22 @@ enum devlink_dpipe_action_type {
DEVLINK_DPIPE_ACTION_TYPE_FIELD_MODIFY,
};
+enum devlink_dpipe_field_ethernet_id {
+ DEVLINK_DPIPE_FIELD_ETHERNET_DST_MAC,
+};
+
+enum devlink_dpipe_field_ipv4_id {
+ DEVLINK_DPIPE_FIELD_IPV4_DST_IP,
+};
+
+enum devlink_dpipe_field_ipv6_id {
+ DEVLINK_DPIPE_FIELD_IPV6_DST_IP,
+};
+
+enum devlink_dpipe_header_id {
+ DEVLINK_DPIPE_HEADER_ETHERNET,
+ DEVLINK_DPIPE_HEADER_IPV4,
+ DEVLINK_DPIPE_HEADER_IPV6,
+};
+
#endif /* _LINUX_DEVLINK_H_ */
--
2.4.11
^ permalink raw reply related
* [PATCH iproute2 0/4] Add support for dpipe's global header formatting
From: Arkadi Sharshevsky @ 2017-09-07 14:26 UTC (permalink / raw)
To: netdev; +Cc: davem, stephen, jiri, mlxsw, andrew, Arkadi Sharshevsky
Some dpipe's global header values need special formatting, for example
Ethernet and IP addresses. This patchset adds support for IPv4/6 and
Ethernet's special format.
Arkadi Sharshevsky (4):
devlink: Make match/action parsing more flexible
devlink: Add support for special format protocol headers
devlink: Update devlink UAPI file
devlink: Add support for protocol IPv4/IPv6/Ethernet special formats
devlink/devlink.c | 319 +++++++++++++++++++++++++++++++++++++-----------
include/linux/devlink.h | 18 +++
2 files changed, 265 insertions(+), 72 deletions(-)
--
2.4.11
^ permalink raw reply
* [PATCH iproute2 1/4] devlink: Make match/action parsing more flexible
From: Arkadi Sharshevsky @ 2017-09-07 14:26 UTC (permalink / raw)
To: netdev; +Cc: davem, stephen, jiri, mlxsw, andrew, Arkadi Sharshevsky,
Jiri Pirko
In-Reply-To: <1504794403-45690-1-git-send-email-arkadis@mellanox.com>
This patch decouples the match/action parsing from printing. This is
done as a preparation for adding the ability to print global header
values, for example print IPv4 address, which require special formatting.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 127 ++++++++++++++++++++++++++++++++++--------------------
1 file changed, 80 insertions(+), 47 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 8f11f86..36a2b36 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -3077,27 +3077,42 @@ static const char
}
}
-static void pr_out_dpipe_action(struct dpipe_ctx *ctx,
- uint32_t header_id, uint32_t field_id,
- uint32_t action_type, bool global)
+struct dpipe_op_info {
+ uint32_t header_id;
+ uint32_t field_id;
+ bool header_global;
+};
+
+struct dpipe_action {
+ struct dpipe_op_info info;
+ uint32_t type;
+};
+
+static void pr_out_dpipe_action(struct dpipe_action *action,
+ struct dpipe_ctx *ctx)
{
+ struct dpipe_op_info *op_info = &action->info;
const char *mapping;
- pr_out_str(ctx->dl, "type", dpipe_action_type_e2s(action_type));
- pr_out_str(ctx->dl, "header", dpipe_header_id2s(ctx, header_id,
- global));
- pr_out_str(ctx->dl, "field", dpipe_field_id2s(ctx, header_id, field_id,
- global));
- mapping = dpipe_mapping_get(ctx, header_id, field_id, global);
+ pr_out_str(ctx->dl, "type",
+ dpipe_action_type_e2s(action->type));
+ pr_out_str(ctx->dl, "header",
+ dpipe_header_id2s(ctx, op_info->header_id,
+ op_info->header_global));
+ pr_out_str(ctx->dl, "field",
+ dpipe_field_id2s(ctx, op_info->header_id,
+ op_info->field_id,
+ op_info->header_global));
+ mapping = dpipe_mapping_get(ctx, op_info->header_id,
+ op_info->field_id,
+ op_info->header_global);
if (mapping)
pr_out_str(ctx->dl, "mapping", mapping);
}
-static int dpipe_action_show(struct dpipe_ctx *ctx, struct nlattr *nl)
+static int dpipe_action_parse(struct dpipe_action *action, struct nlattr *nl)
{
struct nlattr *nla_action[DEVLINK_ATTR_MAX + 1] = {};
- uint32_t header_id, field_id, action_type;
- bool global;
int err;
err = mnl_attr_parse_nested(nl, attr_cb, nla_action);
@@ -3111,12 +3126,11 @@ static int dpipe_action_show(struct dpipe_ctx *ctx, struct nlattr *nl)
return -EINVAL;
}
- header_id = mnl_attr_get_u32(nla_action[DEVLINK_ATTR_DPIPE_HEADER_ID]);
- field_id = mnl_attr_get_u32(nla_action[DEVLINK_ATTR_DPIPE_FIELD_ID]);
- action_type = mnl_attr_get_u32(nla_action[DEVLINK_ATTR_DPIPE_ACTION_TYPE]);
- global = !!mnl_attr_get_u8(nla_action[DEVLINK_ATTR_DPIPE_HEADER_GLOBAL]);
+ action->type = mnl_attr_get_u32(nla_action[DEVLINK_ATTR_DPIPE_ACTION_TYPE]);
+ action->info.header_id = mnl_attr_get_u32(nla_action[DEVLINK_ATTR_DPIPE_HEADER_ID]);
+ action->info.field_id = mnl_attr_get_u32(nla_action[DEVLINK_ATTR_DPIPE_FIELD_ID]);
+ action->info.header_global = !!mnl_attr_get_u8(nla_action[DEVLINK_ATTR_DPIPE_HEADER_GLOBAL]);
- pr_out_dpipe_action(ctx, header_id, field_id, action_type, global);
return 0;
}
@@ -3124,16 +3138,18 @@ static int dpipe_table_actions_show(struct dpipe_ctx *ctx,
struct nlattr *nla_actions)
{
struct nlattr *nla_action;
+ struct dpipe_action action;
mnl_attr_for_each_nested(nla_action, nla_actions) {
pr_out_entry_start(ctx->dl);
- if (dpipe_action_show(ctx, nla_action))
- goto err_action_show;
+ if (dpipe_action_parse(&action, nla_action))
+ goto err_action_parse;
+ pr_out_dpipe_action(&action, ctx);
pr_out_entry_end(ctx->dl);
}
return 0;
-err_action_show:
+err_action_parse:
pr_out_entry_end(ctx->dl);
return -EINVAL;
}
@@ -3149,28 +3165,38 @@ dpipe_match_type_e2s(enum devlink_dpipe_match_type match_type)
}
}
-static void pr_out_dpipe_match(struct dpipe_ctx *ctx,
- uint32_t header_id, uint32_t field_id,
- uint32_t match_type, bool global)
+struct dpipe_match {
+ struct dpipe_op_info info;
+ uint32_t type;
+};
+
+static void pr_out_dpipe_match(struct dpipe_match *match,
+ struct dpipe_ctx *ctx)
{
+ struct dpipe_op_info *op_info = &match->info;
const char *mapping;
- pr_out_str(ctx->dl, "type", dpipe_match_type_e2s(match_type));
- pr_out_str(ctx->dl, "header", dpipe_header_id2s(ctx, header_id,
- global));
- pr_out_str(ctx->dl, "field", dpipe_field_id2s(ctx, header_id, field_id,
- global));
- mapping = dpipe_mapping_get(ctx, header_id, field_id, global);
+ pr_out_str(ctx->dl, "type",
+ dpipe_match_type_e2s(match->type));
+ pr_out_str(ctx->dl, "header",
+ dpipe_header_id2s(ctx, op_info->header_id,
+ op_info->header_global));
+ pr_out_str(ctx->dl, "field",
+ dpipe_field_id2s(ctx, op_info->header_id,
+ op_info->field_id,
+ op_info->header_global));
+ mapping = dpipe_mapping_get(ctx, op_info->header_id,
+ op_info->field_id,
+ op_info->header_global);
if (mapping)
pr_out_str(ctx->dl, "mapping", mapping);
-
}
-static int dpipe_match_show(struct dpipe_ctx *ctx, struct nlattr *nl)
+static int dpipe_match_parse(struct dpipe_match *match,
+ struct nlattr *nl)
+
{
struct nlattr *nla_match[DEVLINK_ATTR_MAX + 1] = {};
- uint32_t header_id, field_id, match_type;
- bool global;
int err;
err = mnl_attr_parse_nested(nl, attr_cb, nla_match);
@@ -3184,12 +3210,11 @@ static int dpipe_match_show(struct dpipe_ctx *ctx, struct nlattr *nl)
return -EINVAL;
}
- match_type = mnl_attr_get_u32(nla_match[DEVLINK_ATTR_DPIPE_MATCH_TYPE]);
- header_id = mnl_attr_get_u32(nla_match[DEVLINK_ATTR_DPIPE_HEADER_ID]);
- field_id = mnl_attr_get_u32(nla_match[DEVLINK_ATTR_DPIPE_FIELD_ID]);
- global = !!mnl_attr_get_u8(nla_match[DEVLINK_ATTR_DPIPE_HEADER_GLOBAL]);
+ match->type = mnl_attr_get_u32(nla_match[DEVLINK_ATTR_DPIPE_MATCH_TYPE]);
+ match->info.header_id = mnl_attr_get_u32(nla_match[DEVLINK_ATTR_DPIPE_HEADER_ID]);
+ match->info.field_id = mnl_attr_get_u32(nla_match[DEVLINK_ATTR_DPIPE_FIELD_ID]);
+ match->info.header_global = !!mnl_attr_get_u8(nla_match[DEVLINK_ATTR_DPIPE_HEADER_GLOBAL]);
- pr_out_dpipe_match(ctx, header_id, field_id, match_type, global);
return 0;
}
@@ -3197,16 +3222,18 @@ static int dpipe_table_matches_show(struct dpipe_ctx *ctx,
struct nlattr *nla_matches)
{
struct nlattr *nla_match;
+ struct dpipe_match match;
mnl_attr_for_each_nested(nla_match, nla_matches) {
pr_out_entry_start(ctx->dl);
- if (dpipe_match_show(ctx, nla_match))
- goto err_match_show;
+ if (dpipe_match_parse(&match, nla_match))
+ goto err_match_parse;
+ pr_out_dpipe_match(&match, ctx);
pr_out_entry_end(ctx->dl);
}
return 0;
-err_match_show:
+err_match_parse:
pr_out_entry_end(ctx->dl);
return -EINVAL;
}
@@ -3382,6 +3409,7 @@ static int dpipe_entry_match_value_show(struct dpipe_ctx *ctx,
struct nlattr *nl)
{
struct nlattr *nla_match_value[DEVLINK_ATTR_MAX + 1] = {};
+ struct dpipe_match match;
int err;
err = mnl_attr_parse_nested(nl, attr_cb, nla_match_value);
@@ -3394,16 +3422,18 @@ static int dpipe_entry_match_value_show(struct dpipe_ctx *ctx,
}
pr_out_entry_start(ctx->dl);
- if (dpipe_match_show(ctx, nla_match_value[DEVLINK_ATTR_DPIPE_MATCH]))
- goto err_match_show;
+ if (dpipe_match_parse(&match,
+ nla_match_value[DEVLINK_ATTR_DPIPE_MATCH]))
+ goto err_match_parse;
+ pr_out_dpipe_match(&match, ctx);
if (dpipe_entry_value_show(ctx, nla_match_value))
goto err_value_show;
pr_out_entry_end(ctx->dl);
return 0;
-err_match_show:
err_value_show:
+err_match_parse:
pr_out_entry_end(ctx->dl);
return -EINVAL;
}
@@ -3412,6 +3442,7 @@ static int dpipe_entry_action_value_show(struct dpipe_ctx *ctx,
struct nlattr *nl)
{
struct nlattr *nla_action_value[DEVLINK_ATTR_MAX + 1] = {};
+ struct dpipe_action action;
int err;
err = mnl_attr_parse_nested(nl, attr_cb, nla_action_value);
@@ -3424,16 +3455,18 @@ static int dpipe_entry_action_value_show(struct dpipe_ctx *ctx,
}
pr_out_entry_start(ctx->dl);
- if (dpipe_action_show(ctx, nla_action_value[DEVLINK_ATTR_DPIPE_ACTION]))
- goto err_action_show;
+ if (dpipe_action_parse(&action,
+ nla_action_value[DEVLINK_ATTR_DPIPE_ACTION]))
+ goto err_action_parse;
+ pr_out_dpipe_action(&action, ctx);
if (dpipe_entry_value_show(ctx, nla_action_value))
goto err_value_show;
pr_out_entry_end(ctx->dl);
return 0;
-err_action_show:
err_value_show:
+err_action_parse:
pr_out_entry_end(ctx->dl);
return -EINVAL;
}
--
2.4.11
^ permalink raw reply related
* [PATCH iproute2 2/4] devlink: Add support for special format protocol headers
From: Arkadi Sharshevsky @ 2017-09-07 14:26 UTC (permalink / raw)
To: netdev; +Cc: davem, stephen, jiri, mlxsw, andrew, Arkadi Sharshevsky,
Jiri Pirko
In-Reply-To: <1504794403-45690-1-git-send-email-arkadis@mellanox.com>
In case of global header (protocol header), the header:field ids are used
to perform lookup for special format printer. In case no printer existence
fallback to plain value printing.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 119 ++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 94 insertions(+), 25 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 36a2b36..b87de38 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -3372,9 +3372,89 @@ static int cmd_dpipe_table_set(struct dl *dl)
return _mnlg_socket_sndrcv(dl->nlg, nlh, NULL, NULL);
}
-static int dpipe_entry_value_show(struct dpipe_ctx *ctx,
- struct nlattr **nla_match_value)
+enum dpipe_value_type {
+ DPIPE_VALUE_TYPE_VALUE,
+ DPIPE_VALUE_TYPE_MASK,
+};
+
+static const char *
+dpipe_value_type_e2s(enum dpipe_value_type type)
+{
+ switch (type) {
+ case DPIPE_VALUE_TYPE_VALUE:
+ return "value";
+ case DPIPE_VALUE_TYPE_MASK:
+ return "value_mask";
+ default:
+ return "<unknown>";
+ }
+}
+
+struct dpipe_field_printer {
+ unsigned int field_id;
+ void (*printer)(struct dpipe_ctx *, enum dpipe_value_type, void *);
+};
+
+struct dpipe_header_printer {
+ struct dpipe_field_printer *printers;
+ unsigned int printers_count;
+ unsigned int header_id;
+};
+
+static struct dpipe_header_printer *dpipe_header_printers[] = {};
+
+static int dpipe_print_prot_header(struct dpipe_ctx *ctx,
+ struct dpipe_op_info *info,
+ enum dpipe_value_type type,
+ void *value)
{
+ unsigned int header_printers_count = ARRAY_SIZE(dpipe_header_printers);
+ struct dpipe_header_printer *header_printer;
+ struct dpipe_field_printer *field_printer;
+ unsigned int field_printers_count;
+ int j;
+ int i;
+
+ for (i = 0; i < header_printers_count; i++) {
+ header_printer = dpipe_header_printers[i];
+ if (header_printer->header_id != info->header_id)
+ continue;
+ field_printers_count = header_printer->printers_count;
+ for (j = 0; j < field_printers_count; j++) {
+ field_printer = &header_printer->printers[j];
+ if (field_printer->field_id != info->field_id)
+ continue;
+ field_printer->printer(ctx, type, value);
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+
+static void __pr_out_entry_value(struct dpipe_ctx *ctx,
+ void *value,
+ unsigned int value_len,
+ struct dpipe_op_info *info,
+ enum dpipe_value_type type)
+{
+ if (info->header_global &&
+ !dpipe_print_prot_header(ctx, info, type, value))
+ return;
+
+ if (value_len == sizeof(uint32_t)) {
+ uint32_t *value_32 = value;
+
+ pr_out_uint(ctx->dl, dpipe_value_type_e2s(type), *value_32);
+ }
+}
+
+static void pr_out_dpipe_entry_value(struct dpipe_ctx *ctx,
+ struct nlattr **nla_match_value,
+ struct dpipe_op_info *info)
+{
+ void *value, *value_mask;
+ uint32_t value_mapping;
uint16_t value_len;
bool mask, mapping;
@@ -3382,27 +3462,20 @@ static int dpipe_entry_value_show(struct dpipe_ctx *ctx,
mapping = !!nla_match_value[DEVLINK_ATTR_DPIPE_VALUE_MAPPING];
value_len = mnl_attr_get_payload_len(nla_match_value[DEVLINK_ATTR_DPIPE_VALUE]);
- if (value_len == sizeof(uint32_t)) {
- uint32_t value, value_mask, value_mapping;
-
- if (mapping) {
- value_mapping = mnl_attr_get_u32(nla_match_value[DEVLINK_ATTR_DPIPE_VALUE_MAPPING]);
- pr_out_uint(ctx->dl, "mapping_value", value_mapping);
- }
-
- if (mask) {
- value_mask = mnl_attr_get_u32(nla_match_value[DEVLINK_ATTR_DPIPE_VALUE_MASK]);
- pr_out_uint(ctx->dl, "mask_value", value_mask);
- }
+ value = mnl_attr_get_payload(nla_match_value[DEVLINK_ATTR_DPIPE_VALUE]);
- value = mnl_attr_get_u32(nla_match_value[DEVLINK_ATTR_DPIPE_VALUE]);
- pr_out_uint(ctx->dl, "value", value);
+ if (mapping) {
+ value_mapping = mnl_attr_get_u32(nla_match_value[DEVLINK_ATTR_DPIPE_VALUE_MAPPING]);
+ pr_out_uint(ctx->dl, "mapping_value", value_mapping);
+ }
- } else {
- return -EINVAL;
+ if (mask) {
+ value_mask = mnl_attr_get_payload(nla_match_value[DEVLINK_ATTR_DPIPE_VALUE]);
+ __pr_out_entry_value(ctx, value_mask, value_len, info,
+ DPIPE_VALUE_TYPE_MASK);
}
- return 0;
+ __pr_out_entry_value(ctx, value, value_len, info, DPIPE_VALUE_TYPE_VALUE);
}
static int dpipe_entry_match_value_show(struct dpipe_ctx *ctx,
@@ -3426,13 +3499,11 @@ static int dpipe_entry_match_value_show(struct dpipe_ctx *ctx,
nla_match_value[DEVLINK_ATTR_DPIPE_MATCH]))
goto err_match_parse;
pr_out_dpipe_match(&match, ctx);
- if (dpipe_entry_value_show(ctx, nla_match_value))
- goto err_value_show;
+ pr_out_dpipe_entry_value(ctx, nla_match_value, &match.info);
pr_out_entry_end(ctx->dl);
return 0;
-err_value_show:
err_match_parse:
pr_out_entry_end(ctx->dl);
return -EINVAL;
@@ -3459,13 +3530,11 @@ static int dpipe_entry_action_value_show(struct dpipe_ctx *ctx,
nla_action_value[DEVLINK_ATTR_DPIPE_ACTION]))
goto err_action_parse;
pr_out_dpipe_action(&action, ctx);
- if (dpipe_entry_value_show(ctx, nla_action_value))
- goto err_value_show;
+ pr_out_dpipe_entry_value(ctx, nla_action_value, &action.info);
pr_out_entry_end(ctx->dl);
return 0;
-err_value_show:
err_action_parse:
pr_out_entry_end(ctx->dl);
return -EINVAL;
--
2.4.11
^ permalink raw reply related
* [PATCH iproute2 4/4] devlink: Add support for protocol IPv4/IPv6/Ethernet special formats
From: Arkadi Sharshevsky @ 2017-09-07 14:26 UTC (permalink / raw)
To: netdev; +Cc: davem, stephen, jiri, mlxsw, andrew, Arkadi Sharshevsky,
Jiri Pirko
In-Reply-To: <1504794403-45690-1-git-send-email-arkadis@mellanox.com>
Add support for protocol IPv4/IPv6/Ethernet special formats.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 74 insertions(+), 1 deletion(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index b87de38..39cda06 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -20,6 +20,7 @@
#include <linux/genetlink.h>
#include <linux/devlink.h>
#include <libmnl/libmnl.h>
+#include <netinet/ether.h>
#include "SNAPSHOT.h"
#include "list.h"
@@ -3401,7 +3402,79 @@ struct dpipe_header_printer {
unsigned int header_id;
};
-static struct dpipe_header_printer *dpipe_header_printers[] = {};
+static void dpipe_field_printer_ipv4_addr(struct dpipe_ctx *ctx,
+ enum dpipe_value_type type,
+ void *value)
+{
+ struct in_addr ip_addr;
+
+ ip_addr.s_addr = htonl(*(uint32_t *)value);
+ pr_out_str(ctx->dl, dpipe_value_type_e2s(type), inet_ntoa(ip_addr));
+}
+
+static void
+dpipe_field_printer_ethernet_addr(struct dpipe_ctx *ctx,
+ enum dpipe_value_type type,
+ void *value)
+{
+ pr_out_str(ctx->dl, dpipe_value_type_e2s(type),
+ ether_ntoa((struct ether_addr *)value));
+}
+
+static void dpipe_field_printer_ipv6_addr(struct dpipe_ctx *ctx,
+ enum dpipe_value_type type,
+ void *value)
+{
+ char str[INET6_ADDRSTRLEN];
+
+ inet_ntop(AF_INET6, value, str, INET6_ADDRSTRLEN);
+ pr_out_str(ctx->dl, dpipe_value_type_e2s(type), str);
+}
+
+static struct dpipe_field_printer dpipe_field_printers_ipv4[] = {
+ {
+ .printer = dpipe_field_printer_ipv4_addr,
+ .field_id = DEVLINK_DPIPE_FIELD_IPV4_DST_IP,
+ }
+};
+
+static struct dpipe_header_printer dpipe_header_printer_ipv4 = {
+ .printers = dpipe_field_printers_ipv4,
+ .printers_count = ARRAY_SIZE(dpipe_field_printers_ipv4),
+ .header_id = DEVLINK_DPIPE_HEADER_IPV4,
+};
+
+static struct dpipe_field_printer dpipe_field_printers_ethernet[] = {
+ {
+ .printer = dpipe_field_printer_ethernet_addr,
+ .field_id = DEVLINK_DPIPE_FIELD_ETHERNET_DST_MAC,
+ },
+};
+
+static struct dpipe_header_printer dpipe_header_printer_ethernet = {
+ .printers = dpipe_field_printers_ethernet,
+ .printers_count = ARRAY_SIZE(dpipe_field_printers_ethernet),
+ .header_id = DEVLINK_DPIPE_HEADER_ETHERNET,
+};
+
+static struct dpipe_field_printer dpipe_field_printers_ipv6[] = {
+ {
+ .printer = dpipe_field_printer_ipv6_addr,
+ .field_id = DEVLINK_DPIPE_FIELD_IPV6_DST_IP,
+ }
+};
+
+static struct dpipe_header_printer dpipe_header_printer_ipv6 = {
+ .printers = dpipe_field_printers_ipv6,
+ .printers_count = ARRAY_SIZE(dpipe_field_printers_ipv6),
+ .header_id = DEVLINK_DPIPE_HEADER_IPV6,
+};
+
+static struct dpipe_header_printer *dpipe_header_printers[] = {
+ &dpipe_header_printer_ipv4,
+ &dpipe_header_printer_ethernet,
+ &dpipe_header_printer_ipv6,
+};
static int dpipe_print_prot_header(struct dpipe_ctx *ctx,
struct dpipe_op_info *info,
--
2.4.11
^ permalink raw reply related
* Re: [PATCH net 0/3] lan78xx: Fixes to lan78xx driver
From: Andrew Lunn @ 2017-09-07 14:19 UTC (permalink / raw)
To: Woojung.Huh; +Cc: Nisar.Sayed, davem, UNGLinuxDriver, netdev
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40B1ADB7@CHN-SV-EXMX02.mchp-main.com>
On Thu, Sep 07, 2017 at 02:03:38PM +0000, Woojung.Huh@microchip.com wrote:
> > On Thu, Sep 07, 2017 at 07:10:51AM +0000, Nisar.Sayed@microchip.com
> > wrote:
> > > From: Nisar Sayed <Nisar.Sayed@microchip.com>
> > >
> > > This series of patches are for lan78xx driver.
> > >
> > > These patches fixes potential issues associated with lan78xx driver
> >
> > Hi Nisar
> >
> > So this is version 2? Please include v2 in the subject line.
> >
> > Also, briefly list what is different from the previous version.
> Hi Andrew,
>
> Because Nisar dropped of non-mdio patch in the series,
> I suggested to treat as new patch than version 2.
> In this case, it is still considered as version 2 than new series?
Hi Woojung
So it is not a clear hard rule here. But we have seen these patches
before, and they have been modified based on my comments. So i would
say these are version 2. But a new series would also be O.K. What
really matters is that the cover note explains what is going on. That
some of the patches in the previous version have been dropped and will
be posted later, and what changes have been made to the remaining
patches.
Andrew
^ permalink raw reply
* Re: [PATCH] netfilter: xt_hashlimit: avoid 64-bit division
From: Geert Uytterhoeven @ 2017-09-07 14:16 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Vishwanath Pai, Josh Hunt, netfilter-devel,
coreteam, netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20170906195825.3715290-1-arnd@arndb.de>
Hi Arnd,
On Wed, Sep 6, 2017 at 9:57 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 64-bit division is expensive on 32-bit architectures, and
> requires a special function call to avoid a link error like:
>
> net/netfilter/xt_hashlimit.o: In function `hashlimit_mt_common':
> xt_hashlimit.c:(.text+0x1328): undefined reference to `__aeabi_uldivmod'
>
> In the case of hashlimit_mt_common, we don't actually need a
> 64-bit operation, we can simply rewrite the function slightly
> to make that clear to the compiler.
>
> Fixes: bea74641e378 ("netfilter: xt_hashlimit: add rate match mode")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Thanks, this fixes a similar issue (__udivdi3 undefined) on m68k.
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* Re: [V2 PATCH net-next 2/2] xdp: catch invalid XDP_REDIRECT API usage
From: Daniel Borkmann @ 2017-09-07 14:13 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, David S. Miller
Cc: Daniel Borkmann, John Fastabend, Andy Gospodarek,
alexei.starovoitov
In-Reply-To: <150478759820.28665.14031878598812204399.stgit@firesoul>
Hey Jesper,
On 09/07/2017 02:33 PM, Jesper Dangaard Brouer wrote:
> Catch different invalid XDP_REDIRECT and bpf_redirect_map API usage.
>
> It is fairly easy to create a dangling redirect_info->map pointer,
> which (until John or Daniel fix this) can crash the kernel.
[...]
Here's what I wrote up yesterday, test looked good, feel
free to give it a spin as well if you want. Was planning
to submit it later today as official patch. Definitely less
intrusive than adding something to napi hot path impacting
everyone.
Cheers,
Daniel
From 56ad381c87dcc2b32156c5970c75bf98818ea7f2 Mon Sep 17 00:00:00 2001
Message-Id: <56ad381c87dcc2b32156c5970c75bf98818ea7f2.1504790124.git.daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed, 6 Sep 2017 21:21:27 +0200
Subject: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs
We can potentially run into a couple of issues with the XDP
bpf_redirect_map() helper. The ri->map in the per CPU storage
can become stale in several ways, mostly due to misuse, where
we can then trigger a use after free on the map:
i) prog A is calling bpf_redirect_map(), returning XDP_REDIRECT
and running on a driver not supporting XDP_REDIRECT yet. The
ri->map on that CPU becomes stale when the XDP program is unloaded
on the driver, and a prog B loaded on a different driver which
supports XDP_REDIRECT return code. prog B would have to omit
calling to bpf_redirect_map() and just return XDP_REDIRECT, which
would then access the freed map in xdp_do_redirect() since not
cleared for that CPU.
ii) prog A is calling bpf_redirect_map(), returning a code other
than XDP_REDIRECT. prog A is then detached, which triggers release
of the map. prog B is attached which, similarly as in i), would
just return XDP_REDIRECT without having called bpf_redirect_map()
and thus be accessing the freed map in xdp_do_redirect() since
not cleared for that CPU.
iii) prog A is attached to generic XDP, calling the bpf_redirect_map()
helper and returning XDP_REDIRECT. xdp_do_generic_redirect() is
currently not handling ri->map (will be fixed by Jesper), so it's
not being reset. Later loading a e.g. native prog B which would,
say, call bpf_xdp_redirect() and then returns XDP_REDIRECT would
find in xdp_do_redirect() that a map was set and uses that causing
use after free on map access.
Fix thus needs to avoid accessing stale ri->map pointers, naive
way would be to call a BPF function from drivers that just resets
it to NULL for all XDP return codes but XDP_REDIRECT and including
XDP_REDIRECT for drivers not supporting it yet (and let ri->map
being handled in xdp_do_generic_redirect()). There is a less
intrusive way w/o letting drivers call a reset for each BPF run.
The verifier knows we're calling into bpf_xdp_redirect_map()
helper, so it can do a small insn rewrite transparent to the prog
itself in the sense that it fills R4 with a pointer to the own
bpf_prog. We have that pointer at verification time anyway and
R4 is allowed to be used as per calling convention we scratch
R0 to R5 anyway, so they become inaccessible and program cannot
read them prior to a write. Then, the helper would store the prog
pointer in the current CPUs struct redirect_info. Later in
xdp_do_*_redirect() we check whether the redirect_info's prog
pointer is the same as passed xdp_prog pointer, and if that's
the case then all good, since the prog holds a ref on the map
anyway, so it is always valid at that point in time and must
have a reference count of at least 1. If in the unlikely case
they are not equal, it means we got a stale pointer, so we clear
and bail out right there. Also do reset map and the owning prog
in bpf_xdp_redirect(), so that bpf_xdp_redirect_map() and
bpf_xdp_redirect() won't get mixed up, only the last call should
take precedence. A tc bpf_redirect() doesn't use map anywhere
yet, so no need to clear it there since never accessed in that
layer.
Note that in case the prog is released, and thus the map as
well we're still under RCU read critical section at that time
and have preemption disabled as well. Once we commit with the
__dev_map_insert_ctx() from xdp_do_redirect_map() and set the
map to ri->map_to_flush, we still wait for a xdp_do_flush_map()
to finish in devmap dismantle time once flush_needed bit is set,
so that is fine.
Fixes: 97f91a7cf04f ("bpf: add bpf_redirect_map helper routine")
Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/verifier.c | 16 ++++++++++++++++
net/core/filter.c | 21 +++++++++++++++++++--
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d690c7d..af13987 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4203,6 +4203,22 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
continue;
}
+ if (insn->imm == BPF_FUNC_redirect_map) {
+ uint64_t addr = (unsigned long)prog;
+ struct bpf_insn r4_ld[] = {
+ BPF_LD_IMM64(BPF_REG_4, addr),
+ *insn,
+ };
+ cnt = ARRAY_SIZE(r4_ld);
+
+ new_prog = bpf_patch_insn_data(env, i + delta, r4_ld, cnt);
+ if (!new_prog)
+ return -ENOMEM;
+
+ delta += cnt - 1;
+ env->prog = prog = new_prog;
+ insn = new_prog->insnsi + i + delta;
+ }
patch_call_imm:
fn = prog->aux->ops->get_func_proto(insn->imm);
/* all functions that have prototype and verifier allowed
diff --git a/net/core/filter.c b/net/core/filter.c
index 5912c73..0848df2 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1794,6 +1794,7 @@ struct redirect_info {
u32 flags;
struct bpf_map *map;
struct bpf_map *map_to_flush;
+ const struct bpf_prog *map_owner;
};
static DEFINE_PER_CPU(struct redirect_info, redirect_info);
@@ -1807,7 +1808,6 @@ struct redirect_info {
ri->ifindex = ifindex;
ri->flags = flags;
- ri->map = NULL;
return TC_ACT_REDIRECT;
}
@@ -2504,6 +2504,7 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_prog *xdp_prog)
{
struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ const struct bpf_prog *map_owner = ri->map_owner;
struct bpf_map *map = ri->map;
u32 index = ri->ifindex;
struct net_device *fwd;
@@ -2511,6 +2512,15 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
ri->ifindex = 0;
ri->map = NULL;
+ ri->map_owner = NULL;
+
+ /* This is really only caused by a deliberately crappy
+ * BPF program, normally we would never hit that case,
+ * so no need to inform someone via tracepoints either,
+ * just bail out.
+ */
+ if (unlikely(map_owner != xdp_prog))
+ return -EINVAL;
fwd = __dev_map_lookup_elem(map, index);
if (!fwd) {
@@ -2607,6 +2617,8 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
ri->ifindex = ifindex;
ri->flags = flags;
+ ri->map = NULL;
+ ri->map_owner = NULL;
return XDP_REDIRECT;
}
@@ -2619,7 +2631,8 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
.arg2_type = ARG_ANYTHING,
};
-BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex, u64, flags)
+BPF_CALL_4(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex, u64, flags,
+ const struct bpf_prog *, map_owner)
{
struct redirect_info *ri = this_cpu_ptr(&redirect_info);
@@ -2629,10 +2642,14 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
ri->ifindex = ifindex;
ri->flags = flags;
ri->map = map;
+ ri->map_owner = map_owner;
return XDP_REDIRECT;
}
+/* Note, arg4 is hidden from users and populated by the verifier
+ * with the right pointer.
+ */
static const struct bpf_func_proto bpf_xdp_redirect_map_proto = {
.func = bpf_xdp_redirect_map,
.gpl_only = false,
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 3/3] lan78xx: Use default value loaded from EEPROM/OTP when resetting
From: Andrew Lunn @ 2017-09-07 14:13 UTC (permalink / raw)
To: Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev
In-Reply-To: <CE371C1263339941885964188A0225FA333A8D@CHN-SV-EXMX03.mchp-main.com>
On Thu, Sep 07, 2017 at 07:11:50AM +0000, Nisar.Sayed@microchip.com wrote:
> From: Nisar Sayed <Nisar.Sayed@microchip.com>
>
> Use default value loaded from EEPROM/OTP when resetting
Hi Nisar
Subject: [PATCH 3/3]
Is this a fix for net, or further development for net-next?
Why do we want the default values?
Andrew
^ permalink raw reply
* Re: [PATCH net 2/3] lan78xx: Allow EEPROM write for less than MAX_EEPROM_SIZE
From: Andrew Lunn @ 2017-09-07 14:10 UTC (permalink / raw)
To: Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev
In-Reply-To: <CE371C1263339941885964188A0225FA333A7C@CHN-SV-EXMX03.mchp-main.com>
On Thu, Sep 07, 2017 at 07:11:26AM +0000, Nisar.Sayed@microchip.com wrote:
> From: Nisar Sayed <Nisar.Sayed@microchip.com>
>
> Allow EEPROM write for less than MAX_EEPROM_SIZE
>
> Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
> ---
> drivers/net/usb/lan78xx.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index baf91c7..94ef943 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -1299,6 +1299,10 @@ static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
struct ethtool_eeprom *ee, u8 *data)
{
struct lan78xx_net *dev = netdev_priv(netdev);
/* Allow entire eeprom update only */
if ((ee->magic == LAN78XX_EEPROM_MAGIC) &&
(ee->offset == 0) &&
(ee->len == 512) &&
(data[0] == EEPROM_INDICATOR))
return lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
else if ((ee->magic == LAN78XX_OTP_MAGIC) &&
> (ee->len == 512) &&
> (data[0] == OTP_INDICATOR_1))
> ret = lan78xx_write_raw_otp(dev, ee->offset, ee->len, data);
> + else if ((ee->magic == LAN78XX_EEPROM_MAGIC) &&
> + (ee->offset >= 0 && ee->offset < MAX_EEPROM_SIZE) &&
> + (ee->len > 0 && (ee->offset + ee->len) <= MAX_EEPROM_SIZE))
> + ret = lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
>
> usb_autopm_put_interface(dev->intf);
Hi Nisar
You should explain why this change cannot be folded into the first if
statement.
Andrew
^ permalink raw reply
* Re: [V2 PATCH net-next 1/2] xdp: implement xdp_redirect_map for generic XDP
From: Daniel Borkmann @ 2017-09-07 14:09 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, David S. Miller
Cc: Daniel Borkmann, John Fastabend, Andy Gospodarek,
alexei.starovoitov
In-Reply-To: <150478759310.28665.17184783248584070473.stgit@firesoul>
On 09/07/2017 02:33 PM, Jesper Dangaard Brouer wrote:
> Using bpf_redirect_map is allowed for generic XDP programs, but the
> appropriate map lookup was never performed in xdp_do_generic_redirect().
>
> Instead the map-index is directly used as the ifindex. For the
> xdp_redirect_map sample in SKB-mode '-S', this resulted in trying
> sending on ifindex 0 which isn't valid, resulting in getting SKB
> packets dropped. Thus, the reported performance numbers are wrong in
> commit 24251c264798 ("samples/bpf: add option for native and skb mode
> for redirect apps") for the 'xdp_redirect_map -S' case.
>
> It might seem innocent this was lacking, but it can actually crash the
> kernel. The potential crash is caused by not consuming redirect_info->map.
> The bpf_redirect_map helper will set this_cpu_ptr(&redirect_info)->map
> pointer, which will survive even after unloading the xdp bpf_prog and
> deallocating the devmap data-structure. This leaves a dead map
> pointer around. The kernel will crash when loading the xdp_redirect
> sample (in native XDP mode) as it doesn't reset map (via bpf_redirect)
> and returns XDP_REDIRECT, which will cause it to dereference the map
> pointer.
>
> Fixes: 6103aa96ec07 ("net: implement XDP_REDIRECT for xdp generic")
> Fixes: 24251c264798 ("samples/bpf: add option for native and skb mode for redirect apps")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> include/trace/events/xdp.h | 4 ++--
> net/core/filter.c | 14 +++++++++++---
> 2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
> index 862575ac8da9..4e16c43fba10 100644
> --- a/include/trace/events/xdp.h
> +++ b/include/trace/events/xdp.h
> @@ -138,11 +138,11 @@ DEFINE_EVENT_PRINT(xdp_redirect_template, xdp_redirect_map_err,
>
> #define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx) \
> trace_xdp_redirect_map(dev, xdp, fwd ? fwd->ifindex : 0, \
> - 0, map, idx);
> + 0, map, idx)
>
> #define _trace_xdp_redirect_map_err(dev, xdp, fwd, map, idx, err) \
> trace_xdp_redirect_map_err(dev, xdp, fwd ? fwd->ifindex : 0, \
> - err, map, idx);
> + err, map, idx)
>
> #endif /* _TRACE_XDP_H */
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 5912c738a7b2..3767470cab6c 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2566,13 +2566,19 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
> struct bpf_prog *xdp_prog)
> {
> struct redirect_info *ri = this_cpu_ptr(&redirect_info);
> + struct bpf_map *map = ri->map;
> u32 index = ri->ifindex;
> struct net_device *fwd;
> unsigned int len;
> int err = 0;
>
> - fwd = dev_get_by_index_rcu(dev_net(dev), index);
> ri->ifindex = 0;
> + ri->map = NULL;
> +
> + if (map)
> + fwd = __dev_map_lookup_elem(map, index);
> + else
> + fwd = dev_get_by_index_rcu(dev_net(dev), index);
> if (unlikely(!fwd)) {
> err = -EINVAL;
> goto err;
> @@ -2590,10 +2596,12 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
> }
>
> skb->dev = fwd;
Looks much better above, thanks!
> - _trace_xdp_redirect(dev, xdp_prog, index);
> + map ? _trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index)
> + : _trace_xdp_redirect(dev, xdp_prog, index);
Could we rather make this in a way such that when the two
tracepoints are disabled and thus patched out, that we can
also omit the extra conditional which has no purpose then?
Perhaps just a consolidated _trace_xdp_generic_redirect_map()
would be better to avoid this altogether given we have twice
the same anyway, here and in err path.
Thanks,
Daniel
> return 0;
> err:
> - _trace_xdp_redirect_err(dev, xdp_prog, index, err);
> + map ? _trace_xdp_redirect_map_err(dev, xdp_prog, fwd, map, index, err)
> + : _trace_xdp_redirect_err(dev, xdp_prog, index, err);
> return err;
> }
> EXPORT_SYMBOL_GPL(xdp_do_generic_redirect);
>
^ permalink raw reply
* Re: nfp bpf offload add/replace
From: Jiri Pirko @ 2017-09-07 14:05 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, mlxsw, Daniel Borkmann, Simon Horman
In-Reply-To: <20170907144412.7a4a7cf7@cakuba.netronome.com>
Thu, Sep 07, 2017 at 03:44:12PM CEST, kubakici@wp.pl wrote:
>On Thu, 7 Sep 2017 11:10:33 +0200, Jiri Pirko wrote:
>> Hi Kuba.
>>
>> I'm looking into cls_bpf code and nfp_net_bpf_offload function in your
>> driver. Why do you need TC_CLSBPF_ADD? Seems like TC_CLSBPF_REPLACE
>> should be enough. It would make the cls_bpf code easier.
>>
>> Note that other cls just have replace/destroy (u32 too, as drivers
>> handle NEW/REPLACE in one switch-case - will patch this).
>
>Could we clarify what the REPLACE is actually supposed to do? :)
>
>In the flower code and the REPLACE looks a lot like ADD on the
>surface... If change is called it will invoke REPLACE with the new
>filter and then if there was an old filter, it will do DELETE. Is my
>understanding correct?
Yes, correct.
>
>If so I found this model of operation somehow confusing. Plus the
>management of flows may get slightly tricky if there is a possibility of
>"replacing" a flow with an identical one. Flower may make calls like
>these:
>
>add flower vlan_id 100 action ...
># REPLACE vid 100 ...
>change ... flower vlan_id 100 action ...
># REPLACE vid 100 ...
># DELETE vid 100 ...
Yes, that is the flow.
>
>Doesn't this force driver/HW to implement refcounting on the rules?
Why do you think so? There is a cookie that is passed from flower down
and driver uses it to remove the entry.
>
>On why I need the replace - BPF unlike other classifiers usually
>installs a single program, I think offloading multiple TC filters is
>questionable (people will use tailcalls instead most likely). I want to
>be able to implement atomic replace of that single program (i.e. not ADD
>followed by DELETE) because that simplifies the driver quite a bit.
Understood. So, looks like the REPLACE/DESTROY would be sufficient for
bpf. ADD is not needed as it can be done by REPLACE-NULL, right?
On the other hand, the rest of the cls, namely flower, u32 and matchall
need ADD/DESTROY as they don't really do no replacing.
Makes sense?
^ permalink raw reply
* Re: [PATCH net 1/3] lan78xx: Fix for eeprom read/write when device autosuspend
From: Andrew Lunn @ 2017-09-07 14:04 UTC (permalink / raw)
To: Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev
In-Reply-To: <CE371C1263339941885964188A0225FA333A6B@CHN-SV-EXMX03.mchp-main.com>
On Thu, Sep 07, 2017 at 07:11:06AM +0000, Nisar.Sayed@microchip.com wrote:
> From: Nisar Sayed <Nisar.Sayed@microchip.com>
>
> Fix for eeprom read/write when device autosuspend
So this is a fix? And you said [PATCH net 1/3], meaning it is supposed
to go into stable? You should try to add a fixes tag. Something like:
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Andrew
^ permalink raw reply
* RE: [PATCH net 0/3] lan78xx: Fixes to lan78xx driver
From: Woojung.Huh @ 2017-09-07 14:03 UTC (permalink / raw)
To: andrew, Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev
In-Reply-To: <20170907135909.GH11248@lunn.ch>
> On Thu, Sep 07, 2017 at 07:10:51AM +0000, Nisar.Sayed@microchip.com
> wrote:
> > From: Nisar Sayed <Nisar.Sayed@microchip.com>
> >
> > This series of patches are for lan78xx driver.
> >
> > These patches fixes potential issues associated with lan78xx driver
>
> Hi Nisar
>
> So this is version 2? Please include v2 in the subject line.
>
> Also, briefly list what is different from the previous version.
Hi Andrew,
Because Nisar dropped of non-mdio patch in the series,
I suggested to treat as new patch than version 2.
In this case, it is still considered as version 2 than new series?
- Woojung
^ permalink raw reply
* Re: [PATCH net 0/3] lan78xx: Fixes to lan78xx driver
From: Andrew Lunn @ 2017-09-07 13:59 UTC (permalink / raw)
To: Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev
In-Reply-To: <CE371C1263339941885964188A0225FA333A58@CHN-SV-EXMX03.mchp-main.com>
On Thu, Sep 07, 2017 at 07:10:51AM +0000, Nisar.Sayed@microchip.com wrote:
> From: Nisar Sayed <Nisar.Sayed@microchip.com>
>
> This series of patches are for lan78xx driver.
>
> These patches fixes potential issues associated with lan78xx driver
Hi Nisar
So this is version 2? Please include v2 in the subject line.
Also, briefly list what is different from the previous version.
Andrew
^ permalink raw reply
* Re: [PATCH net] smsc95xx: Configure pause time to 0xffff when tx flow control enabled
From: Andrew Lunn @ 2017-09-07 13:56 UTC (permalink / raw)
To: Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev, steve.glendinning
In-Reply-To: <CE371C1263339941885964188A0225FA3339DB@CHN-SV-EXMX03.mchp-main.com>
On Thu, Sep 07, 2017 at 06:51:37AM +0000, Nisar.Sayed@microchip.com wrote:
> From: Nisar Sayed <Nisar.Sayed@microchip.com>
>
> Configure pause time to 0xffff when tx flow control enabled
Hi Nisar
You should explain the 'Why' in the commit message. Why do we want a
pause time of 0xffff?
Andrew
^ permalink raw reply
* Re: nfp bpf offload add/replace
From: Jakub Kicinski @ 2017-09-07 13:44 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, mlxsw, Daniel Borkmann, Simon Horman
In-Reply-To: <20170907091033.GD1967@nanopsycho>
On Thu, 7 Sep 2017 11:10:33 +0200, Jiri Pirko wrote:
> Hi Kuba.
>
> I'm looking into cls_bpf code and nfp_net_bpf_offload function in your
> driver. Why do you need TC_CLSBPF_ADD? Seems like TC_CLSBPF_REPLACE
> should be enough. It would make the cls_bpf code easier.
>
> Note that other cls just have replace/destroy (u32 too, as drivers
> handle NEW/REPLACE in one switch-case - will patch this).
Could we clarify what the REPLACE is actually supposed to do? :)
In the flower code and the REPLACE looks a lot like ADD on the
surface... If change is called it will invoke REPLACE with the new
filter and then if there was an old filter, it will do DELETE. Is my
understanding correct?
If so I found this model of operation somehow confusing. Plus the
management of flows may get slightly tricky if there is a possibility of
"replacing" a flow with an identical one. Flower may make calls like
these:
add flower vlan_id 100 action ...
# REPLACE vid 100 ...
change ... flower vlan_id 100 action ...
# REPLACE vid 100 ...
# DELETE vid 100 ...
Doesn't this force driver/HW to implement refcounting on the rules?
On why I need the replace - BPF unlike other classifiers usually
installs a single program, I think offloading multiple TC filters is
questionable (people will use tailcalls instead most likely). I want to
be able to implement atomic replace of that single program (i.e. not ADD
followed by DELETE) because that simplifies the driver quite a bit.
^ 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