* Re: [PATCH net] sit: use ipv6_mod_enabled to check if ipv6 is disabled
From: Eric Dumazet @ 2019-02-26 4:36 UTC (permalink / raw)
To: Hangbin Liu, David Ahern; +Cc: netdev, Stefano Brivio, David S . Miller
In-Reply-To: <20190226040759.GU10051@dhcp-12-139.nay.redhat.com>
On 02/25/2019 08:08 PM, Hangbin Liu wrote:
> Hi David,
> On Mon, Feb 25, 2019 at 07:15:26PM -0700, David Ahern wrote:
>> On 2/25/19 6:55 PM, Hangbin Liu wrote:
>>> Just as I said, this issue only occurs when IPv6 is disabled at boot time
>>> as there is no IPv6 route entry. Disable ipv6 on specific interface should
>>> not be affected(IPv6 route/fib has inited). So I think use ipv6_mod_enabled()
>>> would be more suitable in this scenario. Did I miss something?
>>
>> From a readability perspective the code path depends on whether ipv6 is
>> enabled on the device. I think it is better to leave that as it is.
>
> When I posted 173656accaf5 ("sit: check if IPv6 enabled before calling
> ip6_err_gen_icmpv6_unreach()"), my purpose is to check if IPv6 disabled
> at boot time. I didn't know we have ipv6_mod_enabled() at that time, so I
> just used __in6_dev_get() as a trick way/work around.
>
> A few days later I saw your commit e434863718d4 ("net: vrf: Fix crash when
> IPv6 is disabled at boot time") and I thought this would be a more clear way
> to tell people that we are checking if IPv6 disabled at boot time. So I posted
> these two follow up fixes.
>
> I don't know why you thought check IPv6 is enbled on the device is better.
> Because it's more strict? Maybe I missed something here. But if you feel it
> is better to leave as it it, then let's keep it.
>
I do not get it.
We really do not care if IPv6 is enabled on some device on the host.
Here the correct test is checking if IPv6 is enabled for _this_ device.
It is not about fixing a crash (it is already fixed), but not having to call
ip6_err_gen_icmpv6_unreach() knowing it will fail anyway.
So the current code is better.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next v4 6/6] devlink: require non-NULL ops for devlink instances
From: Florian Fainelli @ 2019-02-26 4:29 UTC (permalink / raw)
To: Jakub Kicinski, davem, jiri; +Cc: mkubecek, andrew, netdev, oss-drivers
In-Reply-To: <20190226033407.32625-7-jakub.kicinski@netronome.com>
On 2/25/2019 7:34 PM, Jakub Kicinski wrote:
> Commit 76726ccb7f46 ("devlink: add flash update command") and
> commit 2d8dc5bbf4e7 ("devlink: Add support for reload")
> access devlink ops without NULL-checking. There is, however, no
> driver which would pass in NULL ops, so let's just make that
> a requirement. Remove the now unnecessary NULL-checking.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH bpf-next 1/4] bpf: enable program stats
From: Alexei Starovoitov @ 2019-02-26 4:27 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov
Cc: Roman Gushchin, Alexei Starovoitov, davem@davemloft.net,
netdev@vger.kernel.org, bpf@vger.kernel.org, Kernel Team
In-Reply-To: <439aa0d4-3f31-1385-076a-bb8dab515bef@iogearbox.net>
On 2/25/19 2:36 AM, Daniel Borkmann wrote:
>
> Not through the stack, but was more thinking something like low-overhead
> kprobes-style extension for a BPF prog where such sequence would be added
> 'inline' at beginning / exit of BPF prog invocation with normal ctx access
> and helpers as the native program (e.g. time-stamping plus read/write into
> mark as one example which kprobes couldn't do). But might go much beyond
> context of this stats collection.
see below.
> That's actually an interesting thought, given the original prog->bpf_func
> is a known address at that time, this could be templated where an inner
> dummy bpf_func call to the normal BPF prog gets later replaced with the
> actual prog->bpf_func address to have no indirect call. This would also
> allow to keep the actual prog->bpf_func entry call-sites small and simple.
My first thought was that we indeed can have a template of stats
collecting wrapper in .text,
then at 'switch stats on' event vmalloc exec region, copy wrapper code
into it and manually patch 'call foo' x86 insn with direct jump.
That is still quite involved from coding side.
It's similar to what kprobe/ftrace is doing, but probably
an overkill for stats due to potential security
concerns with new executable code.
But it won't work due to tail_calls.
I've looked into single wrapper approach with indirect call
(because it's much easer than patching x86) and realized it
won't work for the same reason.
Inline kprobe+kretprobe insertion won't work either.
All because we have tail_calls.
we cannot have executable epilogue in progs.
tail_call will jmp into next prog and second part of stats collection
won't run. Essentially no epilogue allowed unless we disable tail_call.
Or we somehow reimplement tail_calls so that prog returns,
an epilogue is executed and then jumps into the next prog.
tail_call turned out be the worst corner case feature to deal with.
With static_key approach the main prog accounts the time
of all progs called via tail_call.
We can argue whether it's ideal semantics, but looks like
it's the only thing we can do.
I don't see a way how tail_called progs can count their own time.
Stats would somehow need to be computed right before jumping
into next prog. Which means heavy changes in all JITs
plus extra performance cost at runtime, since such if (stats_on)
check in tail_call handling would have to be done at runtime,
since JITed code is read-only and regenerating progs due to stats
is non-starter.
Or tail-called next prog would need to update stats for previous
prog, but there is no pointer to 'aux->stats' there.
So imo that is dead end.
static_key approach is the simplest by far.
^ permalink raw reply
* Re: [PATCH net-next v4 5/6] devlink: hold a reference to the netdevice around ethtool compat
From: Florian Fainelli @ 2019-02-26 4:26 UTC (permalink / raw)
To: Jakub Kicinski, davem, jiri; +Cc: mkubecek, andrew, netdev, oss-drivers
In-Reply-To: <20190226033407.32625-6-jakub.kicinski@netronome.com>
On 2/25/2019 7:34 PM, Jakub Kicinski wrote:
> When ethtool is calling into devlink compat code make sure we have
> a reference on the netdevice on which the operation was invoked.
>
> v3: move the hold/lock logic into devlink_compat_* functions (Florian)
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net] sit: use ipv6_mod_enabled to check if ipv6 is disabled
From: Hangbin Liu @ 2019-02-26 4:08 UTC (permalink / raw)
To: David Ahern; +Cc: Eric Dumazet, netdev, Stefano Brivio, David S . Miller
In-Reply-To: <da79cacd-c95d-6f52-109e-10273f7452b3@cumulusnetworks.com>
Hi David,
On Mon, Feb 25, 2019 at 07:15:26PM -0700, David Ahern wrote:
> On 2/25/19 6:55 PM, Hangbin Liu wrote:
> > Just as I said, this issue only occurs when IPv6 is disabled at boot time
> > as there is no IPv6 route entry. Disable ipv6 on specific interface should
> > not be affected(IPv6 route/fib has inited). So I think use ipv6_mod_enabled()
> > would be more suitable in this scenario. Did I miss something?
>
> From a readability perspective the code path depends on whether ipv6 is
> enabled on the device. I think it is better to leave that as it is.
When I posted 173656accaf5 ("sit: check if IPv6 enabled before calling
ip6_err_gen_icmpv6_unreach()"), my purpose is to check if IPv6 disabled
at boot time. I didn't know we have ipv6_mod_enabled() at that time, so I
just used __in6_dev_get() as a trick way/work around.
A few days later I saw your commit e434863718d4 ("net: vrf: Fix crash when
IPv6 is disabled at boot time") and I thought this would be a more clear way
to tell people that we are checking if IPv6 disabled at boot time. So I posted
these two follow up fixes.
I don't know why you thought check IPv6 is enbled on the device is better.
Because it's more strict? Maybe I missed something here. But if you feel it
is better to leave as it it, then let's keep it.
Thanks
Hangbin
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Christoph Hellwig @ 2019-02-26 3:57 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Miller, Alexei Starovoitov, Masami Hiramatsu,
Steven Rostedt, Andy Lutomirski, Linux List Kernel Mailing,
Ingo Molnar, Andrew Morton, stable, Changbin Du, Jann Horn,
Kees Cook, Andrew Lutomirski, Daniel Borkmann, Netdev, bpf
In-Reply-To: <CAHk-=wh9uWJaih5i6qBPer5rzcs=A+yb_ejU093b1esAhoFveQ@mail.gmail.com>
On Fri, Feb 22, 2019 at 01:59:10PM -0800, Linus Torvalds wrote:
> On Fri, Feb 22, 2019 at 1:38 PM David Miller <davem@davemloft.net> wrote:
> >
> > Don't be surprised if we see more separation like this in the future too.
>
> Yes, with the whole meltdown fiasco, there's actually more pressure to
> add more support for separation of kernel/user address spaces. As Andy
> pointed out, it's been discussed as a future wish-list for x86-64 too.
S/390 is another example.
I've also proposed a RISC-V extension for it, including prototypes
for Rocketchip and Qemu, and a Linux kernel support, but it didn't go
any way.
^ permalink raw reply
* Re: [PATCH net] ipv4: Add ICMPv6 support when parse route ipproto
From: Hangbin Liu @ 2019-02-26 3:48 UTC (permalink / raw)
To: David Ahern; +Cc: Sabrina Dubroca, netdev, Roopa Prabhu, David S . Miller
In-Reply-To: <20f3652d-479b-7be1-cf13-e7d8a61dca9e@gmail.com>
Hi David,
On Mon, Feb 25, 2019 at 07:23:33PM -0700, David Ahern wrote:
> On 2/25/19 7:17 PM, Hangbin Liu wrote:
> > I also thought about this issue. Currently we didn't check the ipproto in both
> > IPv4 and IPv6. You can set icmp in ip6 rules or icmpv6 in ipv4 rules.
> > This looks don't make any serious problem. It's just a user mis-configuration,
> > the kernel check the proto number and won't match normal IP/IPv6 headers.
> >
> > But yes, we should make it more strict, do you think if I should add a new
> > rtm_getroute_parse_ip6_proto() function, or just add a family parameter
> > in previous function?
>
> I see now. rtm_getroute_parse_ip_proto is used for ipv4 and ipv6. For v4
> IPPROTO_ICMPV6 should not be allowed and for v6 IPPROTO_ICMP should
> fail. You could a version argument to rtm_getroute_parse_ip_proto and
> fail as needed.
Sorry I didn't get here. Do you mean add an IPv6 version of
rtm_getroute_parse_ip_proto?
Thanks
Hangbin
^ permalink raw reply
* Re: [PATCH v3 bpf-next 1/4] bpf: enable program stats
From: Alexei Starovoitov @ 2019-02-26 3:42 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Alexei Starovoitov, davem@davemloft.net, daniel@iogearbox.net,
edumazet@google.com, netdev@vger.kernel.org, bpf@vger.kernel.org,
Kernel Team
In-Reply-To: <20190226031049.GD32115@mini-arch>
On 2/25/19 7:10 PM, Stanislav Fomichev wrote:
> On 02/25, Alexei Starovoitov wrote:
>> On 2/25/19 3:07 PM, Stanislav Fomichev wrote:
>>>> +#define BPF_PROG_RUN(prog, ctx) ({ \
>>>> + u32 ret; \
>>>> + cant_sleep(); \
>>>> + if (static_branch_unlikely(&bpf_stats_enabled_key)) { \
>>>> + struct bpf_prog_stats *stats; \
>>>> + u64 start = sched_clock(); \
>>> QQ: why sched_clock() and not, for example, ktime_get_ns() which we do
>>> in the bpf_test_run()? Or even why not local_clock?
>>> I'm just wondering what king of trade off we are doing here
>>> regarding precision vs run time cost.
>>
>>
>> I'm making this decision based on documentation:
>> Documentation/timers/timekeeping.txt
>> "Compared to clock sources, sched_clock() has to be very fast: it is
>> called much more often, especially by the scheduler. If you have to do
>> trade-offs between accuracy compared to the clock source, you may
>> sacrifice accuracy for speed in sched_clock()."
> So sched_clock is fast, but imprecise; and ktime_get_ns (and
> lock_clock?) are slow(er), but more precise?
>
> If that's the case, would it make sense to use a more precise
> measurement? I suppose the BPF program execution time is on the order of
> nanoseconds and if sched_close has msec or usec resolution, all we get is
> essentially noise?
>
> I understand that you want this feature to have almost no overhead, but
> since it's gated by the static key, should we aim for a higher precision?
>
Considering everything I believe sched_clock() strikes the best trade off.
^ permalink raw reply
* [PATCH net-next v4 6/6] devlink: require non-NULL ops for devlink instances
From: Jakub Kicinski @ 2019-02-26 3:34 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190226033407.32625-1-jakub.kicinski@netronome.com>
Commit 76726ccb7f46 ("devlink: add flash update command") and
commit 2d8dc5bbf4e7 ("devlink: Add support for reload")
access devlink ops without NULL-checking. There is, however, no
driver which would pass in NULL ops, so let's just make that
a requirement. Remove the now unnecessary NULL-checking.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
net/core/devlink.c | 48 +++++++++++++++++++++-------------------------
1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index ecff6e63bc4d..a49dee67e66f 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -723,7 +723,7 @@ static int devlink_port_type_set(struct devlink *devlink,
{
int err;
- if (devlink->ops && devlink->ops->port_type_set) {
+ if (devlink->ops->port_type_set) {
if (port_type == DEVLINK_PORT_TYPE_NOTSET)
return -EINVAL;
if (port_type == devlink_port->type)
@@ -760,7 +760,7 @@ static int devlink_port_split(struct devlink *devlink, u32 port_index,
u32 count, struct netlink_ext_ack *extack)
{
- if (devlink->ops && devlink->ops->port_split)
+ if (devlink->ops->port_split)
return devlink->ops->port_split(devlink, port_index, count,
extack);
return -EOPNOTSUPP;
@@ -786,7 +786,7 @@ static int devlink_port_unsplit(struct devlink *devlink, u32 port_index,
struct netlink_ext_ack *extack)
{
- if (devlink->ops && devlink->ops->port_unsplit)
+ if (devlink->ops->port_unsplit)
return devlink->ops->port_unsplit(devlink, port_index, extack);
return -EOPNOTSUPP;
}
@@ -961,7 +961,7 @@ static int devlink_nl_cmd_sb_pool_get_doit(struct sk_buff *skb,
if (err)
return err;
- if (!devlink->ops || !devlink->ops->sb_pool_get)
+ if (!devlink->ops->sb_pool_get)
return -EOPNOTSUPP;
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
@@ -1017,7 +1017,7 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
mutex_lock(&devlink_mutex);
list_for_each_entry(devlink, &devlink_list, list) {
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
- !devlink->ops || !devlink->ops->sb_pool_get)
+ !devlink->ops->sb_pool_get)
continue;
mutex_lock(&devlink->lock);
list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
@@ -1046,7 +1046,7 @@ static int devlink_sb_pool_set(struct devlink *devlink, unsigned int sb_index,
{
const struct devlink_ops *ops = devlink->ops;
- if (ops && ops->sb_pool_set)
+ if (ops->sb_pool_set)
return ops->sb_pool_set(devlink, sb_index, pool_index,
size, threshold_type);
return -EOPNOTSUPP;
@@ -1151,7 +1151,7 @@ static int devlink_nl_cmd_sb_port_pool_get_doit(struct sk_buff *skb,
if (err)
return err;
- if (!devlink->ops || !devlink->ops->sb_port_pool_get)
+ if (!devlink->ops->sb_port_pool_get)
return -EOPNOTSUPP;
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
@@ -1213,7 +1213,7 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
mutex_lock(&devlink_mutex);
list_for_each_entry(devlink, &devlink_list, list) {
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
- !devlink->ops || !devlink->ops->sb_port_pool_get)
+ !devlink->ops->sb_port_pool_get)
continue;
mutex_lock(&devlink->lock);
list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
@@ -1242,7 +1242,7 @@ static int devlink_sb_port_pool_set(struct devlink_port *devlink_port,
{
const struct devlink_ops *ops = devlink_port->devlink->ops;
- if (ops && ops->sb_port_pool_set)
+ if (ops->sb_port_pool_set)
return ops->sb_port_pool_set(devlink_port, sb_index,
pool_index, threshold);
return -EOPNOTSUPP;
@@ -1355,7 +1355,7 @@ static int devlink_nl_cmd_sb_tc_pool_bind_get_doit(struct sk_buff *skb,
if (err)
return err;
- if (!devlink->ops || !devlink->ops->sb_tc_pool_bind_get)
+ if (!devlink->ops->sb_tc_pool_bind_get)
return -EOPNOTSUPP;
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
@@ -1439,7 +1439,7 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
mutex_lock(&devlink_mutex);
list_for_each_entry(devlink, &devlink_list, list) {
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
- !devlink->ops || !devlink->ops->sb_tc_pool_bind_get)
+ !devlink->ops->sb_tc_pool_bind_get)
continue;
mutex_lock(&devlink->lock);
@@ -1471,7 +1471,7 @@ static int devlink_sb_tc_pool_bind_set(struct devlink_port *devlink_port,
{
const struct devlink_ops *ops = devlink_port->devlink->ops;
- if (ops && ops->sb_tc_pool_bind_set)
+ if (ops->sb_tc_pool_bind_set)
return ops->sb_tc_pool_bind_set(devlink_port, sb_index,
tc_index, pool_type,
pool_index, threshold);
@@ -1519,7 +1519,7 @@ static int devlink_nl_cmd_sb_occ_snapshot_doit(struct sk_buff *skb,
struct devlink_sb *devlink_sb = info->user_ptr[1];
const struct devlink_ops *ops = devlink->ops;
- if (ops && ops->sb_occ_snapshot)
+ if (ops->sb_occ_snapshot)
return ops->sb_occ_snapshot(devlink, devlink_sb->index);
return -EOPNOTSUPP;
}
@@ -1531,7 +1531,7 @@ static int devlink_nl_cmd_sb_occ_max_clear_doit(struct sk_buff *skb,
struct devlink_sb *devlink_sb = info->user_ptr[1];
const struct devlink_ops *ops = devlink->ops;
- if (ops && ops->sb_occ_max_clear)
+ if (ops->sb_occ_max_clear)
return ops->sb_occ_max_clear(devlink, devlink_sb->index);
return -EOPNOTSUPP;
}
@@ -1594,13 +1594,9 @@ static int devlink_nl_cmd_eswitch_get_doit(struct sk_buff *skb,
struct genl_info *info)
{
struct devlink *devlink = info->user_ptr[0];
- const struct devlink_ops *ops = devlink->ops;
struct sk_buff *msg;
int err;
- if (!ops)
- return -EOPNOTSUPP;
-
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (!msg)
return -ENOMEM;
@@ -1625,9 +1621,6 @@ static int devlink_nl_cmd_eswitch_set_doit(struct sk_buff *skb,
int err = 0;
u16 mode;
- if (!ops)
- return -EOPNOTSUPP;
-
if (info->attrs[DEVLINK_ATTR_ESWITCH_MODE]) {
if (!ops->eswitch_mode_set)
return -EOPNOTSUPP;
@@ -3869,7 +3862,7 @@ static int devlink_nl_cmd_info_get_doit(struct sk_buff *skb,
struct sk_buff *msg;
int err;
- if (!devlink->ops || !devlink->ops->info_get)
+ if (!devlink->ops->info_get)
return -EOPNOTSUPP;
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
@@ -5232,6 +5225,9 @@ struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size)
{
struct devlink *devlink;
+ if (WARN_ON(!ops))
+ return NULL;
+
devlink = kzalloc(sizeof(*devlink) + priv_size, GFP_KERNEL);
if (!devlink)
return NULL;
@@ -6091,7 +6087,7 @@ __devlink_param_driverinit_value_set(struct devlink *devlink,
int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
union devlink_param_value *init_val)
{
- if (!devlink->ops || !devlink->ops->reload)
+ if (!devlink->ops->reload)
return -EOPNOTSUPP;
return __devlink_param_driverinit_value_get(&devlink->param_list,
@@ -6138,7 +6134,7 @@ int devlink_port_param_driverinit_value_get(struct devlink_port *devlink_port,
{
struct devlink *devlink = devlink_port->devlink;
- if (!devlink->ops || !devlink->ops->reload)
+ if (!devlink->ops->reload)
return -EOPNOTSUPP;
return __devlink_param_driverinit_value_get(&devlink_port->param_list,
@@ -6435,7 +6431,7 @@ void devlink_compat_running_version(struct net_device *dev,
mutex_lock(&devlink_mutex);
devlink = netdev_to_devlink(dev);
- if (!devlink || !devlink->ops || !devlink->ops->info_get)
+ if (!devlink || !devlink->ops->info_get)
goto unlock_list;
mutex_lock(&devlink->lock);
@@ -6458,7 +6454,7 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
mutex_lock(&devlink_mutex);
devlink = netdev_to_devlink(dev);
- if (!devlink || !devlink->ops || !devlink->ops->flash_update)
+ if (!devlink || !devlink->ops->flash_update)
goto unlock_list;
mutex_lock(&devlink->lock);
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v4 5/6] devlink: hold a reference to the netdevice around ethtool compat
From: Jakub Kicinski @ 2019-02-26 3:34 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190226033407.32625-1-jakub.kicinski@netronome.com>
When ethtool is calling into devlink compat code make sure we have
a reference on the netdevice on which the operation was invoked.
v3: move the hold/lock logic into devlink_compat_* functions (Florian)
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
net/core/devlink.c | 13 +++++++++++++
net/core/ethtool.c | 13 ++-----------
2 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 24bfbd2d71e7..ecff6e63bc4d 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6430,6 +6430,9 @@ void devlink_compat_running_version(struct net_device *dev,
{
struct devlink *devlink;
+ dev_hold(dev);
+ rtnl_unlock();
+
mutex_lock(&devlink_mutex);
devlink = netdev_to_devlink(dev);
if (!devlink || !devlink->ops || !devlink->ops->info_get)
@@ -6440,6 +6443,9 @@ void devlink_compat_running_version(struct net_device *dev,
mutex_unlock(&devlink->lock);
unlock_list:
mutex_unlock(&devlink_mutex);
+
+ rtnl_lock();
+ dev_put(dev);
}
int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
@@ -6447,6 +6453,9 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
struct devlink *devlink;
int ret = -EOPNOTSUPP;
+ dev_hold(dev);
+ rtnl_unlock();
+
mutex_lock(&devlink_mutex);
devlink = netdev_to_devlink(dev);
if (!devlink || !devlink->ops || !devlink->ops->flash_update)
@@ -6457,6 +6466,10 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
mutex_unlock(&devlink->lock);
unlock_list:
mutex_unlock(&devlink_mutex);
+
+ rtnl_lock();
+ dev_put(dev);
+
return ret;
}
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 1320e8dce559..47558ffae423 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -805,11 +805,9 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
if (ops->get_eeprom_len)
info.eedump_len = ops->get_eeprom_len(dev);
- rtnl_unlock();
if (!info.fw_version[0])
devlink_compat_running_version(dev, info.fw_version,
sizeof(info.fw_version));
- rtnl_lock();
if (copy_to_user(useraddr, &info, sizeof(info)))
return -EFAULT;
@@ -2040,15 +2038,8 @@ static noinline_for_stack int ethtool_flash_device(struct net_device *dev,
return -EFAULT;
efl.data[ETHTOOL_FLASH_MAX_FILENAME - 1] = 0;
- if (!dev->ethtool_ops->flash_device) {
- int ret;
-
- rtnl_unlock();
- ret = devlink_compat_flash_update(dev, efl.data);
- rtnl_lock();
-
- return ret;
- }
+ if (!dev->ethtool_ops->flash_device)
+ return devlink_compat_flash_update(dev, efl.data);
return dev->ethtool_ops->flash_device(dev, &efl);
}
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v4 4/6] nfp: remove ethtool flashing fallback
From: Jakub Kicinski @ 2019-02-26 3:34 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190226033407.32625-1-jakub.kicinski@netronome.com>
Now that devlink fallback will be called reliably, we can remove
the ethtool flashing code.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
.../ethernet/netronome/nfp/nfp_net_ethtool.c | 24 -------------------
1 file changed, 24 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 8f189149efc5..690b62718dbb 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -1234,28 +1234,6 @@ static int nfp_net_set_channels(struct net_device *netdev,
return nfp_net_set_num_rings(nn, total_rx, total_tx);
}
-static int
-nfp_net_flash_device(struct net_device *netdev, struct ethtool_flash *flash)
-{
- struct nfp_app *app;
- int ret;
-
- if (flash->region != ETHTOOL_FLASH_ALL_REGIONS)
- return -EOPNOTSUPP;
-
- app = nfp_app_from_netdev(netdev);
- if (!app)
- return -EOPNOTSUPP;
-
- dev_hold(netdev);
- rtnl_unlock();
- ret = nfp_flash_update_common(app->pf, flash->data, NULL);
- rtnl_lock();
- dev_put(netdev);
-
- return ret;
-}
-
static const struct ethtool_ops nfp_net_ethtool_ops = {
.get_drvinfo = nfp_net_get_drvinfo,
.get_link = ethtool_op_get_link,
@@ -1266,7 +1244,6 @@ static const struct ethtool_ops nfp_net_ethtool_ops = {
.get_sset_count = nfp_net_get_sset_count,
.get_rxnfc = nfp_net_get_rxnfc,
.set_rxnfc = nfp_net_set_rxnfc,
- .flash_device = nfp_net_flash_device,
.get_rxfh_indir_size = nfp_net_get_rxfh_indir_size,
.get_rxfh_key_size = nfp_net_get_rxfh_key_size,
.get_rxfh = nfp_net_get_rxfh,
@@ -1292,7 +1269,6 @@ const struct ethtool_ops nfp_port_ethtool_ops = {
.get_strings = nfp_port_get_strings,
.get_ethtool_stats = nfp_port_get_stats,
.get_sset_count = nfp_port_get_sset_count,
- .flash_device = nfp_net_flash_device,
.set_dump = nfp_app_set_dump,
.get_dump_flag = nfp_app_get_dump_flag,
.get_dump_data = nfp_app_get_dump_data,
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v4 3/6] nfp: add .ndo_get_devlink
From: Jakub Kicinski @ 2019-02-26 3:34 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190226033407.32625-1-jakub.kicinski@netronome.com>
Support getting devlink instance from a new NDO.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/netronome/nfp/nfp_app.h | 2 ++
drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 11 +++++++++++
drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 1 +
drivers/net/ethernet/netronome/nfp/nfp_net_repr.c | 1 +
4 files changed, 15 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
index d578d856a009..f8d422713705 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
@@ -433,4 +433,6 @@ int nfp_app_nic_vnic_alloc(struct nfp_app *app, struct nfp_net *nn,
int nfp_app_nic_vnic_init_phy_port(struct nfp_pf *pf, struct nfp_app *app,
struct nfp_net *nn, unsigned int id);
+struct devlink *nfp_devlink_get_devlink(struct net_device *netdev);
+
#endif
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index db2da99f6aa7..e9eca99cf493 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -376,3 +376,14 @@ void nfp_devlink_port_unregister(struct nfp_port *port)
{
devlink_port_unregister(&port->dl_port);
}
+
+struct devlink *nfp_devlink_get_devlink(struct net_device *netdev)
+{
+ struct nfp_app *app;
+
+ app = nfp_app_from_netdev(netdev);
+ if (!app)
+ return NULL;
+
+ return priv_to_devlink(app->pf);
+}
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 776f6c07701b..6d1b8816552e 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3531,6 +3531,7 @@ const struct net_device_ops nfp_net_netdev_ops = {
.ndo_udp_tunnel_del = nfp_net_del_vxlan_port,
.ndo_bpf = nfp_net_xdp,
.ndo_get_port_parent_id = nfp_port_get_port_parent_id,
+ .ndo_get_devlink = nfp_devlink_get_devlink,
};
/**
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
index 62839807e21e..d2c803bb4e56 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
@@ -273,6 +273,7 @@ const struct net_device_ops nfp_repr_netdev_ops = {
.ndo_set_features = nfp_port_set_features,
.ndo_set_mac_address = eth_mac_addr,
.ndo_get_port_parent_id = nfp_port_get_port_parent_id,
+ .ndo_get_devlink = nfp_devlink_get_devlink,
};
void
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v4 2/6] devlink: create a special NDO for getting the devlink instance
From: Jakub Kicinski @ 2019-02-26 3:34 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190226033407.32625-1-jakub.kicinski@netronome.com>
Instead of iterating over all devlink ports add a NDO which
will return the devlink instance from the driver.
v2: add the netdev_to_devlink() helper (Michal)
v3: check that devlink has ops (Florian)
v4: hold devlink_mutex (Jiri)
Suggested-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
include/linux/netdevice.h | 7 +++++
include/net/devlink.h | 9 +++++++
net/core/devlink.c | 56 ++++++++++++---------------------------
3 files changed, 33 insertions(+), 39 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ffbddd03242b..58e83bd7a861 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -941,6 +941,8 @@ struct dev_ifalias {
char ifalias[];
};
+struct devlink;
+
/*
* This structure defines the management hooks for network devices.
* The following hooks can be defined; unless noted otherwise, they are
@@ -1249,6 +1251,10 @@ struct dev_ifalias {
* that got dropped are freed/returned via xdp_return_frame().
* Returns negative number, means general error invoking ndo, meaning
* no frames were xmit'ed and core-caller will free all frames.
+ * struct devlink *(*ndo_get_devlink)(struct net_device *dev);
+ * Get devlink instance associated with a given netdev.
+ * Called with a reference on the netdevice and devlink locks only,
+ * rtnl_lock is not held.
*/
struct net_device_ops {
int (*ndo_init)(struct net_device *dev);
@@ -1447,6 +1453,7 @@ struct net_device_ops {
u32 flags);
int (*ndo_xsk_async_xmit)(struct net_device *dev,
u32 queue_id);
+ struct devlink * (*ndo_get_devlink)(struct net_device *dev);
};
/**
diff --git a/include/net/devlink.h b/include/net/devlink.h
index f9f7fe974652..7f5a0bdca228 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -538,6 +538,15 @@ static inline struct devlink *priv_to_devlink(void *priv)
return container_of(priv, struct devlink, priv);
}
+static inline struct devlink *netdev_to_devlink(struct net_device *dev)
+{
+#if IS_ENABLED(CONFIG_NET_DEVLINK)
+ if (dev->netdev_ops->ndo_get_devlink)
+ return dev->netdev_ops->ndo_get_devlink(dev);
+#endif
+ return NULL;
+}
+
struct ib_device;
#if IS_ENABLED(CONFIG_NET_DEVLINK)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 05e04ea0a5c7..24bfbd2d71e7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6397,9 +6397,6 @@ static void __devlink_compat_running_version(struct devlink *devlink,
struct sk_buff *msg;
int rem, err;
- if (!devlink->ops->info_get)
- return;
-
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (!msg)
return;
@@ -6431,55 +6428,36 @@ static void __devlink_compat_running_version(struct devlink *devlink,
void devlink_compat_running_version(struct net_device *dev,
char *buf, size_t len)
{
- struct devlink_port *devlink_port;
struct devlink *devlink;
mutex_lock(&devlink_mutex);
- list_for_each_entry(devlink, &devlink_list, list) {
- mutex_lock(&devlink->lock);
- list_for_each_entry(devlink_port, &devlink->port_list, list) {
- if (devlink_port->type == DEVLINK_PORT_TYPE_ETH &&
- devlink_port->type_dev == dev) {
- __devlink_compat_running_version(devlink,
- buf, len);
- mutex_unlock(&devlink->lock);
- goto out;
- }
- }
- mutex_unlock(&devlink->lock);
- }
-out:
+ devlink = netdev_to_devlink(dev);
+ if (!devlink || !devlink->ops || !devlink->ops->info_get)
+ goto unlock_list;
+
+ mutex_lock(&devlink->lock);
+ __devlink_compat_running_version(devlink, buf, len);
+ mutex_unlock(&devlink->lock);
+unlock_list:
mutex_unlock(&devlink_mutex);
}
int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
{
- struct devlink_port *devlink_port;
struct devlink *devlink;
+ int ret = -EOPNOTSUPP;
mutex_lock(&devlink_mutex);
- list_for_each_entry(devlink, &devlink_list, list) {
- mutex_lock(&devlink->lock);
- list_for_each_entry(devlink_port, &devlink->port_list, list) {
- int ret = -EOPNOTSUPP;
-
- if (devlink_port->type != DEVLINK_PORT_TYPE_ETH ||
- devlink_port->type_dev != dev)
- continue;
+ devlink = netdev_to_devlink(dev);
+ if (!devlink || !devlink->ops || !devlink->ops->flash_update)
+ goto unlock_list;
- mutex_unlock(&devlink_mutex);
- if (devlink->ops->flash_update)
- ret = devlink->ops->flash_update(devlink,
- file_name,
- NULL, NULL);
- mutex_unlock(&devlink->lock);
- return ret;
- }
- mutex_unlock(&devlink->lock);
- }
+ mutex_lock(&devlink->lock);
+ ret = devlink->ops->flash_update(devlink, file_name, NULL, NULL);
+ mutex_unlock(&devlink->lock);
+unlock_list:
mutex_unlock(&devlink_mutex);
-
- return -EOPNOTSUPP;
+ return ret;
}
static int __init devlink_init(void)
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v4 1/6] net: devlink: turn devlink into a built-in
From: Jakub Kicinski @ 2019-02-26 3:34 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190226033407.32625-1-jakub.kicinski@netronome.com>
Being able to build devlink as a module causes growing pains.
First all drivers had to add a meta dependency to make sure
they are not built in when devlink is built as a module. Now
we are struggling to invoke ethtool compat code reliably.
Make devlink code built-in, users can still not build it at
all but the dynamically loadable module option is removed.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/infiniband/hw/bnxt_re/Kconfig | 1 -
drivers/infiniband/hw/mlx4/Kconfig | 1 -
drivers/net/Kconfig | 1 -
drivers/net/ethernet/broadcom/Kconfig | 1 -
drivers/net/ethernet/cavium/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlx4/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlxsw/Kconfig | 1 -
drivers/net/ethernet/netronome/Kconfig | 1 -
include/net/devlink.h | 10 ++++------
net/Kconfig | 11 +----------
net/core/devlink.c | 15 ++-------------
net/dsa/Kconfig | 2 +-
13 files changed, 8 insertions(+), 39 deletions(-)
diff --git a/drivers/infiniband/hw/bnxt_re/Kconfig b/drivers/infiniband/hw/bnxt_re/Kconfig
index 18f5ed082f41..19982a4a9bba 100644
--- a/drivers/infiniband/hw/bnxt_re/Kconfig
+++ b/drivers/infiniband/hw/bnxt_re/Kconfig
@@ -1,7 +1,6 @@
config INFINIBAND_BNXT_RE
tristate "Broadcom Netxtreme HCA support"
depends on ETHERNET && NETDEVICES && PCI && INET && DCB
- depends on MAY_USE_DEVLINK
select NET_VENDOR_BROADCOM
select BNXT
---help---
diff --git a/drivers/infiniband/hw/mlx4/Kconfig b/drivers/infiniband/hw/mlx4/Kconfig
index d1de3285fd88..4e9936731867 100644
--- a/drivers/infiniband/hw/mlx4/Kconfig
+++ b/drivers/infiniband/hw/mlx4/Kconfig
@@ -2,7 +2,6 @@ config MLX4_INFINIBAND
tristate "Mellanox ConnectX HCA support"
depends on NETDEVICES && ETHERNET && PCI && INET
depends on INFINIBAND_USER_ACCESS || !INFINIBAND_USER_ACCESS
- depends on MAY_USE_DEVLINK
select NET_VENDOR_MELLANOX
select MLX4_CORE
---help---
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6210757772ed..5e4ca082cfcd 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -505,7 +505,6 @@ source "drivers/net/hyperv/Kconfig"
config NETDEVSIM
tristate "Simulated networking device"
depends on DEBUG_FS
- depends on MAY_USE_DEVLINK
help
This driver is a developer testing tool and software model that can
be used to test various control path networking APIs, especially
diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index c1d3ee9baf7e..716bfbba59cf 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -194,7 +194,6 @@ config SYSTEMPORT
config BNXT
tristate "Broadcom NetXtreme-C/E support"
depends on PCI
- depends on MAY_USE_DEVLINK
select FW_LOADER
select LIBCRC32C
---help---
diff --git a/drivers/net/ethernet/cavium/Kconfig b/drivers/net/ethernet/cavium/Kconfig
index 05f4a3b21e29..6650e2a5f171 100644
--- a/drivers/net/ethernet/cavium/Kconfig
+++ b/drivers/net/ethernet/cavium/Kconfig
@@ -64,7 +64,6 @@ config CAVIUM_PTP
config LIQUIDIO
tristate "Cavium LiquidIO support"
depends on 64BIT && PCI
- depends on MAY_USE_DEVLINK
depends on PCI
imply PTP_1588_CLOCK
select FW_LOADER
diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
index f200b8c420d5..ff8057ed97ee 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
@@ -4,7 +4,6 @@
config MLX4_EN
tristate "Mellanox Technologies 1/10/40Gbit Ethernet support"
- depends on MAY_USE_DEVLINK
depends on PCI && NETDEVICES && ETHERNET && INET
select MLX4_CORE
imply PTP_1588_CLOCK
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
index 37a551436e4a..6debffb8336b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
@@ -4,7 +4,6 @@
config MLX5_CORE
tristate "Mellanox 5th generation network adapters (ConnectX series) core driver"
- depends on MAY_USE_DEVLINK
depends on PCI
imply PTP_1588_CLOCK
imply VXLAN
diff --git a/drivers/net/ethernet/mellanox/mlxsw/Kconfig b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
index b9a25aed5d11..9c195dfed031 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
@@ -4,7 +4,6 @@
config MLXSW_CORE
tristate "Mellanox Technologies Switch ASICs support"
- depends on MAY_USE_DEVLINK
---help---
This driver supports Mellanox Technologies Switch ASICs family.
diff --git a/drivers/net/ethernet/netronome/Kconfig b/drivers/net/ethernet/netronome/Kconfig
index 66f15b05b65e..549898d5d450 100644
--- a/drivers/net/ethernet/netronome/Kconfig
+++ b/drivers/net/ethernet/netronome/Kconfig
@@ -19,7 +19,6 @@ config NFP
tristate "Netronome(R) NFP4000/NFP6000 NIC driver"
depends on PCI && PCI_MSI
depends on VXLAN || VXLAN=n
- depends on MAY_USE_DEVLINK
---help---
This driver supports the Netronome(R) NFP4000/NFP6000 based
cards working as a advanced Ethernet NIC. It works with both
diff --git a/include/net/devlink.h b/include/net/devlink.h
index a2da49dd9147..f9f7fe974652 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -707,6 +707,10 @@ devlink_health_reporter_priv(struct devlink_health_reporter *reporter);
int devlink_health_report(struct devlink_health_reporter *reporter,
const char *msg, void *priv_ctx);
+void devlink_compat_running_version(struct net_device *dev,
+ char *buf, size_t len);
+int devlink_compat_flash_update(struct net_device *dev, const char *file_name);
+
#else
static inline struct devlink *devlink_alloc(const struct devlink_ops *ops,
@@ -1190,13 +1194,7 @@ devlink_health_report(struct devlink_health_reporter *reporter,
{
return 0;
}
-#endif
-#if IS_REACHABLE(CONFIG_NET_DEVLINK)
-void devlink_compat_running_version(struct net_device *dev,
- char *buf, size_t len);
-int devlink_compat_flash_update(struct net_device *dev, const char *file_name);
-#else
static inline void
devlink_compat_running_version(struct net_device *dev, char *buf, size_t len)
{
diff --git a/net/Kconfig b/net/Kconfig
index 62da6148e9f8..1efe1f9ee492 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -429,21 +429,12 @@ config NET_SOCK_MSG
with the help of BPF programs.
config NET_DEVLINK
- tristate "Network physical/parent device Netlink interface"
+ bool "Network physical/parent device Netlink interface"
help
Network physical/parent device Netlink interface provides
infrastructure to support access to physical chip-wide config and
monitoring.
-config MAY_USE_DEVLINK
- tristate
- default m if NET_DEVLINK=m
- default y if NET_DEVLINK=y || NET_DEVLINK=n
- help
- Drivers using the devlink infrastructure should have a dependency
- on MAY_USE_DEVLINK to ensure they do not cause link errors when
- devlink is a loadable module and the driver using it is built-in.
-
config PAGE_POOL
bool
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4f31ddc883e7..05e04ea0a5c7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6482,20 +6482,9 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
return -EOPNOTSUPP;
}
-static int __init devlink_module_init(void)
+static int __init devlink_init(void)
{
return genl_register_family(&devlink_nl_family);
}
-static void __exit devlink_module_exit(void)
-{
- genl_unregister_family(&devlink_nl_family);
-}
-
-module_init(devlink_module_init);
-module_exit(devlink_module_exit);
-
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Jiri Pirko <jiri@mellanox.com>");
-MODULE_DESCRIPTION("Network physical device Netlink interface");
-MODULE_ALIAS_GENL_FAMILY(DEVLINK_GENL_NAME);
+subsys_initcall(devlink_init);
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 91e52973ee13..fab49132345f 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -6,7 +6,7 @@ config HAVE_NET_DSA
config NET_DSA
tristate "Distributed Switch Architecture"
- depends on HAVE_NET_DSA && MAY_USE_DEVLINK
+ depends on HAVE_NET_DSA
depends on BRIDGE || BRIDGE=n
select NET_SWITCHDEV
select PHYLINK
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v4 0/6] devlink: make ethtool compat reliable
From: Jakub Kicinski @ 2019-02-26 3:34 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
Hi!
This is a follow up to the series which added device flash
updates via devlink. I went with the approach of adding a
new NDO in the end. It seems to end up looking cleaner.
First patch removes the option to build devlink as a module.
Users can still decide to not build it, but the module option
ends up not being worth the maintenance cost.
Next two patches add a NDO which can be used to ask the driver
to return a devlink instance associated with a given netdev,
instead of iterating over devlink ports. Drivers which implement
this NDO must take into account the potential impact on the
visibility of the devlink instance.
With the new NDO in place we can remove NFP ethtool flash update
code.
Fifth patch makes sure we hold a reference to dev while
callbacks are active.
Last but not least the NULL-check of devlink->ops is moved
to instance allocation time.
Last but not least missing checks for devlink->ops are added.
There is currently no driver registering devlink without ops,
so can just fix this in -next.
v2 (Michal): add netdev_to_devlink() in patch 3.
v3 (Florian):
- add missing checks for devlink->ops;
- move locking/holding into devlink_compat_ functions.
v4 (Jiri):
- hold devlink_mutex around callbacks (patch 2);
- require non-NULL ops (patch 6).
Jakub Kicinski (6):
net: devlink: turn devlink into a built-in
devlink: create a special NDO for getting the devlink instance
nfp: add .ndo_get_devlink
nfp: remove ethtool flashing fallback
devlink: hold a reference to the netdevice around ethtool compat
devlink: require non-NULL ops for devlink instances
drivers/infiniband/hw/bnxt_re/Kconfig | 1 -
drivers/infiniband/hw/mlx4/Kconfig | 1 -
drivers/net/Kconfig | 1 -
drivers/net/ethernet/broadcom/Kconfig | 1 -
drivers/net/ethernet/cavium/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlx4/Kconfig | 1 -
.../net/ethernet/mellanox/mlx5/core/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlxsw/Kconfig | 1 -
drivers/net/ethernet/netronome/Kconfig | 1 -
drivers/net/ethernet/netronome/nfp/nfp_app.h | 2 +
.../net/ethernet/netronome/nfp/nfp_devlink.c | 11 ++
.../ethernet/netronome/nfp/nfp_net_common.c | 1 +
.../ethernet/netronome/nfp/nfp_net_ethtool.c | 24 ----
.../net/ethernet/netronome/nfp/nfp_net_repr.c | 1 +
include/linux/netdevice.h | 7 +
include/net/devlink.h | 19 ++-
net/Kconfig | 11 +-
net/core/devlink.c | 126 +++++++-----------
net/core/ethtool.c | 13 +-
net/dsa/Kconfig | 2 +-
20 files changed, 90 insertions(+), 136 deletions(-)
--
2.19.2
^ permalink raw reply
* Re: [PATCH v2 bpf-next 2/9] bpf: Add bpf helper bpf_tcp_enter_cwr
From: Stanislav Fomichev @ 2019-02-26 3:32 UTC (permalink / raw)
To: Martin Lau
Cc: Lawrence Brakmo, netdev, Alexei Starovoitov, Daniel Borkmann,
Eric Dumazet, Kernel Team
In-Reply-To: <20190226012959.4gttysrj3khqumc5@kafai-mbp.dhcp.thefacebook.com>
On 02/26, Martin Lau wrote:
> On Mon, Feb 25, 2019 at 03:14:38PM -0800, Stanislav Fomichev wrote:
> [ ... ]
>
> > >
> > > To ensure it is only called from BPF_CGROUP_INET_EGRESS, the
> > > attr->expected_attach_type must be specified as BPF_CGROUP_INET_EGRESS
> > > during load time if the prog uses this new helper.
> > > The newly added prog->enforce_expected_attach_type bit will also be set
> > > if this new helper is used. This bit is for backward compatibility reason
> > > because currently prog->expected_attach_type has been ignored in
> > > BPF_PROG_TYPE_CGROUP_SKB. During attach time,
> > > prog->expected_attach_type is only enforced if the
> > > prog->enforce_expected_attach_type bit is set.
> > > i.e. prog->expected_attach_type is only enforced if this new helper
> > > is used by the prog.
> > >
> [ ... ]
>
> > > @@ -1725,6 +1733,10 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> > > case BPF_PROG_TYPE_CGROUP_SOCK:
> > > case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
> > > return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
> > > + case BPF_PROG_TYPE_CGROUP_SKB:
> > > + return prog->enforce_expected_attach_type &&
> > > + prog->expected_attach_type != attach_type ?
> > > + -EINVAL : 0;
> > > default:
> > > return 0;
> > > }
> [ ... ]
>
> > > diff --git a/net/core/filter.c b/net/core/filter.c
> > > index 97916eedfe69..ca57ef25279c 100644
> > > --- a/net/core/filter.c
> > > +++ b/net/core/filter.c
> > > @@ -5426,6 +5426,24 @@ static const struct bpf_func_proto bpf_tcp_sock_proto = {
> > > .arg1_type = ARG_PTR_TO_SOCK_COMMON,
> > > };
> > >
> > > +BPF_CALL_1(bpf_tcp_enter_cwr, struct tcp_sock *, tp)
> > > +{
> > > + struct sock *sk = (struct sock *)tp;
> > > +
> > > + if (sk->sk_state == TCP_ESTABLISHED) {
> > > + tcp_enter_cwr(sk);
> > > + return 0;
> > > + }
> > > +
> > > + return -EINVAL;
> > > +}
> > > +
> > > +static const struct bpf_func_proto bpf_tcp_enter_cwr_proto = {
> > > + .func = bpf_tcp_enter_cwr,
> > > + .gpl_only = false,
> > > + .ret_type = RET_INTEGER,
> > > + .arg1_type = ARG_PTR_TO_TCP_SOCK,
> > > +};
> > > #endif /* CONFIG_INET */
> > >
> > > bool bpf_helper_changes_pkt_data(void *func)
> > > @@ -5585,6 +5603,13 @@ cg_skb_func_proto(enum bpf_func_id func_id, struct bpf_prog *prog)
> > > #ifdef CONFIG_INET
> > > case BPF_FUNC_tcp_sock:
> > > return &bpf_tcp_sock_proto;
> >
> > [...]
> > > + case BPF_FUNC_tcp_enter_cwr:
> > > + if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS) {
> > > + prog->enforce_expected_attach_type = 1;
> > > + return &bpf_tcp_enter_cwr_proto;
> > Instead of this back and forth with enforce_expected_attach_type, can we
> > just do here:
> >
> > if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS)
> > return &bpf_tcp_enter_cwr_proto;
> > else
> > return null;
> >
> > Wouldn't it have the same effect?
> The attr->expected_attach_type is currently ignored (i.e. not checked)
> during the bpf load time.
But nothing stops you form checking prog->expected_attach_type in
the cg_skb_func_proto, right? That is done at the time of loading.
So depending on prog->expected_attach_type just return null or non-null
and the verifier will take care of the rest. Then, at attach time just
make sure we are attaching it to the expected_attach_type.
We also should not have any existing use cases for
BPF_FUNC_tcp_enter_cwr I suppose.
In other words, why something like below won't work? Am I missing something?
---
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index b155cd17c1bd..86dc7cd00f34 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1678,6 +1678,10 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
case BPF_PROG_TYPE_CGROUP_SOCK:
case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
+ case BPF_PROG_TYPE_CGROUP_SKB:
+ if (prog->expected_attach_type)
+ return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
+ return 0;
default:
return 0;
}
diff --git a/net/core/filter.c b/net/core/filter.c
index 7559d6835ecb..56f70468fc7a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5396,6 +5396,11 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
switch (func_id) {
case BPF_FUNC_get_local_storage:
return &bpf_get_local_storage_proto;
+ case BPF_FUNC_tcp_enter_cwr:
+ if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS)
+ return &bpf_tcp_enter_cwr_proto;
+ else
+ return NULL;
default:
return sk_filter_func_proto(func_id, prog);
}
>
> How to avoid breaking backward compatibility without selectively
> enforcing prog->expected_attach_type during attach time?
^ permalink raw reply related
* Re: [PATCH v3 bpf-next 1/4] bpf: enable program stats
From: Stanislav Fomichev @ 2019-02-26 3:10 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, davem@davemloft.net, daniel@iogearbox.net,
edumazet@google.com, netdev@vger.kernel.org, bpf@vger.kernel.org,
Kernel Team
In-Reply-To: <2eeb7f8d-d184-07d1-2b7b-76c93b4b1bfe@fb.com>
On 02/25, Alexei Starovoitov wrote:
> On 2/25/19 3:07 PM, Stanislav Fomichev wrote:
> >> +#define BPF_PROG_RUN(prog, ctx) ({ \
> >> + u32 ret; \
> >> + cant_sleep(); \
> >> + if (static_branch_unlikely(&bpf_stats_enabled_key)) { \
> >> + struct bpf_prog_stats *stats; \
> >> + u64 start = sched_clock(); \
> > QQ: why sched_clock() and not, for example, ktime_get_ns() which we do
> > in the bpf_test_run()? Or even why not local_clock?
> > I'm just wondering what king of trade off we are doing here
> > regarding precision vs run time cost.
>
>
> I'm making this decision based on documentation:
> Documentation/timers/timekeeping.txt
> "Compared to clock sources, sched_clock() has to be very fast: it is
> called much more often, especially by the scheduler. If you have to do
> trade-offs between accuracy compared to the clock source, you may
> sacrifice accuracy for speed in sched_clock()."
So sched_clock is fast, but imprecise; and ktime_get_ns (and
lock_clock?) are slow(er), but more precise?
If that's the case, would it make sense to use a more precise
measurement? I suppose the BPF program execution time is on the order of
nanoseconds and if sched_close has msec or usec resolution, all we get is
essentially noise?
I understand that you want this feature to have almost no overhead, but
since it's gated by the static key, should we aim for a higher precision?
^ permalink raw reply
* Re: [PATCH net-next v3 5/6] devlink: hold a reference to the netdevice around ethtool compat
From: Jakub Kicinski @ 2019-02-26 3:07 UTC (permalink / raw)
To: Jiri Pirko; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190225103127.4f17f900@cakuba.netronome.com>
On Mon, 25 Feb 2019 10:31:27 -0800, Jakub Kicinski wrote:
> +static bool devlink_is_registered(struct devlink *devlink)
> +{
> + return list_empty(&devlink->list);
> +}
Nevermind, this won't really help the drivers that much, those
registering devlink after netdevs will have to take care of the
potential race by marking the devlink instance as dead in their priv
data structure from the thread doing the devlink_unregister, using
their own locking.
^ permalink raw reply
* Re: [PATCH net] ipv4: Add ICMPv6 support when parse route ipproto
From: David Ahern @ 2019-02-26 2:23 UTC (permalink / raw)
To: Hangbin Liu; +Cc: Sabrina Dubroca, netdev, Roopa Prabhu, David S . Miller
In-Reply-To: <20190226021739.GS10051@dhcp-12-139.nay.redhat.com>
On 2/25/19 7:17 PM, Hangbin Liu wrote:
> I also thought about this issue. Currently we didn't check the ipproto in both
> IPv4 and IPv6. You can set icmp in ip6 rules or icmpv6 in ipv4 rules.
> This looks don't make any serious problem. It's just a user mis-configuration,
> the kernel check the proto number and won't match normal IP/IPv6 headers.
>
> But yes, we should make it more strict, do you think if I should add a new
> rtm_getroute_parse_ip6_proto() function, or just add a family parameter
> in previous function?
I see now. rtm_getroute_parse_ip_proto is used for ipv4 and ipv6. For v4
IPPROTO_ICMPV6 should not be allowed and for v6 IPPROTO_ICMP should
fail. You could a version argument to rtm_getroute_parse_ip_proto and
fail as needed.
^ permalink raw reply
* Re: [PATCH net] ipv4: Add ICMPv6 support when parse route ipproto
From: Hangbin Liu @ 2019-02-26 2:17 UTC (permalink / raw)
To: David Ahern; +Cc: Sabrina Dubroca, netdev, Roopa Prabhu, David S . Miller
In-Reply-To: <212750de-2d6a-ebb9-5079-bacf234f6cf6@gmail.com>
On Mon, Feb 25, 2019 at 11:46:51AM -0700, David Ahern wrote:
> On 2/25/19 4:14 AM, Sabrina Dubroca wrote:
> > 2019-02-25, 15:47:00 +0800, Hangbin Liu wrote:
> >> @@ -14,6 +15,7 @@ int rtm_getroute_parse_ip_proto(struct nlattr *attr, u8 *ip_proto,
> >> case IPPROTO_TCP:
> >> case IPPROTO_UDP:
> >> case IPPROTO_ICMP:
> >> + case IPPROTO_ICMPV6:
> >
> > Is IPPROTO_ICMPV6 supposed to be valid in the IPv4 code path calling
> > this function?
>
> no. I do not see how that makes sense.
I also thought about this issue. Currently we didn't check the ipproto in both
IPv4 and IPv6. You can set icmp in ip6 rules or icmpv6 in ipv4 rules.
This looks don't make any serious problem. It's just a user mis-configuration,
the kernel check the proto number and won't match normal IP/IPv6 headers.
But yes, we should make it more strict, do you think if I should add a new
rtm_getroute_parse_ip6_proto() function, or just add a family parameter
in previous function?
The #if IS_ENABLED(CONFIG_IPV6) guardian makes sense. I will fix it.
Thanks
Hangbin
^ permalink raw reply
* Re: [PATCH net] sit: use ipv6_mod_enabled to check if ipv6 is disabled
From: David Ahern @ 2019-02-26 2:15 UTC (permalink / raw)
To: Hangbin Liu; +Cc: Eric Dumazet, netdev, Stefano Brivio, David S . Miller
In-Reply-To: <20190226015533.GR10051@dhcp-12-139.nay.redhat.com>
On 2/25/19 6:55 PM, Hangbin Liu wrote:
> Just as I said, this issue only occurs when IPv6 is disabled at boot time
> as there is no IPv6 route entry. Disable ipv6 on specific interface should
> not be affected(IPv6 route/fib has inited). So I think use ipv6_mod_enabled()
> would be more suitable in this scenario. Did I miss something?
From a readability perspective the code path depends on whether ipv6 is
enabled on the device. I think it is better to leave that as it is.
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Michael S. Tsirkin @ 2019-02-26 2:08 UTC (permalink / raw)
To: si-wei liu
Cc: Samudrala, Sridhar, Siwei Liu, Jiri Pirko, Stephen Hemminger,
David Miller, Netdev, virtualization, virtio-dev,
Brandeburg, Jesse, Alexander Duyck, Jakub Kicinski, Jason Wang,
liran.alon
In-Reply-To: <e6a53bd1-83ab-f170-406a-03276e8c87e2@oracle.com>
On Mon, Feb 25, 2019 at 04:58:07PM -0800, si-wei liu wrote:
>
>
> On 2/22/2019 7:14 AM, Michael S. Tsirkin wrote:
> > On Thu, Feb 21, 2019 at 11:55:11PM -0800, si-wei liu wrote:
> > >
> > > On 2/21/2019 11:00 PM, Samudrala, Sridhar wrote:
> > > >
> > > > On 2/21/2019 7:33 PM, si-wei liu wrote:
> > > > >
> > > > > On 2/21/2019 5:39 PM, Michael S. Tsirkin wrote:
> > > > > > On Thu, Feb 21, 2019 at 05:14:44PM -0800, Siwei Liu wrote:
> > > > > > > Sorry for replying to this ancient thread. There was some remaining
> > > > > > > issue that I don't think the initial net_failover patch got addressed
> > > > > > > cleanly, see:
> > > > > > >
> > > > > > > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1815268
> > > > > > >
> > > > > > > The renaming of 'eth0' to 'ens4' fails because the udev userspace was
> > > > > > > not specifically writtten for such kernel automatic enslavement.
> > > > > > > Specifically, if it is a bond or team, the slave would typically get
> > > > > > > renamed *before* virtual device gets created, that's what udev can
> > > > > > > control (without getting netdev opened early by the other part of
> > > > > > > kernel) and other userspace components for e.g. initramfs,
> > > > > > > init-scripts can coordinate well in between. The in-kernel
> > > > > > > auto-enslavement of net_failover breaks this userspace convention,
> > > > > > > which don't provides a solution if user care about consistent naming
> > > > > > > on the slave netdevs specifically.
> > > > > > >
> > > > > > > Previously this issue had been specifically called out when IFF_HIDDEN
> > > > > > > and the 1-netdev was proposed, but no one gives out a solution to this
> > > > > > > problem ever since. Please share your mind how to proceed and solve
> > > > > > > this userspace issue if netdev does not welcome a 1-netdev model.
> > > > > > Above says:
> > > > > >
> > > > > > there's no motivation in the systemd/udevd community at
> > > > > > this point to refactor the rename logic and make it work well with
> > > > > > 3-netdev.
> > > > > >
> > > > > > What would the fix be? Skip slave devices?
> > > > > >
> > > > > There's nothing user can get if just skipping slave devices - the
> > > > > name is still unchanged and unpredictable e.g. eth0, or eth1 the
> > > > > next reboot, while the rest may conform to the naming scheme (ens3
> > > > > and such). There's no way one can fix this in userspace alone - when
> > > > > the failover is created the enslaved netdev was opened by the kernel
> > > > > earlier than the userspace is made aware of, and there's no
> > > > > negotiation protocol for kernel to know when userspace has done
> > > > > initial renaming of the interface. I would expect netdev list should
> > > > > at least provide the direction in general for how this can be
> > > > > solved...
> >
> > I was just wondering what did you mean when you said
> > "refactor the rename logic and make it work well with 3-netdev" -
> > was there a proposal udev rejected?
> No. I never believed this particular issue can be fixed in userspace alone.
> Previously someone had said it could be, but I never see any work or
> relevant discussion ever happened in various userspace communities (for e.g.
> dracut, initramfs-tools, systemd, udev, and NetworkManager). IMHO the root
> of the issue derives from the kernel, it makes more sense to start from
> netdev, work out and decide on a solution: see what can be done in the
> kernel in order to fix it, then after that engage userspace community for
> the feasibility...
>
> > Anyway, can we write a time diagram for what happens in which order that
> > leads to failure? That would help look for triggers that we can tie
> > into, or add new ones.
> >
>
> See attached diagram.
>
> >
> >
> >
> >
> > > > Is there an issue if slave device names are not predictable? The user/admin scripts are expected
> > > > to only work with the master failover device.
> > > Where does this expectation come from?
> > >
> > > Admin users may have ethtool or tc configurations that need to deal with
> > > predictable interface name. Third-party app which was built upon specifying
> > > certain interface name can't be modified to chase dynamic names.
> > >
> > > Specifically, we have pre-canned image that uses ethtool to fine tune VF
> > > offload settings post boot for specific workload. Those images won't work
> > > well if the name is constantly changing just after couple rounds of live
> > > migration.
> > It should be possible to specify the ethtool configuration on the
> > master and have it automatically propagated to the slave.
> >
> > BTW this is something we should look at IMHO.
> I was elaborating a few examples that the expectation and assumption that
> user/admin scripts only deal with master failover device is incorrect. It
> had never been taken good care of, although I did try to emphasize it from
> the very beginning.
>
> Basically what you said about propagating the ethtool configuration down to
> the slave is the key pursuance of 1-netdev model. However, what I am seeking
> now is any alternative that can also fix the specific udev rename problem,
> before concluding that 1-netdev is the only solution. Generally a 1-netdev
> scheme would take time to implement, while I'm trying to find a way out to
> fix this particular naming problem under 3-netdev.
>
> >
> > > > Moreover, you were suggesting hiding the lower slave devices anyway. There was some discussion
> > > > about moving them to a hidden network namespace so that they are not visible from the default namespace.
> > > > I looked into this sometime back, but did not find the right kernel api to create a network namespace within
> > > > kernel. If so, we could use this mechanism to simulate a 1-netdev model.
> > > Yes, that's one possible implementation (IMHO the key is to make 1-netdev
> > > model as much transparent to a real NIC as possible, while a hidden netns is
> > > just the vehicle). However, I recall there was resistance around this
> > > discussion that even the concept of hiding itself is a taboo for Linux
> > > netdev. I would like to summon potential alternatives before concluding
> > > 1-netdev is the only solution too soon.
> > >
> > > Thanks,
> > > -Siwei
> > Your scripts would not work at all then, right?
> At this point we don't claim images with such usage as SR-IOV live
> migrate-able. We would flag it as live migrate-able until this ethtool
> config issue is fully addressed and a transparent live migration solution
> emerges in upstream eventually.
>
>
> Thanks,
> -Siwei
> >
> >
> > > > > -Siwei
> > > > >
> > > > >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> >
>
>
> net_failover(kernel) | network.service (user) | systemd-udevd (user)
> --------------------------------------------------+------------------------------+--------------------------------------------
> (standby virtio-net and net_failover | |
> devices created and initialized, | |
> i.e. virtnet_probe()-> | |
> net_failover_create() | |
> was done.) | |
> | |
> | runs `ifup ens3' -> |
> | ip link set dev ens3 up |
> net_failover_open() | |
> dev_open(virtnet_dev) | |
> virtnet_open(virtnet_dev) | |
> netif_carrier_on(failover_dev) | |
> ... | |
> | |
> (VF hot plugged in) | |
> ixgbevf_probe() | |
> register_netdev(ixgbevf_netdev) | |
> netdev_register_kobject(ixgbevf_netdev) | |
> kobject_add(ixgbevf_dev) | |
> device_add(ixgbevf_dev) | |
> kobject_uevent(&ixgbevf_dev->kobj, KOBJ_ADD) | |
> netlink_broadcast() | |
> ... | |
> call_netdevice_notifiers(NETDEV_REGISTER) | |
> failover_event(..., NETDEV_REGISTER, ...) | |
> failover_slave_register(ixgbevf_netdev) | |
> net_failover_slave_register(ixgbevf_netdev) | |
> dev_open(ixgbevf_netdev) | |
> | |
> | |
> | | received ADD uevent from netlink fd
> | | ...
> | | udev-builtin-net_id.c:dev_pci_slot()
> | | (decided to renamed 'eth0' )
> | | ip link set dev eth0 name ens4
> (dev_change_name() returns -EBUSY as | |
> ixgbevf_netdev->flags has IFF_UP) | |
> | |
>
Given renaming slaves does not work anyway: would it work if we just
hard-coded slave names instead?
E.g.
1. fail slave renames
2. rename of failover to XX automatically renames standby to XXnsby
and primary to XXnpry
--
MST
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Michael S. Tsirkin @ 2019-02-26 2:05 UTC (permalink / raw)
To: Stephen Hemminger
Cc: si-wei liu, Samudrala, Sridhar, Siwei Liu, Jiri Pirko,
David Miller, Netdev, virtualization, virtio-dev,
Brandeburg, Jesse, Alexander Duyck, Jakub Kicinski, Jason Wang,
liran.alon
In-Reply-To: <20190225173912.26b93422@shemminger-XPS-13-9360>
On Mon, Feb 25, 2019 at 05:39:12PM -0800, Stephen Hemminger wrote:
> > >>> Moreover, you were suggesting hiding the lower slave devices anyway. There was some discussion
> > >>> about moving them to a hidden network namespace so that they are not visible from the default namespace.
> > >>> I looked into this sometime back, but did not find the right kernel api to create a network namespace within
> > >>> kernel. If so, we could use this mechanism to simulate a 1-netdev model.
> > >> Yes, that's one possible implementation (IMHO the key is to make 1-netdev
> > >> model as much transparent to a real NIC as possible, while a hidden netns is
> > >> just the vehicle). However, I recall there was resistance around this
> > >> discussion that even the concept of hiding itself is a taboo for Linux
> > >> netdev. I would like to summon potential alternatives before concluding
> > >> 1-netdev is the only solution too soon.
> > >>
> > >> Thanks,
> > >> -Siwei
> > > Your scripts would not work at all then, right?
> > At this point we don't claim images with such usage as SR-IOV live
> > migrate-able. We would flag it as live migrate-able until this ethtool
> > config issue is fully addressed and a transparent live migration
> > solution emerges in upstream eventually.
>
> The hyper-v netvsc with 1-dev model uses a timeout to allow udev to do its rename.
> I proposed a patch to key state change off of the udev rename, but that patch was
> rejected.
Of course that would mean nothing works without udev - was
that the objection? Could you help me find that discussion pls?
--
MST
^ permalink raw reply
* Re: [PATCH net] sit: use ipv6_mod_enabled to check if ipv6 is disabled
From: Hangbin Liu @ 2019-02-26 1:55 UTC (permalink / raw)
To: David Ahern; +Cc: Eric Dumazet, netdev, Stefano Brivio, David S . Miller
In-Reply-To: <df9f765f-2f73-d136-e020-de1b63963535@cumulusnetworks.com>
On Mon, Feb 25, 2019 at 09:57:42AM -0700, David Ahern wrote:
> On 2/25/19 9:39 AM, Eric Dumazet wrote:
> >>> On 02/24/2019 08:12 PM, Hangbin Liu wrote:
> >>>> ipv6_mod_enabled() is more safe and gentle to check if ipv6 is disabled
> >>>> at running time.
> >>>
> >>> Why is it better exactly ?
> >>>
> >>> IPv6 can be enabled on the host, but disabled per device
> >>>
> >>> /proc/sys/net/ipv6/conf/{name}/disable_ipv6
> >>
> >> Sorry, it looks I didn't make it clear in the commit description.
> >> This issue only occurs when IPv6 is disabled at boot time as there is
> >> no IPv6 route entry. Disable ipv6 on specific interface is not affected.
> >> So check ipv6_mod_enabled() is enough and we don't need to worry about
> >> the rcu_read_lock or the dev status.
> >>
> >> Should I update the commit description?
> >
> > Certainly. Are you telling us skb->dev could be NULL here ?
> >
> > Because rcu_read_lock() should already be asserted.
> >
>
> Same response as geneve. The existing check is more appropriate and
> relevant for the code path: is ipv6 enabled on this device versus is
> ipv6 enabled at all.
Hi David,
Just as I said, this issue only occurs when IPv6 is disabled at boot time
as there is no IPv6 route entry. Disable ipv6 on specific interface should
not be affected(IPv6 route/fib has inited). So I think use ipv6_mod_enabled()
would be more suitable in this scenario. Did I miss something?
Thanks
Hangbin
^ permalink raw reply
* Re: [PATCH] tools: testing: selftests: Remove duplicate headers
From: Michael Ellerman @ 2019-02-26 1:48 UTC (permalink / raw)
To: Souptick Joarder, bamv2005, shuah, davem, benh, paulus, adobriyan,
mathieu.desnoyers, peterz, paulmck, boqun.feng, john.stultz, tglx,
sboyd, akpm
Cc: linux-gpio, linux-kselftest, linux-kernel, netdev, linuxppc-dev,
linux-fsdevel, sabyasachi.linux
In-Reply-To: <20190223070901.GA10274@jordon-HP-15-Notebook-PC>
Souptick Joarder <jrdr.linux@gmail.com> writes:
> Remove duplicate headers which are included twice.
>
> Signed-off-by: Sabyasachi Gupta <sabyasachi.linux@gmail.com>
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> ---
...
> tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c | 1 -
I took this hunk via the powerpc tree.
> diff --git a/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c b/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
> index 167135b..af1b802 100644
> --- a/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
> +++ b/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
> @@ -11,7 +11,6 @@
> #include <sys/wait.h>
> #include <unistd.h>
> #include <setjmp.h>
> -#include <signal.h>
>
> #include "ebb.h"
cheers
^ 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