* Re: [PATCH net-next v8 2/3] net: phy: add support for clause 37 auto-negotiation
From: Tao Ren @ 2019-09-15 6:15 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Heiner Kallweit, David S . Miller,
Vladimir Oltean, Arun Parameswaran, Justin Chen,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
openbmc@lists.ozlabs.org
In-Reply-To: <20190914141752.GC27922@lunn.ch>
On 9/14/19 7:17 AM, Andrew Lunn wrote:
> On Mon, Sep 09, 2019 at 01:49:06PM -0700, Tao Ren wrote:
>> From: Heiner Kallweit <hkallweit1@gmail.com>
>>
>> This patch adds support for clause 37 1000Base-X auto-negotiation.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> Signed-off-by: Tao Ren <taoren@fb.com>
>> Tested-by: René van Dorst <opensource@vdorst.com>
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>
> Andrew
Thanks a lot, Andrew.
Cheers,
Tao
^ permalink raw reply
* [PATCH net-next 0/2] drop_monitor: Better sanitize notified packets
From: Ido Schimmel @ 2019-09-15 6:46 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, nhorman, jakub.kicinski, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
When working in 'packet' mode, drop monitor generates a notification
with a potentially truncated payload of the dropped packet. The payload
is copied from the MAC header, but I forgot to check that the MAC header
was set, so do it now.
Patch #1 sets the offsets to the various protocol layers in netdevsim,
so that it will continue to work after the MAC header check is added to
drop monitor in patch #2.
Ido Schimmel (2):
netdevsim: Set offsets to various protocol layers
drop_monitor: Better sanitize notified packets
drivers/net/netdevsim/dev.c | 3 +++
net/core/drop_monitor.c | 6 ++++++
2 files changed, 9 insertions(+)
--
2.21.0
^ permalink raw reply
* [PATCH net-next 1/2] netdevsim: Set offsets to various protocol layers
From: Ido Schimmel @ 2019-09-15 6:46 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, nhorman, jakub.kicinski, mlxsw, Ido Schimmel
In-Reply-To: <20190915064636.6884-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
The driver periodically generates "trapped" UDP packets that it then
passes on to devlink. Set the offsets to the various protocol layers.
This is a prerequisite to the next patch, where drop monitor is taught
to check that the offset to the MAC header was set.
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/netdevsim/dev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 7fba7b271a57..56576d4f34a5 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -374,12 +374,14 @@ static struct sk_buff *nsim_dev_trap_skb_build(void)
return NULL;
tot_len = sizeof(struct iphdr) + sizeof(struct udphdr) + data_len;
+ skb_reset_mac_header(skb);
eth = skb_put(skb, sizeof(struct ethhdr));
eth_random_addr(eth->h_dest);
eth_random_addr(eth->h_source);
eth->h_proto = htons(ETH_P_IP);
skb->protocol = htons(ETH_P_IP);
+ skb_set_network_header(skb, skb->len);
iph = skb_put(skb, sizeof(struct iphdr));
iph->protocol = IPPROTO_UDP;
iph->saddr = in_aton("192.0.2.1");
@@ -392,6 +394,7 @@ static struct sk_buff *nsim_dev_trap_skb_build(void)
iph->check = 0;
iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
+ skb_set_transport_header(skb, skb->len);
udph = skb_put_zero(skb, sizeof(struct udphdr) + data_len);
get_random_bytes(&udph->source, sizeof(u16));
get_random_bytes(&udph->dest, sizeof(u16));
--
2.21.0
^ permalink raw reply related
* [PATCH net-next 2/2] drop_monitor: Better sanitize notified packets
From: Ido Schimmel @ 2019-09-15 6:46 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, nhorman, jakub.kicinski, mlxsw, Ido Schimmel
In-Reply-To: <20190915064636.6884-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
When working in 'packet' mode, drop monitor generates a notification
with a potentially truncated payload of the dropped packet. The payload
is copied from the MAC header, but I forgot to check that the MAC header
was set, so do it now.
Fixes: ca30707dee2b ("drop_monitor: Add packet alert mode")
Fixes: 5e58109b1ea4 ("drop_monitor: Add support for packet alert mode for hardware drops")
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
net/core/drop_monitor.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index cc60cc22e2db..536e032d95c8 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -487,6 +487,9 @@ static void net_dm_packet_trace_kfree_skb_hit(void *ignore,
struct sk_buff *nskb;
unsigned long flags;
+ if (!skb_mac_header_was_set(skb))
+ return;
+
nskb = skb_clone(skb, GFP_ATOMIC);
if (!nskb)
return;
@@ -900,6 +903,9 @@ net_dm_hw_packet_probe(struct sk_buff *skb,
struct sk_buff *nskb;
unsigned long flags;
+ if (!skb_mac_header_was_set(skb))
+ return;
+
nskb = skb_clone(skb, GFP_ATOMIC);
if (!nskb)
return;
--
2.21.0
^ permalink raw reply related
* Re: [patch iproute2-next 2/2] devlink: extend reload command to add support for network namespace change
From: Ido Schimmel @ 2019-09-15 7:16 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, idosch, dsahern, jakub.kicinski, tariqt, saeedm,
kuznet, yoshfuji, shuah, mlxsw
In-Reply-To: <20190914065757.27295-2-jiri@resnulli.us>
On Sat, Sep 14, 2019 at 08:57:57AM +0200, Jiri Pirko wrote:
> diff --git a/man/man8/devlink-dev.8 b/man/man8/devlink-dev.8
> index 1804463b2321..0e1a5523fa7b 100644
> --- a/man/man8/devlink-dev.8
> +++ b/man/man8/devlink-dev.8
> @@ -25,6 +25,13 @@ devlink-dev \- devlink device configuration
> .ti -8
> .B devlink dev help
>
> +.ti -8
> +.BR "devlink dev set"
> +.IR DEV
> +.RI "[ "
> +.BI "netns { " PID " | " NAME " | " ID " }
> +.RI "]"
> +
> .ti -8
> .BR "devlink dev eswitch set"
> .IR DEV
> @@ -92,6 +99,11 @@ Format is:
> .in +2
> BUS_NAME/BUS_ADDRESS
>
> +.SS devlink dev set - sets devlink device attributes
> +
> +.TP
> +.BI "netns { " PID " | " NAME " | " ID " }
This looks like leftover from previous version?
> +
> .SS devlink dev eswitch show - display devlink device eswitch attributes
> .SS devlink dev eswitch set - sets devlink device eswitch attributes
>
> --
> 2.21.0
>
^ permalink raw reply
* Re: [patch net-next 02/15] net: fib_notifier: make FIB notifier per-netns
From: Ido Schimmel @ 2019-09-15 8:06 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, idosch, dsahern, jakub.kicinski, tariqt, saeedm,
kuznet, yoshfuji, shuah, mlxsw
In-Reply-To: <20190914064608.26799-3-jiri@resnulli.us>
On Sat, Sep 14, 2019 at 08:45:55AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Currently all users of FIB notifier only cares about events in init_net.
s/cares/care/
> Later in this patchset, users get interested in other namespaces too.
> However, for every registered block user is interested only about one
> namespace. Make the FIB notifier registration per-netns and avoid
> unnecessary calls of notifier block for other namespaces.
...
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c b/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c
> index 5d20d615663e..fe0cc969cf94 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c
> @@ -248,9 +248,6 @@ static int mlx5_lag_fib_event(struct notifier_block *nb,
> struct net_device *fib_dev;
> struct fib_info *fi;
>
> - if (!net_eq(info->net, &init_net))
> - return NOTIFY_DONE;
I don't see anymore uses of 'info->net'. Can it be removed from 'struct
fib_notifier_info' ?
^ permalink raw reply
* Re: [patch net-next 03/15] net: fib_notifier: propagate possible error during fib notifier registration
From: Ido Schimmel @ 2019-09-15 8:17 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, idosch, dsahern, jakub.kicinski, tariqt, saeedm,
kuznet, yoshfuji, shuah, mlxsw
In-Reply-To: <20190914064608.26799-4-jiri@resnulli.us>
On Sat, Sep 14, 2019 at 08:45:56AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Unlike events for registered notifier, during the registration, the
> errors that happened for the block being registered are not propagated
> up to the caller. For fib rules, this is already present, but not for
What do you mean by "already present" ? You added it below for rules as
well...
> fib entries. So make sure the error is propagated for those as well.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> include/net/ip_fib.h | 2 +-
> net/core/fib_notifier.c | 2 --
> net/core/fib_rules.c | 11 ++++++++---
> net/ipv4/fib_notifier.c | 4 +---
> net/ipv4/fib_trie.c | 31 ++++++++++++++++++++++---------
> net/ipv4/ipmr_base.c | 22 +++++++++++++++-------
> net/ipv6/ip6_fib.c | 36 ++++++++++++++++++++++++------------
> 7 files changed, 71 insertions(+), 37 deletions(-)
>
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index 4cec9ecaa95e..caae0fa610aa 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
> @@ -229,7 +229,7 @@ int __net_init fib4_notifier_init(struct net *net);
> void __net_exit fib4_notifier_exit(struct net *net);
>
> void fib_info_notify_update(struct net *net, struct nl_info *info);
> -void fib_notify(struct net *net, struct notifier_block *nb);
> +int fib_notify(struct net *net, struct notifier_block *nb);
>
> struct fib_table {
> struct hlist_node tb_hlist;
> diff --git a/net/core/fib_notifier.c b/net/core/fib_notifier.c
> index b965f3c0ec9a..fbd029425638 100644
> --- a/net/core/fib_notifier.c
> +++ b/net/core/fib_notifier.c
> @@ -65,8 +65,6 @@ static int fib_net_dump(struct net *net, struct notifier_block *nb)
>
> rcu_read_lock();
> list_for_each_entry_rcu(ops, &fn_net->fib_notifier_ops, list) {
> - int err;
Looks like this should have been removed in previous patch
> -
> if (!try_module_get(ops->owner))
> continue;
> err = ops->fib_dump(net, nb);
> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
> index 28cbf07102bc..592d8aef90e3 100644
> --- a/net/core/fib_rules.c
> +++ b/net/core/fib_rules.c
> @@ -354,15 +354,20 @@ int fib_rules_dump(struct net *net, struct notifier_block *nb, int family)
> {
> struct fib_rules_ops *ops;
> struct fib_rule *rule;
> + int err = 0;
>
> ops = lookup_rules_ops(net, family);
> if (!ops)
> return -EAFNOSUPPORT;
> - list_for_each_entry_rcu(rule, &ops->rules_list, list)
> - call_fib_rule_notifier(nb, FIB_EVENT_RULE_ADD, rule, family);
> + list_for_each_entry_rcu(rule, &ops->rules_list, list) {
> + err = call_fib_rule_notifier(nb, FIB_EVENT_RULE_ADD,
> + rule, family);
Here you add it for rules
> + if (err)
> + break;
> + }
> rules_ops_put(ops);
>
> - return 0;
> + return err;
> }
> EXPORT_SYMBOL_GPL(fib_rules_dump);
^ permalink raw reply
* Re: [PATCH] igb/igc: Don't warn on fatal read failures when the device is removed
From: Feng Tang @ 2019-09-15 8:27 UTC (permalink / raw)
To: Lyude Paul
Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
Neftin, Sasha, Kirsher, Jeffrey T, David S. Miller,
linux-kernel@vger.kernel.org
In-Reply-To: <20190822183318.27634-1-lyude@redhat.com>
On Fri, Aug 23, 2019 at 02:33:18AM +0800, Lyude Paul wrote:
> Fatal read errors are worth warning about, unless of course the device
> was just unplugged from the machine - something that's a rather normal
> occurence when the igb/igc adapter is located on a Thunderbolt dock. So,
> let's only WARN() if there's a fatal read error while the device is
> still present.
>
> This fixes the following WARN splat that's been appearing whenever I
> unplug my Caldigit TS3 Thunderbolt dock from my laptop:
>
> igb 0000:09:00.0 enp9s0: PCIe link lost
> ------------[ cut here ]------------
> igb: Failed to read reg 0x18!
> WARNING: CPU: 7 PID: 516 at
> drivers/net/ethernet/intel/igb/igb_main.c:756 igb_rd32+0x57/0x6a [igb]
> Modules linked in: igb dca thunderbolt fuse vfat fat elan_i2c mei_wdt
> mei_hdcp i915 wmi_bmof intel_wmi_thunderbolt iTCO_wdt
> iTCO_vendor_support x86_pkg_temp_thermal intel_powerclamp joydev
> coretemp crct10dif_pclmul crc32_pclmul i2c_algo_bit ghash_clmulni_intel
> intel_cstate drm_kms_helper intel_uncore syscopyarea sysfillrect
> sysimgblt fb_sys_fops intel_rapl_perf intel_xhci_usb_role_switch mei_me
> drm roles idma64 i2c_i801 ucsi_acpi typec_ucsi mei intel_lpss_pci
> processor_thermal_device typec intel_pch_thermal intel_soc_dts_iosf
> intel_lpss int3403_thermal thinkpad_acpi wmi int340x_thermal_zone
> ledtrig_audio int3400_thermal acpi_thermal_rel acpi_pad video
> pcc_cpufreq ip_tables serio_raw nvme nvme_core crc32c_intel uas
> usb_storage e1000e i2c_dev
> CPU: 7 PID: 516 Comm: kworker/u16:3 Not tainted 5.2.0-rc1Lyude-Test+ #14
> Hardware name: LENOVO 20L8S2N800/20L8S2N800, BIOS N22ET35W (1.12 ) 04/09/2018
> Workqueue: kacpi_hotplug acpi_hotplug_work_fn
> RIP: 0010:igb_rd32+0x57/0x6a [igb]
> Code: 87 b8 fc ff ff 48 c7 47 08 00 00 00 00 48 c7 c6 33 42 9b c0 4c 89
> c7 e8 47 45 cd dc 89 ee 48 c7 c7 43 42 9b c0 e8 c1 94 71 dc <0f> 0b eb
> 08 8b 00 ff c0 75 b0 eb c8 44 89 e0 5d 41 5c c3 0f 1f 44
> RSP: 0018:ffffba5801cf7c48 EFLAGS: 00010286
> RAX: 0000000000000000 RBX: ffff9e7956608840 RCX: 0000000000000007
> RDX: 0000000000000000 RSI: ffffba5801cf7b24 RDI: ffff9e795e3d6a00
> RBP: 0000000000000018 R08: 000000009dec4a01 R09: ffffffff9e61018f
> R10: 0000000000000000 R11: ffffba5801cf7ae5 R12: 00000000ffffffff
> R13: ffff9e7956608840 R14: ffff9e795a6f10b0 R15: 0000000000000000
> FS: 0000000000000000(0000) GS:ffff9e795e3c0000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000564317bc4088 CR3: 000000010e00a006 CR4: 00000000003606e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> igb_release_hw_control+0x1a/0x30 [igb]
> igb_remove+0xc5/0x14b [igb]
> pci_device_remove+0x3b/0x93
> device_release_driver_internal+0xd7/0x17e
> pci_stop_bus_device+0x36/0x75
> pci_stop_bus_device+0x66/0x75
> pci_stop_bus_device+0x66/0x75
> pci_stop_and_remove_bus_device+0xf/0x19
> trim_stale_devices+0xc5/0x13a
> ? __pm_runtime_resume+0x6e/0x7b
> trim_stale_devices+0x103/0x13a
> ? __pm_runtime_resume+0x6e/0x7b
> trim_stale_devices+0x103/0x13a
> acpiphp_check_bridge+0xd8/0xf5
> acpiphp_hotplug_notify+0xf7/0x14b
> ? acpiphp_check_bridge+0xf5/0xf5
> acpi_device_hotplug+0x357/0x3b5
> acpi_hotplug_work_fn+0x1a/0x23
> process_one_work+0x1a7/0x296
> worker_thread+0x1a8/0x24c
> ? process_scheduled_works+0x2c/0x2c
> kthread+0xe9/0xee
> ? kthread_destroy_worker+0x41/0x41
> ret_from_fork+0x35/0x40
> ---[ end trace 252bf10352c63d22 ]---
>
Thanks for the fix.
Acked-by: Feng Tang <feng.tang@intel.com>
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Fixes: 47e16692b26b ("igb/igc: warn when fatal read failure happens")
> Cc: Feng Tang <feng.tang@intel.com>
> Cc: Sasha Neftin <sasha.neftin@intel.com>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 3 ++-
> drivers/net/ethernet/intel/igc/igc_main.c | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index e5b7e638df28..1a7f7cd28df9 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -753,7 +753,8 @@ u32 igb_rd32(struct e1000_hw *hw, u32 reg)
> struct net_device *netdev = igb->netdev;
> hw->hw_addr = NULL;
> netdev_err(netdev, "PCIe link lost\n");
> - WARN(1, "igb: Failed to read reg 0x%x!\n", reg);
> + WARN(pci_device_is_present(igb->pdev),
> + "igb: Failed to read reg 0x%x!\n", reg);
> }
>
> return value;
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 28072b9aa932..f873a4b35eaf 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -3934,7 +3934,8 @@ u32 igc_rd32(struct igc_hw *hw, u32 reg)
> hw->hw_addr = NULL;
> netif_device_detach(netdev);
> netdev_err(netdev, "PCIe link lost, device now detached\n");
> - WARN(1, "igc: Failed to read reg 0x%x!\n", reg);
> + WARN(pci_device_is_present(igc->pdev),
> + "igc: Failed to read reg 0x%x!\n", reg);
> }
>
> return value;
> --
> 2.21.0
^ permalink raw reply
* Re: [patch net-next 08/15] mlxsw: Register port netdevices into net of core
From: Ido Schimmel @ 2019-09-15 8:37 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, idosch, dsahern, jakub.kicinski, tariqt, saeedm,
kuznet, yoshfuji, shuah, mlxsw
In-Reply-To: <20190914064608.26799-9-jiri@resnulli.us>
On Sat, Sep 14, 2019 at 08:46:01AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> When creating netdevices for ports, put then under network namespace
s/then/them/
> that the core/parent devlink belongs to.
^ permalink raw reply
* Re: [patch net-next 09/15] mlxsw: Propagate extack down to register_fib_notifier()
From: Ido Schimmel @ 2019-09-15 8:39 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, idosch, dsahern, jakub.kicinski, tariqt, saeedm,
kuznet, yoshfuji, shuah, mlxsw
In-Reply-To: <20190914064608.26799-10-jiri@resnulli.us>
On Sat, Sep 14, 2019 at 08:46:02AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> During the devlink reaload the extack is present, so propagate it all
s/reaload/reload/
> the way down to register_fib_notifier() call in spectrum_router.c.
^ permalink raw reply
* Здравствуйте! Вас интересуют клиентские базы данных?
From: netdev @ 2019-09-14 23:39 UTC (permalink / raw)
To: netdev
Здравствуйте! Вас интересуют клиентские базы данных?
^ permalink raw reply
* Re: [patch net-next 14/15] net: devlink: allow to change namespaces during reload
From: Ido Schimmel @ 2019-09-15 8:58 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, idosch, dsahern, jakub.kicinski, tariqt, saeedm,
kuznet, yoshfuji, shuah, mlxsw
In-Reply-To: <20190914064608.26799-15-jiri@resnulli.us>
On Sat, Sep 14, 2019 at 08:46:07AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> All devlink instances are created in init_net and stay there for a
> lifetime. Allow user to be able to move devlink instances into
> namespaces during devlink reload operation. That ensures proper
> re-instantiation of driver objects, including netdevices.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/main.c | 4 +
> include/uapi/linux/devlink.h | 4 +
> net/core/devlink.c | 155 ++++++++++++++++++++--
> 3 files changed, 155 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
> index ef3f3d06ff1e..989d0882aaa9 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/main.c
> @@ -3942,6 +3942,10 @@ static int mlx4_devlink_reload_down(struct devlink *devlink,
> struct mlx4_dev *dev = &priv->dev;
> struct mlx4_dev_persistent *persist = dev->persist;
>
> + if (!net_eq(devlink_net(devlink), &init_net)) {
> + NL_SET_ERR_MSG_MOD(extack, "Namespace change is not supported");
> + return -EOPNOTSUPP;
> + }
Are you sure that this actually works? I see that you first invoke
reload_down(), then set the new namespace, then invoke reload_up().
So shouldn't this check be done in reload_up() callback instead?
> if (persist->num_vfs)
> mlx4_warn(persist->dev, "Reload performed on PF, will cause reset on operating Virtual Functions\n");
> mlx4_restart_one_down(persist->pdev);
> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
> index 580b7a2e40e1..b558ea88b766 100644
> --- a/include/uapi/linux/devlink.h
> +++ b/include/uapi/linux/devlink.h
> @@ -421,6 +421,10 @@ enum devlink_attr {
>
> DEVLINK_ATTR_RELOAD_FAILED, /* u8 0 or 1 */
>
> + DEVLINK_ATTR_NETNS_FD, /* u32 */
> + DEVLINK_ATTR_NETNS_PID, /* u32 */
> + DEVLINK_ATTR_NETNS_ID, /* u32 */
> +
> /* add new attributes above here, update the policy in devlink.c */
>
> __DEVLINK_ATTR_MAX,
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 362cbbcca225..2a5db95cce3c 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -435,8 +435,16 @@ static void devlink_nl_post_doit(const struct genl_ops *ops,
> {
> struct devlink *devlink;
>
> - devlink = devlink_get_from_info(info);
> - if (~ops->internal_flags & DEVLINK_NL_FLAG_NO_LOCK)
> + /* When devlink changes netns, it would not be found
> + * by devlink_get_from_info(). So try if it is stored first.
> + */
> + if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_DEVLINK) {
> + devlink = info->user_ptr[0];
> + } else {
> + devlink = devlink_get_from_info(info);
> + WARN_ON(IS_ERR(devlink));
> + }
> + if (!IS_ERR(devlink) && ~ops->internal_flags & DEVLINK_NL_FLAG_NO_LOCK)
> mutex_unlock(&devlink->lock);
> mutex_unlock(&devlink_mutex);
> }
> @@ -2675,6 +2683,73 @@ devlink_resources_validate(struct devlink *devlink,
> return err;
> }
>
> +static struct net *devlink_netns_get(struct sk_buff *skb,
> + struct devlink *devlink,
> + struct genl_info *info)
> +{
> + struct nlattr *netns_pid_attr = info->attrs[DEVLINK_ATTR_NETNS_PID];
> + struct nlattr *netns_fd_attr = info->attrs[DEVLINK_ATTR_NETNS_FD];
> + struct nlattr *netns_id_attr = info->attrs[DEVLINK_ATTR_NETNS_ID];
> + struct net *net;
> +
> + if (!!netns_pid_attr + !!netns_fd_attr + !!netns_id_attr > 1) {
> + NL_SET_ERR_MSG(info->extack, "multiple netns identifying attributes specified");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (netns_pid_attr) {
> + net = get_net_ns_by_pid(nla_get_u32(netns_pid_attr));
> + } else if (netns_fd_attr) {
> + net = get_net_ns_by_fd(nla_get_u32(netns_fd_attr));
> + } else if (netns_id_attr) {
> + net = get_net_ns_by_id(sock_net(skb->sk),
> + nla_get_u32(netns_id_attr));
> + if (!net)
> + net = ERR_PTR(-EINVAL);
> + } else {
> + WARN_ON(1);
> + net = ERR_PTR(-EINVAL);
> + }
> + if (IS_ERR(net)) {
> + NL_SET_ERR_MSG(info->extack, "Unknown network namespace");
> + return ERR_PTR(-EINVAL);
> + }
> + if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
> + put_net(net);
> + return ERR_PTR(-EPERM);
> + }
> + return net;
> +}
> +
> +static void devlink_param_notify(struct devlink *devlink,
> + unsigned int port_index,
> + struct devlink_param_item *param_item,
> + enum devlink_command cmd);
> +
> +static void devlink_reload_netns_change(struct devlink *devlink,
> + struct net *dest_net)
> +{
> + struct devlink_param_item *param_item;
> +
> + /* Userspace needs to be notified about devlink objects
> + * removed from original and entering new network namespace.
> + * The rest of the devlink objects are re-created during
> + * reload process so the notifications are generated separatelly.
> + */
> +
> + list_for_each_entry(param_item, &devlink->param_list, list)
> + devlink_param_notify(devlink, 0, param_item,
> + DEVLINK_CMD_PARAM_DEL);
> + devlink_notify(devlink, DEVLINK_CMD_DEL);
> +
> + devlink_net_set(devlink, dest_net);
> +
> + devlink_notify(devlink, DEVLINK_CMD_NEW);
> + list_for_each_entry(param_item, &devlink->param_list, list)
> + devlink_param_notify(devlink, 0, param_item,
> + DEVLINK_CMD_PARAM_NEW);
> +}
> +
> static bool devlink_reload_supported(struct devlink *devlink)
> {
> return devlink->ops->reload_down && devlink->ops->reload_up;
> @@ -2695,9 +2770,27 @@ bool devlink_is_reload_failed(const struct devlink *devlink)
> }
> EXPORT_SYMBOL_GPL(devlink_is_reload_failed);
>
> +static int devlink_reload(struct devlink *devlink, struct net *dest_net,
> + struct netlink_ext_ack *extack)
> +{
> + int err;
> +
> + err = devlink->ops->reload_down(devlink, extack);
> + if (err)
> + return err;
> +
> + if (dest_net && !net_eq(dest_net, devlink_net(devlink)))
> + devlink_reload_netns_change(devlink, dest_net);
> +
> + err = devlink->ops->reload_up(devlink, extack);
> + devlink_reload_failed_set(devlink, !!err);
> + return err;
> +}
> +
> static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
> {
> struct devlink *devlink = info->user_ptr[0];
> + struct net *dest_net = NULL;
> int err;
>
> if (!devlink_reload_supported(devlink))
> @@ -2708,11 +2801,20 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
> NL_SET_ERR_MSG_MOD(info->extack, "resources size validation failed");
> return err;
> }
> - err = devlink->ops->reload_down(devlink, info->extack);
> - if (err)
> - return err;
> - err = devlink->ops->reload_up(devlink, info->extack);
> - devlink_reload_failed_set(devlink, !!err);
> +
> + if (info->attrs[DEVLINK_ATTR_NETNS_PID] ||
> + info->attrs[DEVLINK_ATTR_NETNS_FD] ||
> + info->attrs[DEVLINK_ATTR_NETNS_ID]) {
> + dest_net = devlink_netns_get(skb, devlink, info);
Hmm, you're never using 'devlink' there, so I guess you can drop it.
> + if (IS_ERR(dest_net))
> + return PTR_ERR(dest_net);
> + }
> +
> + err = devlink_reload(devlink, dest_net, info->extack);
> +
> + if (dest_net)
> + put_net(dest_net);
> +
> return err;
> }
>
> @@ -5794,6 +5896,9 @@ static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
> [DEVLINK_ATTR_TRAP_NAME] = { .type = NLA_NUL_STRING },
> [DEVLINK_ATTR_TRAP_ACTION] = { .type = NLA_U8 },
> [DEVLINK_ATTR_TRAP_GROUP_NAME] = { .type = NLA_NUL_STRING },
> + [DEVLINK_ATTR_NETNS_PID] = { .type = NLA_U32 },
> + [DEVLINK_ATTR_NETNS_FD] = { .type = NLA_U32 },
> + [DEVLINK_ATTR_NETNS_ID] = { .type = NLA_U32 },
> };
>
> static const struct genl_ops devlink_nl_ops[] = {
> @@ -8061,9 +8166,43 @@ int devlink_compat_switch_id_get(struct net_device *dev,
> return 0;
> }
>
> +static void __net_exit devlink_pernet_pre_exit(struct net *net)
> +{
> + struct devlink *devlink;
> + int err;
> +
> + /* In case network namespace is getting destroyed, reload
> + * all devlink instances from this namespace into init_net.
> + */
> + mutex_lock(&devlink_mutex);
> + list_for_each_entry(devlink, &devlink_list, list) {
> + if (net_eq(devlink_net(devlink), net)) {
> + if (WARN_ON(!devlink_reload_supported(devlink)))
> + continue;
> + err = devlink_reload(devlink, &init_net, NULL);
> + if (err)
> + pr_warn("Failed to reload devlink instance into init_net\n");
> + }
> + }
> + mutex_unlock(&devlink_mutex);
> +}
> +
> +static struct pernet_operations devlink_pernet_ops __net_initdata = {
> + .pre_exit = devlink_pernet_pre_exit,
> +};
> +
> static int __init devlink_init(void)
> {
> - return genl_register_family(&devlink_nl_family);
> + int err;
> +
> + err = genl_register_family(&devlink_nl_family);
> + if (err)
> + goto out;
> + err = register_pernet_subsys(&devlink_pernet_ops);
> +
> +out:
> + WARN_ON(err);
> + return err;
> }
>
> subsys_initcall(devlink_init);
> --
> 2.21.0
>
^ permalink raw reply
* rt_uses_gateway was removed?
From: Julian Anastasov @ 2019-09-15 9:08 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Ido Schimmel
Hello,
Looks like I'm a bit late with the storm of changes
in the routing.
By default, after allocation rt_uses_gateway was set to 0.
Later it can be set to 1 if nh_gw is not the final route target,
i.e. it is indirect GW and not a target on LAN (the RT_SCOPE_LINK
check in rt_set_nexthop).
What remains hidden for the rt_uses_gateway semantic
is this code in rt_set_nexthop:
if (unlikely(!cached)) {
/* Routes we intend to cache in nexthop exception or
* FIB nexthop have the DST_NOCACHE bit clear.
* However, if we are unsuccessful at storing this
* route into the cache we really need to set it.
*/
if (!rt->rt_gateway)
rt->rt_gateway = daddr;
rt_add_uncached_list(rt);
}
and this code in rt_bind_exception:
if (!rt->rt_gateway)
rt->rt_gateway = daddr;
I.e. even if rt_uses_gateway remains 0, rt_gateway
can contain address, i.e. the target is on LAN and not
behind nh_gw.
Now I see commit 1550c171935d wrongly changes that to
"If rt_gw_family is set it implies rt_uses_gateway.".
As result, we set rt_gw_family while rt_uses_gateway was 0
for above cases. Think about it in this way: there should be
a reason why we used rt_uses_gateway flag instead a simple
"rt_gateway != 0" check, right?
Replacing rt->rt_gateway checks with rt_gw_family
checks is right but rt_uses_gateway checks should be put
back because they indicates the route has more hops to
target.
As the problem is related to some FNHW and non-cached
routes, redirects, etc the easiest way to see the problem is with
'ip route get LOCAL_IP oif eth0' where extra 'via GW' line is
shown.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [patch net-next 02/15] net: fib_notifier: make FIB notifier per-netns
From: Jiri Pirko @ 2019-09-15 9:37 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, idosch, dsahern, jakub.kicinski, tariqt, saeedm,
kuznet, yoshfuji, shuah, mlxsw
In-Reply-To: <20190915080602.GA11194@splinter>
Sun, Sep 15, 2019 at 10:06:02AM CEST, idosch@idosch.org wrote:
>On Sat, Sep 14, 2019 at 08:45:55AM +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Currently all users of FIB notifier only cares about events in init_net.
>
>s/cares/care/
ok
>
>> Later in this patchset, users get interested in other namespaces too.
>> However, for every registered block user is interested only about one
>> namespace. Make the FIB notifier registration per-netns and avoid
>> unnecessary calls of notifier block for other namespaces.
>
>...
>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c b/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c
>> index 5d20d615663e..fe0cc969cf94 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c
>> @@ -248,9 +248,6 @@ static int mlx5_lag_fib_event(struct notifier_block *nb,
>> struct net_device *fib_dev;
>> struct fib_info *fi;
>>
>> - if (!net_eq(info->net, &init_net))
>> - return NOTIFY_DONE;
>
>I don't see anymore uses of 'info->net'. Can it be removed from 'struct
>fib_notifier_info' ?
correct. I missed that.
^ permalink raw reply
* Re: [patch net-next 03/15] net: fib_notifier: propagate possible error during fib notifier registration
From: Jiri Pirko @ 2019-09-15 9:41 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, idosch, dsahern, jakub.kicinski, tariqt, saeedm,
kuznet, yoshfuji, shuah, mlxsw
In-Reply-To: <20190915081746.GB11194@splinter>
Sun, Sep 15, 2019 at 10:17:46AM CEST, idosch@idosch.org wrote:
>On Sat, Sep 14, 2019 at 08:45:56AM +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Unlike events for registered notifier, during the registration, the
>> errors that happened for the block being registered are not propagated
>> up to the caller. For fib rules, this is already present, but not for
>
>What do you mean by "already present" ? You added it below for rules as
>well...
Right, will fix.
>
>> fib entries. So make sure the error is propagated for those as well.
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> include/net/ip_fib.h | 2 +-
>> net/core/fib_notifier.c | 2 --
>> net/core/fib_rules.c | 11 ++++++++---
>> net/ipv4/fib_notifier.c | 4 +---
>> net/ipv4/fib_trie.c | 31 ++++++++++++++++++++++---------
>> net/ipv4/ipmr_base.c | 22 +++++++++++++++-------
>> net/ipv6/ip6_fib.c | 36 ++++++++++++++++++++++++------------
>> 7 files changed, 71 insertions(+), 37 deletions(-)
>>
>> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
>> index 4cec9ecaa95e..caae0fa610aa 100644
>> --- a/include/net/ip_fib.h
>> +++ b/include/net/ip_fib.h
>> @@ -229,7 +229,7 @@ int __net_init fib4_notifier_init(struct net *net);
>> void __net_exit fib4_notifier_exit(struct net *net);
>>
>> void fib_info_notify_update(struct net *net, struct nl_info *info);
>> -void fib_notify(struct net *net, struct notifier_block *nb);
>> +int fib_notify(struct net *net, struct notifier_block *nb);
>>
>> struct fib_table {
>> struct hlist_node tb_hlist;
>> diff --git a/net/core/fib_notifier.c b/net/core/fib_notifier.c
>> index b965f3c0ec9a..fbd029425638 100644
>> --- a/net/core/fib_notifier.c
>> +++ b/net/core/fib_notifier.c
>> @@ -65,8 +65,6 @@ static int fib_net_dump(struct net *net, struct notifier_block *nb)
>>
>> rcu_read_lock();
>> list_for_each_entry_rcu(ops, &fn_net->fib_notifier_ops, list) {
>> - int err;
>
>Looks like this should have been removed in previous patch
Correct, will move.
>
>> -
>> if (!try_module_get(ops->owner))
>> continue;
>> err = ops->fib_dump(net, nb);
>> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
>> index 28cbf07102bc..592d8aef90e3 100644
>> --- a/net/core/fib_rules.c
>> +++ b/net/core/fib_rules.c
>> @@ -354,15 +354,20 @@ int fib_rules_dump(struct net *net, struct notifier_block *nb, int family)
>> {
>> struct fib_rules_ops *ops;
>> struct fib_rule *rule;
>> + int err = 0;
>>
>> ops = lookup_rules_ops(net, family);
>> if (!ops)
>> return -EAFNOSUPPORT;
>> - list_for_each_entry_rcu(rule, &ops->rules_list, list)
>> - call_fib_rule_notifier(nb, FIB_EVENT_RULE_ADD, rule, family);
>> + list_for_each_entry_rcu(rule, &ops->rules_list, list) {
>> + err = call_fib_rule_notifier(nb, FIB_EVENT_RULE_ADD,
>> + rule, family);
>
>Here you add it for rules
>
>> + if (err)
>> + break;
>> + }
>> rules_ops_put(ops);
>>
>> - return 0;
>> + return err;
>> }
>> EXPORT_SYMBOL_GPL(fib_rules_dump);
^ permalink raw reply
* Re: [patch net-next 14/15] net: devlink: allow to change namespaces during reload
From: Jiri Pirko @ 2019-09-15 9:43 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, idosch, dsahern, jakub.kicinski, tariqt, saeedm,
kuznet, yoshfuji, shuah, mlxsw
In-Reply-To: <20190915085829.GE11194@splinter>
Sun, Sep 15, 2019 at 10:58:29AM CEST, idosch@idosch.org wrote:
>On Sat, Sep 14, 2019 at 08:46:07AM +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> All devlink instances are created in init_net and stay there for a
>> lifetime. Allow user to be able to move devlink instances into
>> namespaces during devlink reload operation. That ensures proper
>> re-instantiation of driver objects, including netdevices.
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> drivers/net/ethernet/mellanox/mlx4/main.c | 4 +
>> include/uapi/linux/devlink.h | 4 +
>> net/core/devlink.c | 155 ++++++++++++++++++++--
>> 3 files changed, 155 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
>> index ef3f3d06ff1e..989d0882aaa9 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/main.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/main.c
>> @@ -3942,6 +3942,10 @@ static int mlx4_devlink_reload_down(struct devlink *devlink,
>> struct mlx4_dev *dev = &priv->dev;
>> struct mlx4_dev_persistent *persist = dev->persist;
>>
>> + if (!net_eq(devlink_net(devlink), &init_net)) {
>> + NL_SET_ERR_MSG_MOD(extack, "Namespace change is not supported");
>> + return -EOPNOTSUPP;
>> + }
>
>Are you sure that this actually works? I see that you first invoke
>reload_down(), then set the new namespace, then invoke reload_up().
>
>So shouldn't this check be done in reload_up() callback instead?
Correct, need to fix this. But I need to do this in down phase so the
objects are not removed.
>
>> if (persist->num_vfs)
>> mlx4_warn(persist->dev, "Reload performed on PF, will cause reset on operating Virtual Functions\n");
>> mlx4_restart_one_down(persist->pdev);
>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>> index 580b7a2e40e1..b558ea88b766 100644
>> --- a/include/uapi/linux/devlink.h
>> +++ b/include/uapi/linux/devlink.h
>> @@ -421,6 +421,10 @@ enum devlink_attr {
>>
>> DEVLINK_ATTR_RELOAD_FAILED, /* u8 0 or 1 */
>>
>> + DEVLINK_ATTR_NETNS_FD, /* u32 */
>> + DEVLINK_ATTR_NETNS_PID, /* u32 */
>> + DEVLINK_ATTR_NETNS_ID, /* u32 */
>> +
>> /* add new attributes above here, update the policy in devlink.c */
>>
>> __DEVLINK_ATTR_MAX,
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index 362cbbcca225..2a5db95cce3c 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -435,8 +435,16 @@ static void devlink_nl_post_doit(const struct genl_ops *ops,
>> {
>> struct devlink *devlink;
>>
>> - devlink = devlink_get_from_info(info);
>> - if (~ops->internal_flags & DEVLINK_NL_FLAG_NO_LOCK)
>> + /* When devlink changes netns, it would not be found
>> + * by devlink_get_from_info(). So try if it is stored first.
>> + */
>> + if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_DEVLINK) {
>> + devlink = info->user_ptr[0];
>> + } else {
>> + devlink = devlink_get_from_info(info);
>> + WARN_ON(IS_ERR(devlink));
>> + }
>> + if (!IS_ERR(devlink) && ~ops->internal_flags & DEVLINK_NL_FLAG_NO_LOCK)
>> mutex_unlock(&devlink->lock);
>> mutex_unlock(&devlink_mutex);
>> }
>> @@ -2675,6 +2683,73 @@ devlink_resources_validate(struct devlink *devlink,
>> return err;
>> }
>>
>> +static struct net *devlink_netns_get(struct sk_buff *skb,
>> + struct devlink *devlink,
>> + struct genl_info *info)
>> +{
>> + struct nlattr *netns_pid_attr = info->attrs[DEVLINK_ATTR_NETNS_PID];
>> + struct nlattr *netns_fd_attr = info->attrs[DEVLINK_ATTR_NETNS_FD];
>> + struct nlattr *netns_id_attr = info->attrs[DEVLINK_ATTR_NETNS_ID];
>> + struct net *net;
>> +
>> + if (!!netns_pid_attr + !!netns_fd_attr + !!netns_id_attr > 1) {
>> + NL_SET_ERR_MSG(info->extack, "multiple netns identifying attributes specified");
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + if (netns_pid_attr) {
>> + net = get_net_ns_by_pid(nla_get_u32(netns_pid_attr));
>> + } else if (netns_fd_attr) {
>> + net = get_net_ns_by_fd(nla_get_u32(netns_fd_attr));
>> + } else if (netns_id_attr) {
>> + net = get_net_ns_by_id(sock_net(skb->sk),
>> + nla_get_u32(netns_id_attr));
>> + if (!net)
>> + net = ERR_PTR(-EINVAL);
>> + } else {
>> + WARN_ON(1);
>> + net = ERR_PTR(-EINVAL);
>> + }
>> + if (IS_ERR(net)) {
>> + NL_SET_ERR_MSG(info->extack, "Unknown network namespace");
>> + return ERR_PTR(-EINVAL);
>> + }
>> + if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
>> + put_net(net);
>> + return ERR_PTR(-EPERM);
>> + }
>> + return net;
>> +}
>> +
>> +static void devlink_param_notify(struct devlink *devlink,
>> + unsigned int port_index,
>> + struct devlink_param_item *param_item,
>> + enum devlink_command cmd);
>> +
>> +static void devlink_reload_netns_change(struct devlink *devlink,
>> + struct net *dest_net)
>> +{
>> + struct devlink_param_item *param_item;
>> +
>> + /* Userspace needs to be notified about devlink objects
>> + * removed from original and entering new network namespace.
>> + * The rest of the devlink objects are re-created during
>> + * reload process so the notifications are generated separatelly.
>> + */
>> +
>> + list_for_each_entry(param_item, &devlink->param_list, list)
>> + devlink_param_notify(devlink, 0, param_item,
>> + DEVLINK_CMD_PARAM_DEL);
>> + devlink_notify(devlink, DEVLINK_CMD_DEL);
>> +
>> + devlink_net_set(devlink, dest_net);
>> +
>> + devlink_notify(devlink, DEVLINK_CMD_NEW);
>> + list_for_each_entry(param_item, &devlink->param_list, list)
>> + devlink_param_notify(devlink, 0, param_item,
>> + DEVLINK_CMD_PARAM_NEW);
>> +}
>> +
>> static bool devlink_reload_supported(struct devlink *devlink)
>> {
>> return devlink->ops->reload_down && devlink->ops->reload_up;
>> @@ -2695,9 +2770,27 @@ bool devlink_is_reload_failed(const struct devlink *devlink)
>> }
>> EXPORT_SYMBOL_GPL(devlink_is_reload_failed);
>>
>> +static int devlink_reload(struct devlink *devlink, struct net *dest_net,
>> + struct netlink_ext_ack *extack)
>> +{
>> + int err;
>> +
>> + err = devlink->ops->reload_down(devlink, extack);
>> + if (err)
>> + return err;
>> +
>> + if (dest_net && !net_eq(dest_net, devlink_net(devlink)))
>> + devlink_reload_netns_change(devlink, dest_net);
>> +
>> + err = devlink->ops->reload_up(devlink, extack);
>> + devlink_reload_failed_set(devlink, !!err);
>> + return err;
>> +}
>> +
>> static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
>> {
>> struct devlink *devlink = info->user_ptr[0];
>> + struct net *dest_net = NULL;
>> int err;
>>
>> if (!devlink_reload_supported(devlink))
>> @@ -2708,11 +2801,20 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
>> NL_SET_ERR_MSG_MOD(info->extack, "resources size validation failed");
>> return err;
>> }
>> - err = devlink->ops->reload_down(devlink, info->extack);
>> - if (err)
>> - return err;
>> - err = devlink->ops->reload_up(devlink, info->extack);
>> - devlink_reload_failed_set(devlink, !!err);
>> +
>> + if (info->attrs[DEVLINK_ATTR_NETNS_PID] ||
>> + info->attrs[DEVLINK_ATTR_NETNS_FD] ||
>> + info->attrs[DEVLINK_ATTR_NETNS_ID]) {
>> + dest_net = devlink_netns_get(skb, devlink, info);
>
>Hmm, you're never using 'devlink' there, so I guess you can drop it.
Right, will do.
>
>> + if (IS_ERR(dest_net))
>> + return PTR_ERR(dest_net);
>> + }
>> +
>> + err = devlink_reload(devlink, dest_net, info->extack);
>> +
>> + if (dest_net)
>> + put_net(dest_net);
>> +
>> return err;
>> }
>>
>> @@ -5794,6 +5896,9 @@ static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
>> [DEVLINK_ATTR_TRAP_NAME] = { .type = NLA_NUL_STRING },
>> [DEVLINK_ATTR_TRAP_ACTION] = { .type = NLA_U8 },
>> [DEVLINK_ATTR_TRAP_GROUP_NAME] = { .type = NLA_NUL_STRING },
>> + [DEVLINK_ATTR_NETNS_PID] = { .type = NLA_U32 },
>> + [DEVLINK_ATTR_NETNS_FD] = { .type = NLA_U32 },
>> + [DEVLINK_ATTR_NETNS_ID] = { .type = NLA_U32 },
>> };
>>
>> static const struct genl_ops devlink_nl_ops[] = {
>> @@ -8061,9 +8166,43 @@ int devlink_compat_switch_id_get(struct net_device *dev,
>> return 0;
>> }
>>
>> +static void __net_exit devlink_pernet_pre_exit(struct net *net)
>> +{
>> + struct devlink *devlink;
>> + int err;
>> +
>> + /* In case network namespace is getting destroyed, reload
>> + * all devlink instances from this namespace into init_net.
>> + */
>> + mutex_lock(&devlink_mutex);
>> + list_for_each_entry(devlink, &devlink_list, list) {
>> + if (net_eq(devlink_net(devlink), net)) {
>> + if (WARN_ON(!devlink_reload_supported(devlink)))
>> + continue;
>> + err = devlink_reload(devlink, &init_net, NULL);
>> + if (err)
>> + pr_warn("Failed to reload devlink instance into init_net\n");
>> + }
>> + }
>> + mutex_unlock(&devlink_mutex);
>> +}
>> +
>> +static struct pernet_operations devlink_pernet_ops __net_initdata = {
>> + .pre_exit = devlink_pernet_pre_exit,
>> +};
>> +
>> static int __init devlink_init(void)
>> {
>> - return genl_register_family(&devlink_nl_family);
>> + int err;
>> +
>> + err = genl_register_family(&devlink_nl_family);
>> + if (err)
>> + goto out;
>> + err = register_pernet_subsys(&devlink_pernet_ops);
>> +
>> +out:
>> + WARN_ON(err);
>> + return err;
>> }
>>
>> subsys_initcall(devlink_init);
>> --
>> 2.21.0
>>
^ permalink raw reply
* Re: [patch iproute2-next 2/2] devlink: extend reload command to add support for network namespace change
From: Jiri Pirko @ 2019-09-15 9:44 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, idosch, dsahern, jakub.kicinski, tariqt, saeedm,
kuznet, yoshfuji, shuah, mlxsw
In-Reply-To: <20190915071639.GA8776@splinter>
Sun, Sep 15, 2019 at 09:16:39AM CEST, idosch@idosch.org wrote:
>On Sat, Sep 14, 2019 at 08:57:57AM +0200, Jiri Pirko wrote:
>> diff --git a/man/man8/devlink-dev.8 b/man/man8/devlink-dev.8
>> index 1804463b2321..0e1a5523fa7b 100644
>> --- a/man/man8/devlink-dev.8
>> +++ b/man/man8/devlink-dev.8
>> @@ -25,6 +25,13 @@ devlink-dev \- devlink device configuration
>> .ti -8
>> .B devlink dev help
>>
>> +.ti -8
>> +.BR "devlink dev set"
>> +.IR DEV
>> +.RI "[ "
>> +.BI "netns { " PID " | " NAME " | " ID " }
>> +.RI "]"
>> +
>> .ti -8
>> .BR "devlink dev eswitch set"
>> .IR DEV
>> @@ -92,6 +99,11 @@ Format is:
>> .in +2
>> BUS_NAME/BUS_ADDRESS
>>
>> +.SS devlink dev set - sets devlink device attributes
>> +
>> +.TP
>> +.BI "netns { " PID " | " NAME " | " ID " }
>
>This looks like leftover from previous version?
Will fix. Thanks!
>
>> +
>> .SS devlink dev eswitch show - display devlink device eswitch attributes
>> .SS devlink dev eswitch set - sets devlink device eswitch attributes
>>
>> --
>> 2.21.0
>>
^ permalink raw reply
* Re: pull-request: wireless-drivers-next 2019-09-14
From: Kalle Valo @ 2019-09-15 10:32 UTC (permalink / raw)
To: David Miller; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20190914.140843.945413345284987204.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: Kalle Valo <kvalo@codeaurora.org>
> Date: Sat, 14 Sep 2019 13:14:40 +0300
>
>> here's a pull request to net-next tree for v5.4, more info below. Please
>> let me know if there are any problems.
>
> Pulled, thanks Kalle.
Thanks for pulling this but I don't see it in net-next, maybe you forgot
to push? Nothing important, just making sure it didn't get lost.
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH v2] net: mdio: switch to using gpiod_get_optional()
From: Andy Shevchenko @ 2019-09-15 11:13 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andy Shevchenko, Andrew Lunn, Florian Fainelli, Heiner Kallweit,
David S. Miller, Linus Walleij, netdev, Linux Kernel Mailing List
In-Reply-To: <20190915060524.GC237523@dtor-ws>
On Sun, Sep 15, 2019 at 9:26 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Sat, Sep 14, 2019 at 08:09:33PM +0300, Andy Shevchenko wrote:
> > On Fri, Sep 13, 2019 at 03:55:47PM -0700, Dmitry Torokhov wrote:
> > > + mdiodev->reset_gpio = gpiod_get_optional(&mdiodev->dev,
> > > + "reset", GPIOD_OUT_LOW);
> > > + error = PTR_ERR_OR_ZERO(mdiodev->reset_gpio);
> > > + if (error)
> > > + return error;
> > > + if (mdiodev->reset_gpio)
> >
> > This is redundant check.
>
> I see that gpiod_* API handle NULL desc and usually return immediately,
> but frankly I am not that comfortable with it. I'm OK with functions
> that free/destroy objects that recognize NULL resources,
> but it is
> unusual for other types of APIs.
CLK, reset, ... frameworks do check for NULL pointer since they
provide an *_optional() getters. So, it's not unusual.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: pull-request: wireless-drivers-next 2019-09-14
From: David Miller @ 2019-09-15 11:38 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87muf5df3i.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Sun, 15 Sep 2019 13:32:49 +0300
> David Miller <davem@davemloft.net> writes:
>
>> From: Kalle Valo <kvalo@codeaurora.org>
>> Date: Sat, 14 Sep 2019 13:14:40 +0300
>>
>>> here's a pull request to net-next tree for v5.4, more info below. Please
>>> let me know if there are any problems.
>>
>> Pulled, thanks Kalle.
>
> Thanks for pulling this but I don't see it in net-next, maybe you forgot
> to push? Nothing important, just making sure it didn't get lost.
I feel asleep while the build test was running, it should be there now :-)
^ permalink raw reply
* Re: [bpf-next,v3] samples: bpf: add max_pckt_size option at xdp_adjust_tail
From: Daniel T. Lee @ 2019-09-15 3:24 UTC (permalink / raw)
To: Yonghong Song
Cc: Daniel Borkmann, Alexei Starovoitov, netdev@vger.kernel.org,
bpf@vger.kernel.org
In-Reply-To: <7add91c8-a22c-f10e-76a4-495d8be09c9b@fb.com>
On Sat, Sep 14, 2019 at 7:34 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 9/11/19 8:02 PM, Daniel T. Lee wrote:
> > Currently, at xdp_adjust_tail_kern.c, MAX_PCKT_SIZE is limited
> > to 600. To make this size flexible, a new map 'pcktsz' is added.
> >
> > By updating new packet size to this map from the userland,
> > xdp_adjust_tail_kern.o will use this value as a new max_pckt_size.
> >
> > If no '-P <MAX_PCKT_SIZE>' option is used, the size of maximum packet
> > will be 600 as a default.
> >
> > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> >
> > ---
> > Changes in v2:
> > - Change the helper to fetch map from 'bpf_map__next' to
> > 'bpf_object__find_map_fd_by_name'.
> >
> > samples/bpf/xdp_adjust_tail_kern.c | 23 +++++++++++++++++++----
> > samples/bpf/xdp_adjust_tail_user.c | 28 ++++++++++++++++++++++------
> > 2 files changed, 41 insertions(+), 10 deletions(-)
> >
> > diff --git a/samples/bpf/xdp_adjust_tail_kern.c b/samples/bpf/xdp_adjust_tail_kern.c
> > index 411fdb21f8bc..d6d84ffe6a7a 100644
> > --- a/samples/bpf/xdp_adjust_tail_kern.c
> > +++ b/samples/bpf/xdp_adjust_tail_kern.c
> > @@ -25,6 +25,13 @@
> > #define ICMP_TOOBIG_SIZE 98
> > #define ICMP_TOOBIG_PAYLOAD_SIZE 92
> >
> > +struct bpf_map_def SEC("maps") pcktsz = {
> > + .type = BPF_MAP_TYPE_ARRAY,
> > + .key_size = sizeof(__u32),
> > + .value_size = sizeof(__u32),
> > + .max_entries = 1,
> > +};
>
> We have new map definition format like in
> tools/testing/selftests/bpf/progs/bpf_flow.c.
> But looks like most samples/bpf still use SEC("maps").
> I guess we can leave it for now, and if needed,
> later on a massive conversion for all samples/bpf/
> bpf programs can be done.
>
Thanks for the detailed review!
Didn't notice there was an update with map definition.
This new map definition format looks very neat.
> > +
> > struct bpf_map_def SEC("maps") icmpcnt = {
> > .type = BPF_MAP_TYPE_ARRAY,
> > .key_size = sizeof(__u32),
> > @@ -64,7 +71,8 @@ static __always_inline void ipv4_csum(void *data_start, int data_size,
> > *csum = csum_fold_helper(*csum);
> > }
> >
> > -static __always_inline int send_icmp4_too_big(struct xdp_md *xdp)
> > +static __always_inline int send_icmp4_too_big(struct xdp_md *xdp,
> > + __u32 max_pckt_size)
> > {
> > int headroom = (int)sizeof(struct iphdr) + (int)sizeof(struct icmphdr);
> >
> > @@ -92,7 +100,7 @@ static __always_inline int send_icmp4_too_big(struct xdp_md *xdp)
> > orig_iph = data + off;
> > icmp_hdr->type = ICMP_DEST_UNREACH;
> > icmp_hdr->code = ICMP_FRAG_NEEDED;
> > - icmp_hdr->un.frag.mtu = htons(MAX_PCKT_SIZE-sizeof(struct ethhdr));
> > + icmp_hdr->un.frag.mtu = htons(max_pckt_size - sizeof(struct ethhdr));
> > icmp_hdr->checksum = 0;
> > ipv4_csum(icmp_hdr, ICMP_TOOBIG_PAYLOAD_SIZE, &csum);
> > icmp_hdr->checksum = csum;
> > @@ -118,14 +126,21 @@ static __always_inline int handle_ipv4(struct xdp_md *xdp)
> > {
> > void *data_end = (void *)(long)xdp->data_end;
> > void *data = (void *)(long)xdp->data;
> > + __u32 max_pckt_size = MAX_PCKT_SIZE;
> > + __u32 *pckt_sz;
> > + __u32 key = 0;
>
> The above two new definitions may the code not in
> reverse Christmas definition order, could you fix it?
>
I'll fix it right away!
> > int pckt_size = data_end - data;
> > int offset;
> >
> > - if (pckt_size > MAX_PCKT_SIZE) {
> > + pckt_sz = bpf_map_lookup_elem(&pcktsz, &key);
> > + if (pckt_sz && *pckt_sz)
> > + max_pckt_size = *pckt_sz;
> > +
> > + if (pckt_size > max_pckt_size) {
> > offset = pckt_size - ICMP_TOOBIG_SIZE;
> > if (bpf_xdp_adjust_tail(xdp, 0 - offset))
> > return XDP_PASS;
>
> We could have the following scenario:
> max_pckt_size = 1
> pckt_size = 2
> offset = -96
> bpf_xdp_adjust_tail return -EINVAL
> so we return XDP_PASS now
>
> Maybe you want to do
> if (pckt_size > max(max_pckt_size, ICMP_TOOBIG_SIZE)) {
> ...
> }
> as in original code, bpf_xdp_adjust_tail(...) already succeeds.
>
I'll try to fix this way.
> > - return send_icmp4_too_big(xdp);
> > + return send_icmp4_too_big(xdp, max_pckt_size);
> > }
> > return XDP_PASS;
> > }
> > diff --git a/samples/bpf/xdp_adjust_tail_user.c b/samples/bpf/xdp_adjust_tail_user.c
> > index a3596b617c4c..aef6c69a48a7 100644
> > --- a/samples/bpf/xdp_adjust_tail_user.c
> > +++ b/samples/bpf/xdp_adjust_tail_user.c
> > @@ -23,6 +23,7 @@
> > #include "libbpf.h"
> >
> > #define STATS_INTERVAL_S 2U
> > +#define MAX_PCKT_SIZE 600
> >
> > static int ifindex = -1;
> > static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> > @@ -72,6 +73,7 @@ static void usage(const char *cmd)
> > printf("Usage: %s [...]\n", cmd);
> > printf(" -i <ifname|ifindex> Interface\n");
> > printf(" -T <stop-after-X-seconds> Default: 0 (forever)\n");
> > + printf(" -P <MAX_PCKT_SIZE> Default: %u\n", MAX_PCKT_SIZE);
> > printf(" -S use skb-mode\n");
> > printf(" -N enforce native mode\n");
> > printf(" -F force loading prog\n");
> > @@ -85,13 +87,14 @@ int main(int argc, char **argv)
> > .prog_type = BPF_PROG_TYPE_XDP,
> > };
> > unsigned char opt_flags[256] = {};
> > - const char *optstr = "i:T:SNFh";
> > + const char *optstr = "i:T:P:SNFh";
> > struct bpf_prog_info info = {};
> > __u32 info_len = sizeof(info);
> > + __u32 max_pckt_size = 0;
> > + __u32 key = 0;
> > unsigned int kill_after_s = 0;
> > int i, prog_fd, map_fd, opt;
> > struct bpf_object *obj;
> > - struct bpf_map *map;
> > char filename[256];
> > int err;
> >
> > @@ -110,6 +113,9 @@ int main(int argc, char **argv)
> > case 'T':
> > kill_after_s = atoi(optarg);
> > break;
> > + case 'P':
> > + max_pckt_size = atoi(optarg);
> > + break;
> > case 'S':
> > xdp_flags |= XDP_FLAGS_SKB_MODE;
> > break;
> > @@ -150,12 +156,22 @@ int main(int argc, char **argv)
> > if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
> > return 1;
> >
> > - map = bpf_map__next(NULL, obj);
> > - if (!map) {
> > - printf("finding a map in obj file failed\n");
> > + /* update pcktsz map */
> > + if (max_pckt_size) {
> > + map_fd = bpf_object__find_map_fd_by_name(obj, "pcktsz");
> > + if (!map_fd) {
>
> Let us test map_fd and below prog_fd with '< 0" instead of "!= 0'.
> In this particular sample, "! = 0" is okay since we did not close
> stdin. But in programs if stdin is closed, the fd 0 may be reused
> for map_fd. Let us just keep good coding practice here.
>
I didn't think of those details. I'll update this right away!
Once again, I really appreciate your time and effort for the review.
Thank you.
Best,
Daniel
> > + printf("finding a pcktsz map in obj file failed\n");
> > + return 1;
> > + }
> > + bpf_map_update_elem(map_fd, &key, &max_pckt_size, BPF_ANY);
> > + }
> > +
> > + /* fetch icmpcnt map */
> > + map_fd = bpf_object__find_map_fd_by_name(obj, "icmpcnt");
> > + if (!map_fd) {
> > + printf("finding a icmpcnt map in obj file failed\n");
> > return 1;
> > }
> > - map_fd = bpf_map__fd(map);
> >
> > if (!prog_fd) {
> > printf("load_bpf_file: %s\n", strerror(errno));
> >
^ permalink raw reply
* [bpf-next,v4] samples: bpf: add max_pckt_size option at xdp_adjust_tail
From: Daniel T. Lee @ 2019-09-15 12:47 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov; +Cc: netdev, bpf
Currently, at xdp_adjust_tail_kern.c, MAX_PCKT_SIZE is limited
to 600. To make this size flexible, a new map 'pcktsz' is added.
By updating new packet size to this map from the userland,
xdp_adjust_tail_kern.o will use this value as a new max_pckt_size.
If no '-P <MAX_PCKT_SIZE>' option is used, the size of maximum packet
will be 600 as a default.
Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
---
Changes in v4:
- make pckt_size no less than ICMP_TOOBIG_SIZE
- Fix code style
Changes in v2:
- Change the helper to fetch map from 'bpf_map__next' to
'bpf_object__find_map_fd_by_name'.
samples/bpf/xdp_adjust_tail_kern.c | 23 +++++++++++++++++++----
samples/bpf/xdp_adjust_tail_user.c | 28 ++++++++++++++++++++++------
2 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/samples/bpf/xdp_adjust_tail_kern.c b/samples/bpf/xdp_adjust_tail_kern.c
index 411fdb21f8bc..8869bbb160d2 100644
--- a/samples/bpf/xdp_adjust_tail_kern.c
+++ b/samples/bpf/xdp_adjust_tail_kern.c
@@ -25,6 +25,13 @@
#define ICMP_TOOBIG_SIZE 98
#define ICMP_TOOBIG_PAYLOAD_SIZE 92
+struct bpf_map_def SEC("maps") pcktsz = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(__u32),
+ .value_size = sizeof(__u32),
+ .max_entries = 1,
+};
+
struct bpf_map_def SEC("maps") icmpcnt = {
.type = BPF_MAP_TYPE_ARRAY,
.key_size = sizeof(__u32),
@@ -64,7 +71,8 @@ static __always_inline void ipv4_csum(void *data_start, int data_size,
*csum = csum_fold_helper(*csum);
}
-static __always_inline int send_icmp4_too_big(struct xdp_md *xdp)
+static __always_inline int send_icmp4_too_big(struct xdp_md *xdp,
+ __u32 max_pckt_size)
{
int headroom = (int)sizeof(struct iphdr) + (int)sizeof(struct icmphdr);
@@ -92,7 +100,7 @@ static __always_inline int send_icmp4_too_big(struct xdp_md *xdp)
orig_iph = data + off;
icmp_hdr->type = ICMP_DEST_UNREACH;
icmp_hdr->code = ICMP_FRAG_NEEDED;
- icmp_hdr->un.frag.mtu = htons(MAX_PCKT_SIZE-sizeof(struct ethhdr));
+ icmp_hdr->un.frag.mtu = htons(max_pckt_size - sizeof(struct ethhdr));
icmp_hdr->checksum = 0;
ipv4_csum(icmp_hdr, ICMP_TOOBIG_PAYLOAD_SIZE, &csum);
icmp_hdr->checksum = csum;
@@ -118,14 +126,21 @@ static __always_inline int handle_ipv4(struct xdp_md *xdp)
{
void *data_end = (void *)(long)xdp->data_end;
void *data = (void *)(long)xdp->data;
+ __u32 max_pckt_size = MAX_PCKT_SIZE;
int pckt_size = data_end - data;
+ __u32 *pckt_sz;
+ __u32 key = 0;
int offset;
- if (pckt_size > MAX_PCKT_SIZE) {
+ pckt_sz = bpf_map_lookup_elem(&pcktsz, &key);
+ if (pckt_sz && *pckt_sz)
+ max_pckt_size = *pckt_sz;
+
+ if (pckt_size > max(max_pckt_size, ICMP_TOOBIG_SIZE)) {
offset = pckt_size - ICMP_TOOBIG_SIZE;
if (bpf_xdp_adjust_tail(xdp, 0 - offset))
return XDP_PASS;
- return send_icmp4_too_big(xdp);
+ return send_icmp4_too_big(xdp, max_pckt_size);
}
return XDP_PASS;
}
diff --git a/samples/bpf/xdp_adjust_tail_user.c b/samples/bpf/xdp_adjust_tail_user.c
index a3596b617c4c..99e965c68054 100644
--- a/samples/bpf/xdp_adjust_tail_user.c
+++ b/samples/bpf/xdp_adjust_tail_user.c
@@ -23,6 +23,7 @@
#include "libbpf.h"
#define STATS_INTERVAL_S 2U
+#define MAX_PCKT_SIZE 600
static int ifindex = -1;
static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
@@ -72,6 +73,7 @@ static void usage(const char *cmd)
printf("Usage: %s [...]\n", cmd);
printf(" -i <ifname|ifindex> Interface\n");
printf(" -T <stop-after-X-seconds> Default: 0 (forever)\n");
+ printf(" -P <MAX_PCKT_SIZE> Default: %u\n", MAX_PCKT_SIZE);
printf(" -S use skb-mode\n");
printf(" -N enforce native mode\n");
printf(" -F force loading prog\n");
@@ -85,13 +87,14 @@ int main(int argc, char **argv)
.prog_type = BPF_PROG_TYPE_XDP,
};
unsigned char opt_flags[256] = {};
- const char *optstr = "i:T:SNFh";
+ const char *optstr = "i:T:P:SNFh";
struct bpf_prog_info info = {};
__u32 info_len = sizeof(info);
+ __u32 max_pckt_size = 0;
+ __u32 key = 0;
unsigned int kill_after_s = 0;
int i, prog_fd, map_fd, opt;
struct bpf_object *obj;
- struct bpf_map *map;
char filename[256];
int err;
@@ -110,6 +113,9 @@ int main(int argc, char **argv)
case 'T':
kill_after_s = atoi(optarg);
break;
+ case 'P':
+ max_pckt_size = atoi(optarg);
+ break;
case 'S':
xdp_flags |= XDP_FLAGS_SKB_MODE;
break;
@@ -150,12 +156,22 @@ int main(int argc, char **argv)
if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
return 1;
- map = bpf_map__next(NULL, obj);
- if (!map) {
- printf("finding a map in obj file failed\n");
+ /* update pcktsz map */
+ if (max_pckt_size) {
+ map_fd = bpf_object__find_map_fd_by_name(obj, "pcktsz");
+ if (map_fd < 0) {
+ printf("finding a pcktsz map in obj file failed\n");
+ return 1;
+ }
+ bpf_map_update_elem(map_fd, &key, &max_pckt_size, BPF_ANY);
+ }
+
+ /* fetch icmpcnt map */
+ map_fd = bpf_object__find_map_fd_by_name(obj, "icmpcnt");
+ if (map_fd < 0) {
+ printf("finding a icmpcnt map in obj file failed\n");
return 1;
}
- map_fd = bpf_map__fd(map);
if (!prog_fd) {
printf("load_bpf_file: %s\n", strerror(errno));
--
2.20.1
^ permalink raw reply related
* RE: [PATCH net] udp: correct reuseport selection with connected sockets
From: Steve Zabele @ 2019-09-15 13:36 UTC (permalink / raw)
To: 'Willem de Bruijn', netdev
Cc: davem, edumazet, kraig, pabeni, mark.keaton,
'Willem de Bruijn'
In-Reply-To: <20190913011639.55895-1-willemdebruijn.kernel@gmail.com>
Hey Willem,
Thanks a bunch for getting this resolved, *very* much appreciated. This is a
really big help for us
Do you know if this will be backported to 4.19 stable, and if so when it
might be available??
Thanks again
Steve
-----Original Message-----
From: Willem de Bruijn [mailto:willemdebruijn.kernel@gmail.com]
Sent: Thursday, September 12, 2019 9:17 PM
To: netdev@vger.kernel.org
Cc: davem@davemloft.net; edumazet@google.com; kraig@google.com;
zabele@comcast.net; pabeni@redhat.com; mark.keaton@raytheon.com; Willem de
Bruijn
Subject: [PATCH net] udp: correct reuseport selection with connected sockets
From: Willem de Bruijn <willemb@google.com>
UDP reuseport groups can hold a mix unconnected and connected sockets.
Ensure that connections only receive all traffic to their 4-tuple.
Fast reuseport returns on the first reuseport match on the assumption
that all matches are equal. Only if connections are present, return to
the previous behavior of scoring all sockets.
Record if connections are present and if so (1) treat such connected
sockets as an independent match from the group, (2) only return
2-tuple matches from reuseport and (3) do not return on the first
2-tuple reuseport match to allow for a higher scoring match later.
New field has_conns is set without locks. No other fields in the
bitmap are modified at runtime and the field is only ever set
unconditionally, so an RMW cannot miss a change.
Fixes: e32ea7e74727 ("soreuseport: fast reuseport UDP socket selection")
Link:
http://lkml.kernel.org/r/CA+FuTSfRP09aJNYRt04SS6qj22ViiOEWaWmLAwX0psk8-PGNxw
@mail.gmail.com
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
I was unable to compile some older kernels, so the Fixes tag is based
on basic analysis, not bisected to by the regression test.
---
include/net/sock_reuseport.h | 20 +++++++++++++++++++-
net/core/sock_reuseport.c | 15 +++++++++++++--
net/ipv4/datagram.c | 2 ++
net/ipv4/udp.c | 5 +++--
net/ipv6/datagram.c | 2 ++
net/ipv6/udp.c | 5 +++--
6 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/include/net/sock_reuseport.h b/include/net/sock_reuseport.h
index d9112de85261..43f4a818d88f 100644
--- a/include/net/sock_reuseport.h
+++ b/include/net/sock_reuseport.h
@@ -21,7 +21,8 @@ struct sock_reuseport {
unsigned int synq_overflow_ts;
/* ID stays the same even after the size of socks[] grows. */
unsigned int reuseport_id;
- bool bind_inany;
+ unsigned int bind_inany:1;
+ unsigned int has_conns:1;
struct bpf_prog __rcu *prog; /* optional BPF sock
selector */
struct sock *socks[0]; /* array of sock pointers */
};
@@ -37,6 +38,23 @@ extern struct sock *reuseport_select_sock(struct sock
*sk,
extern int reuseport_attach_prog(struct sock *sk, struct bpf_prog *prog);
extern int reuseport_detach_prog(struct sock *sk);
+static inline bool reuseport_has_conns(struct sock *sk, bool set)
+{
+ struct sock_reuseport *reuse;
+ bool ret = false;
+
+ rcu_read_lock();
+ reuse = rcu_dereference(sk->sk_reuseport_cb);
+ if (reuse) {
+ if (set)
+ reuse->has_conns = 1;
+ ret = reuse->has_conns;
+ }
+ rcu_read_unlock();
+
+ return ret;
+}
+
int reuseport_get_id(struct sock_reuseport *reuse);
#endif /* _SOCK_REUSEPORT_H */
diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
index 9408f9264d05..f3ceec93f392 100644
--- a/net/core/sock_reuseport.c
+++ b/net/core/sock_reuseport.c
@@ -295,8 +295,19 @@ struct sock *reuseport_select_sock(struct sock *sk,
select_by_hash:
/* no bpf or invalid bpf result: fall back to hash usage */
- if (!sk2)
- sk2 = reuse->socks[reciprocal_scale(hash, socks)];
+ if (!sk2) {
+ int i, j;
+
+ i = j = reciprocal_scale(hash, socks);
+ while (reuse->socks[i]->sk_state == TCP_ESTABLISHED)
{
+ i++;
+ if (i >= reuse->num_socks)
+ i = 0;
+ if (i == j)
+ goto out;
+ }
+ sk2 = reuse->socks[i];
+ }
}
out:
diff --git a/net/ipv4/datagram.c b/net/ipv4/datagram.c
index 7bd29e694603..9a0fe0c2fa02 100644
--- a/net/ipv4/datagram.c
+++ b/net/ipv4/datagram.c
@@ -15,6 +15,7 @@
#include <net/sock.h>
#include <net/route.h>
#include <net/tcp_states.h>
+#include <net/sock_reuseport.h>
int __ip4_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int
addr_len)
{
@@ -69,6 +70,7 @@ int __ip4_datagram_connect(struct sock *sk, struct
sockaddr *uaddr, int addr_len
}
inet->inet_daddr = fl4->daddr;
inet->inet_dport = usin->sin_port;
+ reuseport_has_conns(sk, true);
sk->sk_state = TCP_ESTABLISHED;
sk_set_txhash(sk);
inet->inet_id = jiffies;
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index d88821c794fb..16486c8b708b 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -423,12 +423,13 @@ static struct sock *udp4_lib_lookup2(struct net *net,
score = compute_score(sk, net, saddr, sport,
daddr, hnum, dif, sdif);
if (score > badness) {
- if (sk->sk_reuseport) {
+ if (sk->sk_reuseport &&
+ sk->sk_state != TCP_ESTABLISHED) {
hash = udp_ehashfn(net, daddr, hnum,
saddr, sport);
result = reuseport_select_sock(sk, hash,
skb,
sizeof(struct
udphdr));
- if (result)
+ if (result && !reuseport_has_conns(sk,
false))
return result;
}
badness = score;
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 9ab897ded4df..96f939248d2f 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -27,6 +27,7 @@
#include <net/ip6_route.h>
#include <net/tcp_states.h>
#include <net/dsfield.h>
+#include <net/sock_reuseport.h>
#include <linux/errqueue.h>
#include <linux/uaccess.h>
@@ -254,6 +255,7 @@ int __ip6_datagram_connect(struct sock *sk, struct
sockaddr *uaddr,
goto out;
}
+ reuseport_has_conns(sk, true);
sk->sk_state = TCP_ESTABLISHED;
sk_set_txhash(sk);
out:
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 827fe7385078..5995fdc99d3f 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -158,13 +158,14 @@ static struct sock *udp6_lib_lookup2(struct net *net,
score = compute_score(sk, net, saddr, sport,
daddr, hnum, dif, sdif);
if (score > badness) {
- if (sk->sk_reuseport) {
+ if (sk->sk_reuseport &&
+ sk->sk_state != TCP_ESTABLISHED) {
hash = udp6_ehashfn(net, daddr, hnum,
saddr, sport);
result = reuseport_select_sock(sk, hash,
skb,
sizeof(struct
udphdr));
- if (result)
+ if (result && !reuseport_has_conns(sk,
false))
return result;
}
result = sk;
--
2.23.0.237.gc6a4ce50a0-goog
^ permalink raw reply related
* [PATCH V1 net] net: ena: don't wake up tx queue when down
From: sameehj @ 2019-09-15 14:29 UTC (permalink / raw)
To: davem, netdev
Cc: Sameeh Jubran, dwmw, zorik, matua, saeedb, msw, aliguori, nafea,
gtzalik, netanel, alisaidi, benh, akiyano
From: Sameeh Jubran <sameehj@amazon.com>
There is a race condition that can occur when calling ena_down().
The ena_clean_tx_irq() - which is a part of the napi handler -
function might wake up the tx queue when the queue is supposed
to be down (during recovery or changing the size of the queues
for example) This causes the ena_start_xmit() function to trigger
and possibly try to access the destroyed queues.
The race is illustrated below:
Flow A: Flow B(napi handler)
ena_down()
netif_carrier_off()
netif_tx_disable()
ena_clean_tx_irq()
netif_tx_wake_queue()
ena_napi_disable_all()
ena_destroy_all_io_queues()
After these flows the tx queue is active and ena_start_xmit() accesses
the destroyed queue which leads to a kernel panic.
fixes: 1738cd3ed342 (net: ena: Add a driver for Amazon Elastic Network Adapters (ENA))
Signed-off-by: Sameeh Jubran <sameehj@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 664e3ed97..d118ed4c5 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -823,7 +823,8 @@ static int ena_clean_tx_irq(struct ena_ring *tx_ring, u32 budget)
above_thresh =
ena_com_sq_have_enough_space(tx_ring->ena_com_io_sq,
ENA_TX_WAKEUP_THRESH);
- if (netif_tx_queue_stopped(txq) && above_thresh) {
+ if (netif_tx_queue_stopped(txq) && above_thresh &&
+ test_bit(ENA_FLAG_DEV_UP, &tx_ring->adapter->flags)) {
netif_tx_wake_queue(txq);
u64_stats_update_begin(&tx_ring->syncp);
tx_ring->tx_stats.queue_wakeup++;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v4 1/2] ethtool: implement Energy Detect Powerdown support via phy-tunable
From: Florian Fainelli @ 2019-09-15 15:08 UTC (permalink / raw)
To: Alexandru Ardelean, netdev, devicetree, linux-kernel
Cc: davem, robh+dt, mark.rutland, hkallweit1, andrew, mkubecek
In-Reply-To: <20190912162812.402-2-alexandru.ardelean@analog.com>
On 9/12/2019 9:28 AM, Alexandru Ardelean wrote:
> The `phy_tunable_id` has been named `ETHTOOL_PHY_EDPD` since it looks like
> this feature is common across other PHYs (like EEE), and defining
> `ETHTOOL_PHY_ENERGY_DETECT_POWER_DOWN` seems too long.
>
> The way EDPD works, is that the RX block is put to a lower power mode,
> except for link-pulse detection circuits. The TX block is also put to low
> power mode, but the PHY wakes-up periodically to send link pulses, to avoid
> lock-ups in case the other side is also in EDPD mode.
>
> Currently, there are 2 PHY drivers that look like they could use this new
> PHY tunable feature: the `adin` && `micrel` PHYs.
>
> The ADIN's datasheet mentions that TX pulses are at intervals of 1 second
> default each, and they can be disabled. For the Micrel KSZ9031 PHY, the
> datasheet does not mention whether they can be disabled, but mentions that
> they can modified.
>
> The way this change is structured, is similar to the PHY tunable downshift
> control:
> * a `ETHTOOL_PHY_EDPD_DFLT_TX_MSECS` value is exposed to cover a default
> TX interval; some PHYs could specify a certain value that makes sense
> * `ETHTOOL_PHY_EDPD_NO_TX` would disable TX when EDPD is enabled
> * `ETHTOOL_PHY_EDPD_DISABLE` will disable EDPD
>
> As noted by the `ETHTOOL_PHY_EDPD_DFLT_TX_MSECS` the interval unit is 1
> millisecond, which should cover a reasonable range of intervals:
> - from 1 millisecond, which does not sound like much of a power-saver
> - to ~65 seconds which is quite a lot to wait for a link to come up when
> plugging a cable
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ 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