* Re: [PATCH net 1/4] net:ethernet:aquantia: Setup max_mtu in ndev to enable jumbo frames
From: Igor Russkikh @ 2017-09-22 7:24 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S . Miller, netdev, David Arcari, Pavel Belous,
Nadezhda Krupnina, Simon Edelhaus
In-Reply-To: <20170921153619.GG27589@lunn.ch>
>> self->ndev->mtu = aq_nic_cfg->mtu - ETH_HLEN;
>> + self->ndev->min_mtu = ETH_MIN_MTU;
> This is not required. It will default to ETH_MIN_MTU.
Thanks Andrew, true.
>> + self->ndev->max_mtu = self->aq_hw_caps.mtu - ETH_FCS_LEN - ETH_HLEN;
>>
>> return 0;
>> }
>> @@ -695,7 +696,7 @@ int aq_nic_set_mtu(struct aq_nic_s *self, int new_mtu)
>> {
>> int err = 0;
>>
>> - if (new_mtu > self->aq_hw_caps.mtu) {
>> + if (new_mtu + ETH_FCS_LEN > self->aq_hw_caps.mtu) {
> If you have set max_mtu correctly, this cannot happen. But it seems
> odd you don't have ETH_HLEN here, where as when setting max_mtu you
> do.
I agree, thats an extra check. Will remove that.
Regarding ETH_HLEN - the baseline code invokes this function with (new_mtu + ETH_HLEN) - thats why this check does not add it. In general, that extra level of storage (aq_nic_cfg.mtu field) is almost useless, I'll cleanup this under a separate patchset.
--
BR,
Igor
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: dsa: lan9303: Add basic offloading of unicast traffic
From: Egil Hjelmeland @ 2017-09-22 7:23 UTC (permalink / raw)
To: Vivien Didelot, andrew, f.fainelli, netdev, linux-kernel
In-Reply-To: <87k20sqg9d.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>
Den 21. sep. 2017 16:26, skrev Vivien Didelot:
> Hi Egil,
>
> Egil Hjelmeland <privat@egil-hjelmeland.no> writes:
>
>> When both user ports are joined to the same bridge, the normal
>> HW MAC learning is enabled. This means that unicast traffic is forwarded
>> in HW.
>>
>> If one of the user ports leave the bridge,
>> the ports goes back to the initial separated operation.
>>
>> Port separation relies on disabled HW MAC learning. Hence the condition
>> that both ports must join same bridge.
>>
>> Add brigde methods port_bridge_join, port_bridge_leave and
>> port_stp_state_set.
>>
>> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
>
> Styling nitpicks below, other than that, the patch LGTM:
>
> Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>
>> #include <linux/mutex.h>
>> #include <linux/mii.h>
>> #include <linux/phy.h>
>> +#include <linux/if_bridge.h>
>
> It's nice to order header inclusions alphabetically.
>
>>
>> #include "lan9303.h"
>>
>> @@ -146,6 +147,7 @@
>> # define LAN9303_SWE_PORT_STATE_FORWARDING_PORT0 (0)
>> # define LAN9303_SWE_PORT_STATE_LEARNING_PORT0 BIT(1)
>> # define LAN9303_SWE_PORT_STATE_BLOCKING_PORT0 BIT(0)
>> +# define LAN9303_SWE_PORT_STATE_DISABLED_PORT0 (3)
>> #define LAN9303_SWE_PORT_MIRROR 0x1846
>> # define LAN9303_SWE_PORT_MIRROR_SNIFF_ALL BIT(8)
>> # define LAN9303_SWE_PORT_MIRROR_SNIFFER_PORT2 BIT(7)
>> @@ -431,6 +433,20 @@ static int lan9303_read_switch_reg(struct lan9303 *chip, u16 regnum, u32 *val)
>> return ret;
>> }
>>
>> +static int lan9303_write_switch_reg_mask(
>> + struct lan9303 *chip, u16 regnum, u32 val, u32 mask)
>
> That is unlikely to respect the Kernel Coding Style. Please fill the
> line as much as possible and align with the opening parenthesis:
>
> static int lan9303_write_switch_reg_mask(struct lan9303 *chip, u16 regnum,
> u32 val, u32 mask)
>
OK. Probably this function will go away in v2 due to Andrews comment.
>> +{
>> + int ret;
>> + u32 reg;
>> +
>> + ret = lan9303_read_switch_reg(chip, regnum, ®);
>> + if (ret)
>> + return ret;
>> + reg = (reg & ~mask) | val;
>> +
>> + return lan9303_write_switch_reg(chip, regnum, reg);
>> +}
>
> ...
>
>> +
>> + portmask = 0x3 << (port * 2);
>> + portstate <<= (port * 2);
>
> Unnecessary alignment, please remove the extra space characters.
>
I think the alignment makes the logic more clear.
>> + lan9303_write_switch_reg_mask(chip, LAN9303_SWE_PORT_STATE,
>> + portstate, portmask);
>> +}
>> +
>> static const struct dsa_switch_ops lan9303_switch_ops = {
>> .get_tag_protocol = lan9303_get_tag_protocol,
>> .setup = lan9303_setup,
>> @@ -855,6 +940,9 @@ static const struct dsa_switch_ops lan9303_switch_ops = {
>> .get_sset_count = lan9303_get_sset_count,
>> .port_enable = lan9303_port_enable,
>> .port_disable = lan9303_port_disable,
>> + .port_bridge_join = lan9303_port_bridge_join,
>> + .port_bridge_leave = lan9303_port_bridge_leave,
>> + .port_stp_state_set = lan9303_port_stp_state_set,
>
> Same here, alignment unnecessary, especially since the rest of the
> structure does not do that.
>
>> };
>>
>> static int lan9303_register_switch(struct lan9303 *chip)
>> diff --git a/drivers/net/dsa/lan9303.h b/drivers/net/dsa/lan9303.h
>> index 4d8be555ff4d..5be246f05965 100644
>> --- a/drivers/net/dsa/lan9303.h
>> +++ b/drivers/net/dsa/lan9303.h
>> @@ -21,6 +21,7 @@ struct lan9303 {
>> struct dsa_switch *ds;
>> struct mutex indirect_mutex; /* protect indexed register access */
>> const struct lan9303_phy_ops *ops;
>> + bool is_bridged; /* true if port 1 and 2 is bridged */
>> };
>>
>> extern const struct regmap_access_table lan9303_register_set;
>
> Please use the checkpatch.pl script to ensure your patch respects the
> kernel conventions before submitting, it can spot nice stuffs!
I have checked _every_ patch with checkpatch.pl and weeded all warnings
before I submitted them.
>
> I use a Git alias(*) to check a commit which does basically this:
>
> git format-patch --stdout -1 | ./scripts/checkpatch.pl -
>
>
> (*) in details, especially convenient during interactive rebases:
>
> $ git config alias.checkcommit '!f () { git format-patch --stdout -1 ${1:-HEAD} | ./scripts/checkpatch.pl - ; }; f'
> $ git checkcommit # i.e. current one
> $ git checkcommit HEAD^^
> $ git checkcommit d329ac88eb21
> ...
>
>
> Thanks,
>
> Vivien
>
^ permalink raw reply
* Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit
From: Julia Lawall @ 2017-09-22 7:23 UTC (permalink / raw)
To: Colin King
Cc: Stanislaw Gruszka, Kalle Valo, linux-wireless, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20170921225630.21916-1-colin.king@canonical.com>
On Thu, 21 Sep 2017, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Don't populate const array ac_to_fifo on the stack in an inlined
> function, instead make it static. Makes the object code smaller
> by over 800 bytes:
>
> text data bss dec hex filename
> 159029 33154 1216 193399 2f377 4965-mac.o
>
> text data bss dec hex filename
> 158122 33250 1216 192588 2f04c 4965-mac.o
>
> (gcc version 7.2.0 x86_64)
Gcc 7 must be much more aggressive about inlining than gcc 4. Here is the
change that I got on this file:
text data bss dec hex filename
- 76346 1494 152 77992 130a8 drivers/net/wireless/intel/iwlegacy/4965-mac.o
+ 76298 1494 152 77944 13078 drivers/net/wireless/intel/iwlegacy/4965-mac.o
decrease of 48
julia
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intel/iwlegacy/4965-mac.c b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> index de9b6522c43f..65eba2c24292 100644
> --- a/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> +++ b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> @@ -1480,7 +1480,7 @@ il4965_get_ac_from_tid(u16 tid)
> static inline int
> il4965_get_fifo_from_tid(u16 tid)
> {
> - const u8 ac_to_fifo[] = {
> + static const u8 ac_to_fifo[] = {
> IL_TX_FIFO_VO,
> IL_TX_FIFO_VI,
> IL_TX_FIFO_BE,
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: linux-next: build failure after merge of the net-next tree
From: Paolo Abeni @ 2017-09-22 7:10 UTC (permalink / raw)
To: David Miller, sfr; +Cc: netdev, linux-next, linux-kernel
In-Reply-To: <20170921.183759.289331373985144347.davem@davemloft.net>
On Thu, 2017-09-21 at 18:37 -0700, David Miller wrote:
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Fri, 22 Sep 2017 11:03:55 +1000
>
> > After merging the net-next tree, today's linux-next build (arm
> > multi_v7_defconfig) failed like this:
> >
> > net/ipv4/fib_frontend.c: In function 'fib_validate_source':
> > net/ipv4/fib_frontend.c:411:16: error: 'struct netns_ipv4' has no member named 'fib_has_custom_local_routes'
> > if (net->ipv4.fib_has_custom_local_routes)
> > ^
> > net/ipv4/fib_frontend.c: In function 'inet_rtm_newroute':
> > net/ipv4/fib_frontend.c:773:12: error: 'struct netns_ipv4' has no member named 'fib_has_custom_local_routes'
> > net->ipv4.fib_has_custom_local_routes = true;
> > ^
> >
> > Caused by commit
> >
> > 6e617de84e87 ("net: avoid a full fib lookup when rp_filter is disabled.")
>
> Paolo, it seems this struct member should be placed outside of
> IP_MULTIPLE_TABLES protection, since users can insert custom
> local routes even without that set.
>
> So I'm installing the following fix for this:
>
> ====================
> [PATCH] ipv4: Move fib_has_custom_local_routes outside of IP_MULTIPLE_TABLES.
>
> > net/ipv4/fib_frontend.c: In function 'fib_validate_source':
> > net/ipv4/fib_frontend.c:411:16: error: 'struct netns_ipv4' has no member named 'fib_has_custom_local_routes'
> > if (net->ipv4.fib_has_custom_local_routes)
> > ^
> > net/ipv4/fib_frontend.c: In function 'inet_rtm_newroute':
> > net/ipv4/fib_frontend.c:773:12: error: 'struct netns_ipv4' has no member named 'fib_has_custom_local_routes'
> > net->ipv4.fib_has_custom_local_routes = true;
> > ^
>
> Fixes: 6e617de84e87 ("net: avoid a full fib lookup when rp_filter is disabled.")
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> include/net/netns/ipv4.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
> index 20720721da4b..8387f099115e 100644
> --- a/include/net/netns/ipv4.h
> +++ b/include/net/netns/ipv4.h
> @@ -49,10 +49,10 @@ struct netns_ipv4 {
> #ifdef CONFIG_IP_MULTIPLE_TABLES
> struct fib_rules_ops *rules_ops;
> bool fib_has_custom_rules;
> - bool fib_has_custom_local_routes;
> struct fib_table __rcu *fib_main;
> struct fib_table __rcu *fib_default;
> #endif
> + bool fib_has_custom_local_routes;
> #ifdef CONFIG_IP_ROUTE_CLASSID
> int fib_num_tclassid_users;
> #endif
> --
> 2.13.5
My fault, I should have fuzzed the configuration before posting.
Fix looks good, thanks David!
Paolo
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: dsa: lan9303: Add basic offloading of unicast traffic
From: Egil Hjelmeland @ 2017-09-22 7:06 UTC (permalink / raw)
To: Andrew Lunn; +Cc: vivien.didelot, f.fainelli, netdev, linux-kernel
In-Reply-To: <20170921142127.GB27589@lunn.ch>
Den 21. sep. 2017 16:21, skrev Andrew Lunn:
> Hi Egil
>
>> +static void lan9303_bridge_ports(struct lan9303 *chip)
>> +{
>> + /* ports bridged: remove mirroring */
>> + lan9303_write_switch_reg(chip, LAN9303_SWE_PORT_MIRROR, 0);
>> +}
>
> Could you replace the 0 with something symbolic which makes this
> easier to understand.
>
> #define LAN9303_SWE_PORT_MIRROR_DISABLED 0
>
OK
>> +
>> static int lan9303_handle_reset(struct lan9303 *chip)
>> {
>> if (!chip->reset_gpio)
>> @@ -844,6 +866,69 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port,
>> }
>> }
>>
>> +static int lan9303_port_bridge_join(struct dsa_switch *ds, int port,
>> + struct net_device *br)
>> +{
>> + struct lan9303 *chip = ds->priv;
>> +
>> + dev_dbg(chip->dev, "%s(port %d)\n", __func__, port);
>> + if (ds->ports[1].bridge_dev == ds->ports[2].bridge_dev) {
>> + lan9303_bridge_ports(chip);
>> + chip->is_bridged = true; /* unleash stp_state_set() */
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void lan9303_port_bridge_leave(struct dsa_switch *ds, int port,
>> + struct net_device *br)
>> +{
>> + struct lan9303 *chip = ds->priv;
>> +
>> + dev_dbg(chip->dev, "%s(port %d)\n", __func__, port);
>> + if (chip->is_bridged) {
>> + lan9303_separate_ports(chip);
>> + chip->is_bridged = false;
>> + }
>> +}
>> +
>> +static void lan9303_port_stp_state_set(struct dsa_switch *ds, int port,
>> + u8 state)
>> +{
>> + int portmask, portstate;
>> + struct lan9303 *chip = ds->priv;
>> +
>> + dev_dbg(chip->dev, "%s(port %d, state %d)\n",
>> + __func__, port, state);
>> + if (!chip->is_bridged)
>> + return; /* touching SWE_PORT_STATE will break port separation */
>
> I'm wondering how this is supposed to work. Please add a good comment
> here, since the hardware is forcing you to do something odd.
>
> Maybe it would be a good idea to save the STP state in chip. And then
> when chip->is_bridged is set true, change the state in the hardware to
> the saved value?
>
> What happens when port 0 is added to the bridge, there is then a
> minute pause and then port 1 is added? I would expect that as soon as
> port 0 is added, the STP state machine for port 0 will start and move
> into listening and then forwarding. Due to hardware limitations it
> looks like you cannot do this. So what state is the hardware
> effectively in? Blocking? Forwarding?
>
> Then port 1 is added. You can then can respect the states. port 1 will
> do blocking->listening->forwarding, but what about port 0? The calls
> won't get repeated? How does it transition to forwarding?
>
> Andrew
>
I see your point with the "minute pause" argument. Although a bit
contrived use case, it is easy to fix by caching the STP state, as
you suggest. So I can do that.
The port separation method is from the original version of the driver,
not by me.
I have read through the datasheet to see if there are other ways
to make port separation, but I could not find anything.
If anybody care to read through the 300+ page lan9303.pdf and prove
me wrong, I am happy to do it differently.
How does other DSA HW chips handle port separation? Knowing that
could perhaps help me know what to look for.
Egil
'
>> +
>> + switch (state) {
>> + case BR_STATE_DISABLED:
>> + portstate = LAN9303_SWE_PORT_STATE_DISABLED_PORT0;
>> + break;
>> + case BR_STATE_BLOCKING:
>> + case BR_STATE_LISTENING:
>> + portstate = LAN9303_SWE_PORT_STATE_BLOCKING_PORT0;
>> + break;
>> + case BR_STATE_LEARNING:
>> + portstate = LAN9303_SWE_PORT_STATE_LEARNING_PORT0;
>> + break;
>> + case BR_STATE_FORWARDING:
>> + portstate = LAN9303_SWE_PORT_STATE_FORWARDING_PORT0;
>> + break;
>> + default:
>> + portstate = LAN9303_SWE_PORT_STATE_DISABLED_PORT0;
>> + dev_err(chip->dev, "unknown stp state: port %d, state %d\n",
>> + port, state);
>> + }
>> +
>> + portmask = 0x3 << (port * 2);
>> + portstate <<= (port * 2);
>> + lan9303_write_switch_reg_mask(chip, LAN9303_SWE_PORT_STATE,
>> + portstate, portmask);
>> +}
>
^ permalink raw reply
* [PATCH net-next] virtio-net: correctly set xdp_xmit for mergeable buffer
From: Jason Wang @ 2017-09-22 6:38 UTC (permalink / raw)
To: mst, jasowang, virtualization, netdev, linux-kernel; +Cc: John Fastabend
We should set xdp_xmit only when xdp_do_redirect() succeed.
Cc: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f6c1f13..dd14a45 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -721,7 +721,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
goto xdp_xmit;
case XDP_REDIRECT:
err = xdp_do_redirect(dev, &xdp, xdp_prog);
- if (err)
+ if (!err)
*xdp_xmit = true;
rcu_read_unlock();
goto xdp_xmit;
--
2.7.4
^ permalink raw reply related
* Re: [patch net-next 03/12] ipmr: Add FIB notification access functions
From: Yotam Gigi @ 2017-09-22 6:35 UTC (permalink / raw)
To: Nikolay Aleksandrov, Jiri Pirko, netdev; +Cc: davem, idosch, mlxsw
In-Reply-To: <24470459-704b-89a2-648a-bbd4d3a1aebb@cumulusnetworks.com>
On 09/21/2017 02:19 PM, Nikolay Aleksandrov wrote:
> On 21/09/17 09:43, Jiri Pirko wrote:
>> From: Yotam Gigi <yotamg@mellanox.com>
>>
>> Make the ipmr module register as a FIB notifier. To do that, implement both
>> the ipmr_seq_read and ipmr_dump ops.
>>
>> The ipmr_seq_read op returns a sequence counter that is incremented on
>> every notification related operation done by the ipmr. To implement that,
>> add a sequence counter in the netns_ipv4 struct and increment it whenever a
>> new MFC route or VIF are added or deleted. The sequence operations are
>> protected by the RTNL lock.
>>
>> The ipmr_dump iterates the list of MFC routes and the list of VIF entries
>> and sends notifications about them. The entries dump is done under RCU.
>>
>> Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
>> Reviewed-by: Ido Schimmel <idosch@mellanox.com>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> include/linux/mroute.h | 15 ++++++
>> include/net/netns/ipv4.h | 3 ++
>> net/ipv4/ipmr.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++-
>> 3 files changed, 151 insertions(+), 2 deletions(-)
>>
> [snip]
>> +
>> +static int ipmr_dump(struct net *net, struct notifier_block *nb)
>> +{
>> + struct mr_table *mrt;
>> + int err;
>> +
>> + err = ipmr_rules_dump(net, nb);
>> + if (err)
>> + return err;
>> +
>> + ipmr_for_each_table(mrt, net) {
>> + struct vif_device *v = &mrt->vif_table[0];
>> + struct mfc_cache *mfc;
>> + int vifi;
>> +
>> + /* Notifiy on table VIF entries */
>> + for (vifi = 0; vifi < mrt->maxvif; vifi++, v++) {
>> + if (!v->dev)
>> + continue;
>> +
>> + call_ipmr_vif_entry_notifier(nb, net, FIB_EVENT_VIF_ADD,
>> + v, vifi, mrt->id);
>> + }
> The VIF table is protected by mrt_lock (rwlock), here with RCU only
> you're not guaranteed to keep v->dev. It can become NULL after the check above.
> For details you can see vif_delete() in net/ipv4/ipmr.c. You need at least
> mrt_lock for reading.
Hmm, that's interesting. That situation would lead the dump to be inconsistent,
thus eventually discarded, right? So I can also just make sure that the driver
ignores dev=NULL notifications and that would be enough to solve it.
I have to think about it a bit more, but anyway, maybe taking the mrt_lock for
the VIF dump is the right solution here.
Anyway, thanks for the review!
>> +
>> + /* Notify on table MFC entries */
>> + list_for_each_entry_rcu(mfc, &mrt->mfc_cache_list, list)
>> + call_ipmr_mfc_entry_notifier(nb, net,
>> + FIB_EVENT_ENTRY_ADD, mfc,
>> + mrt->id);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct fib_notifier_ops ipmr_notifier_ops_template = {
>> + .family = RTNL_FAMILY_IPMR,
>> + .fib_seq_read = ipmr_seq_read,
>> + .fib_dump = ipmr_dump,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +int __net_init ipmr_notifier_init(struct net *net)
>> +{
>> + struct fib_notifier_ops *ops;
>> +
>> + net->ipv4.ipmr_seq = 0;
>> +
>> + ops = fib_notifier_ops_register(&ipmr_notifier_ops_template, net);
>> + if (IS_ERR(ops))
>> + return PTR_ERR(ops);
>> + net->ipv4.ipmr_notifier_ops = ops;
>> +
>> + return 0;
>> +}
>> +
>> +static void __net_exit ipmr_notifier_exit(struct net *net)
>> +{
>> + fib_notifier_ops_unregister(net->ipv4.ipmr_notifier_ops);
>> + net->ipv4.ipmr_notifier_ops = NULL;
>> +}
>> +
>> /* Setup for IP multicast routing */
>> static int __net_init ipmr_net_init(struct net *net)
>> {
>> int err;
>>
>> + err = ipmr_notifier_init(net);
>> + if (err)
>> + goto ipmr_notifier_fail;
>> +
>> err = ipmr_rules_init(net);
>> if (err < 0)
>> - goto fail;
>> + goto ipmr_rules_fail;
>>
>> #ifdef CONFIG_PROC_FS
>> err = -ENOMEM;
>> @@ -3074,7 +3202,9 @@ static int __net_init ipmr_net_init(struct net *net)
>> proc_vif_fail:
>> ipmr_rules_exit(net);
>> #endif
>> -fail:
>> +ipmr_rules_fail:
>> + ipmr_notifier_exit(net);
>> +ipmr_notifier_fail:
>> return err;
>> }
>>
>> @@ -3084,6 +3214,7 @@ static void __net_exit ipmr_net_exit(struct net *net)
>> remove_proc_entry("ip_mr_cache", net->proc_net);
>> remove_proc_entry("ip_mr_vif", net->proc_net);
>> #endif
>> + ipmr_notifier_exit(net);
>> ipmr_rules_exit(net);
>> }
>>
>>
^ permalink raw reply
* Re: [PATCH 00/64] use setup_timer() helper function.
From: Allen @ 2017-09-22 6:22 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-kernel, m.grzeschik, dmitry.tarnyagin, wg, mkl,
Mark Einon, linux, pcnet32, Florian Fainelli,
bcm-kernel-feedback-list, venza, ajk, jes, romieu, khc,
Simon Kelley, linux-can, adi-buildroot-devel
In-Reply-To: <20170921.113927.1684139706973891956.davem@davemloft.net>
>
>> This series uses setup_timer() helper function. The series
>> addresses the files under drivers/net/*.
>
> I've reviewed this series and will apply it to net-next.
>
> But please send out smaller chunks next time, maybe 10-15
> in a bunch? 64 patches at once makes it really hard for
> reviewiers.
My apologies. I'll make sure I break it into smaller chunks
in the future.
--
- Allen
^ permalink raw reply
* Re: tg3 pxe weirdness
From: Siva Reddy Kallam @ 2017-09-22 6:21 UTC (permalink / raw)
To: Berend De Schouwer; +Cc: Linux Netdev List
On Thu, Sep 21, 2017 at 7:53 PM, Berend De Schouwer
<berend.de.schouwer@gmail.com> wrote:
> Hi,
>
> I've got a machine with a Broadcom bcm5762c, using the tg3 driver, that
> fails to receive network packets under some very specific conditions.
>
> It works perfectly using a 1Gbps switch. If, however, it first uses
> PXE and then loads the Linux tg3 driver, and the switch is 100Mbps, it
> no longer receives packets larger than ICMP.
>
> It does do ARP and ping.
>
> If it boots using PXE on a 1Gbps switch, boots into Linux, and then
> it's plugged into 100 Mbps it also stops receiving packets.
>
> mii-diag and dmesg confirm auto-negotiated speed and flow control, and
> confirm temporary disconnect as the cables are moved.
>
> PXE boots using UNDI, which then transfers a kernel using TFTP, which
> transfers correctly. The kernel boots, loads the tg3 driver, connects
> the network. Up to this point everything works. Ping will work too.
> Any other network traffic fails.
>
> Booting from a harddrive works fine. I assume the UNDI driver
> somewhere breaks auto-negotiation. I've tried using mii-tool and
> ethtool, but I haven't managed to make it work yet.
>
> Is it possible to get negotiation working after PXE boot? Are there
> any tg3 driver flags that might make a difference?
>
>
> Berend
Can you please share below details?
1) Model and Manufacturer of the system
2) Linux distro/kernel used?
^ permalink raw reply
* [PATCH net-next v2 6/6] rtnetlink: rtnl_have_link_slave_info doesn't need rtnl
From: Florian Westphal @ 2017-09-22 6:10 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170922061008.14723-1-fw@strlen.de>
Switch it to rcu.
rtnl_link_slave_info_fill on to other hand does need rtnl mutex for now so
add an annotation to its caller as a reminder.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
Change since v1:
- don't add ASSERT_RTNL to rtnl_link_slave_info_fill and
rtnl_link_info_fill they are only called via rtnl_link_fill.
net/core/rtnetlink.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 590823f70cc3..e6f9e36d9d5a 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -522,11 +522,15 @@ static size_t rtnl_link_get_af_size(const struct net_device *dev,
static bool rtnl_have_link_slave_info(const struct net_device *dev)
{
struct net_device *master_dev;
+ bool ret = false;
- master_dev = netdev_master_upper_dev_get((struct net_device *) dev);
+ rcu_read_lock();
+
+ master_dev = netdev_master_upper_dev_get_rcu((struct net_device *)dev);
if (master_dev && master_dev->rtnl_link_ops)
- return true;
- return false;
+ ret = true;
+ rcu_read_unlock();
+ return ret;
}
static int rtnl_link_slave_info_fill(struct sk_buff *skb,
@@ -598,6 +602,8 @@ static int rtnl_link_fill(struct sk_buff *skb, const struct net_device *dev)
struct nlattr *linkinfo;
int err = -EMSGSIZE;
+ ASSERT_RTNL();
+
linkinfo = nla_nest_start(skb, IFLA_LINKINFO);
if (linkinfo == NULL)
goto out;
--
2.13.5
^ permalink raw reply related
* [PATCH net-next v2 5/6] rtnetlink: add helpers to dump vf and netnsid information
From: Florian Westphal @ 2017-09-22 6:10 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170922061008.14723-1-fw@strlen.de>
similar to earlier patches, split out more parts of this function to
better see what is happening and where we assume rtnl is locked.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/core/rtnetlink.c | 80 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 50 insertions(+), 30 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 42ff582a010e..590823f70cc3 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1211,6 +1211,36 @@ static noinline_for_stack int rtnl_fill_vfinfo(struct sk_buff *skb,
return -EMSGSIZE;
}
+static noinline_for_stack int rtnl_fill_vf(struct sk_buff *skb,
+ struct net_device *dev,
+ u32 ext_filter_mask)
+{
+ struct nlattr *vfinfo;
+ int i, num_vfs;
+
+ if (!dev->dev.parent || ((ext_filter_mask & RTEXT_FILTER_VF) == 0))
+ return 0;
+
+ num_vfs = dev_num_vf(dev->dev.parent);
+ if (nla_put_u32(skb, IFLA_NUM_VF, num_vfs))
+ return -EMSGSIZE;
+
+ if (!dev->netdev_ops->ndo_get_vf_config)
+ return 0;
+
+ vfinfo = nla_nest_start(skb, IFLA_VFINFO_LIST);
+ if (!vfinfo)
+ return -EMSGSIZE;
+
+ for (i = 0; i < num_vfs; i++) {
+ if (rtnl_fill_vfinfo(skb, dev, i, vfinfo))
+ return -EMSGSIZE;
+ }
+
+ nla_nest_end(skb, vfinfo);
+ return 0;
+}
+
static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
{
struct rtnl_link_ifmap map;
@@ -1355,6 +1385,23 @@ static noinline int nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
return 0;
}
+static noinline int rtnl_fill_link_netnsid(struct sk_buff *skb,
+ const struct net_device *dev)
+{
+ if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net) {
+ struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
+
+ if (!net_eq(dev_net(dev), link_net)) {
+ int id = peernet2id_alloc(dev_net(dev), link_net);
+
+ if (nla_put_s32(skb, IFLA_LINK_NETNSID, id))
+ return -EMSGSIZE;
+ }
+ }
+
+ return 0;
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask,
@@ -1428,27 +1475,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
if (rtnl_fill_stats(skb, dev))
goto nla_put_failure;
- if (dev->dev.parent && (ext_filter_mask & RTEXT_FILTER_VF) &&
- nla_put_u32(skb, IFLA_NUM_VF, dev_num_vf(dev->dev.parent)))
+ if (rtnl_fill_vf(skb, dev, ext_filter_mask))
goto nla_put_failure;
- if (dev->netdev_ops->ndo_get_vf_config && dev->dev.parent &&
- ext_filter_mask & RTEXT_FILTER_VF) {
- int i;
- struct nlattr *vfinfo;
- int num_vfs = dev_num_vf(dev->dev.parent);
-
- vfinfo = nla_nest_start(skb, IFLA_VFINFO_LIST);
- if (!vfinfo)
- goto nla_put_failure;
- for (i = 0; i < num_vfs; i++) {
- if (rtnl_fill_vfinfo(skb, dev, i, vfinfo))
- goto nla_put_failure;
- }
-
- nla_nest_end(skb, vfinfo);
- }
-
if (rtnl_port_fill(skb, dev, ext_filter_mask))
goto nla_put_failure;
@@ -1460,17 +1489,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
goto nla_put_failure;
}
- if (dev->rtnl_link_ops &&
- dev->rtnl_link_ops->get_link_net) {
- struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
-
- if (!net_eq(dev_net(dev), link_net)) {
- int id = peernet2id_alloc(dev_net(dev), link_net);
-
- if (nla_put_s32(skb, IFLA_LINK_NETNSID, id))
- goto nla_put_failure;
- }
- }
+ if (rtnl_fill_link_netnsid(skb, dev))
+ goto nla_put_failure;
if (!(af_spec = nla_nest_start(skb, IFLA_AF_SPEC)))
goto nla_put_failure;
--
2.13.5
^ permalink raw reply related
* [PATCH net-next v2 4/6] rtnetlink: add helper to dump ifalias
From: Florian Westphal @ 2017-09-22 6:10 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170922061008.14723-1-fw@strlen.de>
ifalias is currently protected by rtnl mutex, add assertion
as a reminder.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/core/rtnetlink.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index ad3f27da37a8..42ff582a010e 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1345,6 +1345,16 @@ static int nla_put_qdisc(struct sk_buff *skb, struct net_device *dev)
return ret;
}
+static noinline int nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
+{
+ ASSERT_RTNL();
+
+ if (dev->ifalias)
+ return nla_put_string(skb, IFLA_IFALIAS, dev->ifalias);
+
+ return 0;
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask,
@@ -1386,8 +1396,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
put_master_ifindex(skb, dev) ||
nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
nla_put_qdisc(skb, dev) ||
- (dev->ifalias &&
- nla_put_string(skb, IFLA_IFALIAS, dev->ifalias)) ||
+ nla_put_ifalias(skb, dev) ||
nla_put_u32(skb, IFLA_CARRIER_CHANGES,
atomic_read(&dev->carrier_changes)) ||
nla_put_u8(skb, IFLA_PROTO_DOWN, dev->proto_down))
--
2.13.5
^ permalink raw reply related
* [PATCH net-next v2 3/6] rtnetlink: add helper to dump qdisc name
From: Florian Westphal @ 2017-09-22 6:10 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170922061008.14723-1-fw@strlen.de>
We can use rcu here to make this safe even if we would not hold rtnl:
qdisc_destroy uses call_rcu to free the Qdisc struct.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/core/rtnetlink.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index c801212ee40e..ad3f27da37a8 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1332,6 +1332,19 @@ static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev)
return nla_put_u32(skb, IFLA_LINK, ifindex);
}
+static int nla_put_qdisc(struct sk_buff *skb, struct net_device *dev)
+{
+ struct Qdisc *q;
+ int ret = 0;
+
+ rcu_read_lock();
+ q = READ_ONCE(dev->qdisc);
+ if (q)
+ ret = nla_put_string(skb, IFLA_QDISC, q->ops->id);
+ rcu_read_unlock();
+ return ret;
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask,
@@ -1372,8 +1385,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
nla_put_iflink(skb, dev) ||
put_master_ifindex(skb, dev) ||
nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
- (dev->qdisc &&
- nla_put_string(skb, IFLA_QDISC, dev->qdisc->ops->id)) ||
+ nla_put_qdisc(skb, dev) ||
(dev->ifalias &&
nla_put_string(skb, IFLA_IFALIAS, dev->ifalias)) ||
nla_put_u32(skb, IFLA_CARRIER_CHANGES,
--
2.13.5
^ permalink raw reply related
* [PATCH net-next v2 2/6] rtnetlink: add helper to put master ifindex
From: Florian Westphal @ 2017-09-22 6:10 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170922061008.14723-1-fw@strlen.de>
rtnl_fill_ifinfo currently requires caller to hold the rtnl mutex.
Unfortunately the function is quite large which makes it harder to see
which spots need the lock, which spots assume it and which ones could do
without.
Add helpers to factor out the ifindex dumping.
One helper can use rcu to remove rtnl dependency.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/core/rtnetlink.c | 32 +++++++++++++++++++++++++++-----
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a78fd61da0ec..c801212ee40e 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1307,6 +1307,31 @@ static u32 rtnl_get_event(unsigned long event)
return rtnl_event_type;
}
+static int put_master_ifindex(struct sk_buff *skb, struct net_device *dev)
+{
+ const struct net_device *upper_dev;
+ int ret = 0;
+
+ rcu_read_lock();
+
+ upper_dev = netdev_master_upper_dev_get_rcu(dev);
+ if (upper_dev)
+ ret = nla_put_u32(skb, IFLA_MASTER, upper_dev->ifindex);
+
+ rcu_read_unlock();
+ return ret;
+}
+
+static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev)
+{
+ int ifindex = dev_get_iflink(dev);
+
+ if (dev->ifindex == ifindex)
+ return 0;
+
+ return nla_put_u32(skb, IFLA_LINK, ifindex);
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask,
@@ -1316,7 +1341,6 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
struct nlmsghdr *nlh;
struct nlattr *af_spec;
struct rtnl_af_ops *af_ops;
- struct net_device *upper_dev = netdev_master_upper_dev_get(dev);
ASSERT_RTNL();
nlh = nlmsg_put(skb, pid, seq, type, sizeof(*ifm), flags);
@@ -1345,10 +1369,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
#ifdef CONFIG_RPS
nla_put_u32(skb, IFLA_NUM_RX_QUEUES, dev->num_rx_queues) ||
#endif
- (dev->ifindex != dev_get_iflink(dev) &&
- nla_put_u32(skb, IFLA_LINK, dev_get_iflink(dev))) ||
- (upper_dev &&
- nla_put_u32(skb, IFLA_MASTER, upper_dev->ifindex)) ||
+ nla_put_iflink(skb, dev) ||
+ put_master_ifindex(skb, dev) ||
nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
(dev->qdisc &&
nla_put_string(skb, IFLA_QDISC, dev->qdisc->ops->id)) ||
--
2.13.5
^ permalink raw reply related
* [PATCH net-next v2 1/6] selftests: rtnetlink.sh: add rudimentary vrf test
From: Florian Westphal @ 2017-09-22 6:10 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal, David Ahern
In-Reply-To: <20170922061008.14723-1-fw@strlen.de>
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
tools/testing/selftests/net/rtnetlink.sh | 42 ++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index 4b48de565cae..6ee2bbaebcd4 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -291,6 +291,47 @@ kci_test_ifalias()
echo "PASS: set ifalias $namewant for $devdummy"
}
+kci_test_vrf()
+{
+ vrfname="test-vrf"
+ ret=0
+
+ ip link show type vrf 2>/dev/null
+ if [ $? -ne 0 ]; then
+ echo "SKIP: vrf: iproute2 too old"
+ return 0
+ fi
+
+ ip link add "$vrfname" type vrf table 10
+ check_err $?
+ if [ $ret -ne 0 ];then
+ echo "FAIL: can't add vrf interface, skipping test"
+ return 0
+ fi
+
+ ip -br link show type vrf | grep -q "$vrfname"
+ check_err $?
+ if [ $ret -ne 0 ];then
+ echo "FAIL: created vrf device not found"
+ return 1
+ fi
+
+ ip link set dev "$vrfname" up
+ check_err $?
+
+ ip link set dev "$devdummy" master "$vrfname"
+ check_err $?
+ ip link del dev "$vrfname"
+ check_err $?
+
+ if [ $ret -ne 0 ];then
+ echo "FAIL: vrf"
+ return 1
+ fi
+
+ echo "PASS: vrf"
+}
+
kci_test_rtnl()
{
kci_add_dummy
@@ -306,6 +347,7 @@ kci_test_rtnl()
kci_test_bridge
kci_test_addrlabel
kci_test_ifalias
+ kci_test_vrf
kci_del_dummy
}
--
2.13.5
^ permalink raw reply related
* [PATCH net-next v2 0/6] rtnetlink: preparation patches for further rtnl lock pushdown/removal
From: Florian Westphal @ 2017-09-22 6:10 UTC (permalink / raw)
To: netdev
First patch adds a rudimentary vrf test case.
Remaining patches split large rtnl_fill_ifinfo into smaller chunks
to better see which parts
1. require rtnl
2. do not require it at all
3. rely on rtnl locking now but could be converted
changes since v2:
- fix subject lines of patches 2 and 5
- drop pure ASSERT_RTNL annotation patch, its not all that useful
- remove two extra ASSERT_RTNL in last patch
^ permalink raw reply
* Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit
From: Johannes Berg @ 2017-09-22 6:00 UTC (permalink / raw)
To: Colin King, Stanislaw Gruszka, Kalle Valo, linux-wireless, netdev
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20170921225630.21916-1-colin.king@canonical.com>
On Thu, 2017-09-21 at 23:56 +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Don't populate const array ac_to_fifo on the stack in an inlined
> function, instead make it static. Makes the object code smaller
> by over 800 bytes:
>
> text data bss dec hex
> filename
> 159029 33154 1216 193399 2f377
> 4965-mac.o
>
> text data bss dec hex
> filename
> 158122 33250 1216 192588 2f04c
> 4965-mac.o
>
> (gcc version 7.2.0 x86_64)
I'm curious - how did you find this?
johannes
^ permalink raw reply
* Re: [PATCH net-next 3/3] virtio-net: support XDP_REDIRECT
From: Jason Wang @ 2017-09-22 4:06 UTC (permalink / raw)
To: John Fastabend, mst, virtualization, netdev, linux-kernel
In-Reply-To: <59C2E588.6060203@gmail.com>
On 2017年09月21日 06:02, John Fastabend wrote:
> On 09/19/2017 02:42 AM, Jason Wang wrote:
>> This patch tries to add XDP_REDIRECT for virtio-net. The changes are
>> not complex as we could use exist XDP_TX helpers for most of the
>> work. The rest is passing the XDP_TX to NAPI handler for implementing
>> batching.
>>
>> Cc: John Fastabend <john.fastabend@gmail.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
> [...]
>
>> @@ -678,12 +711,20 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>> }
>> break;
>> case XDP_TX:
>> - if (unlikely(!virtnet_xdp_xmit(vi, &xdp)))
>> + if (unlikely(!__virtnet_xdp_xmit(vi, &xdp)))
>> trace_xdp_exception(vi->dev, xdp_prog, act);
>> + else
>> + *xdp_xmit = true;
>> if (unlikely(xdp_page != page))
>> goto err_xdp;
>> rcu_read_unlock();
>> goto xdp_xmit;
>> + case XDP_REDIRECT:
>> + err = xdp_do_redirect(dev, &xdp, xdp_prog);
>> + if (err)
>> + *xdp_xmit = true;
> Is this a typo? Should above be
>
> if (!err)
> *xdp_xmit = true;
>
> this would match the pattern in receive_small and also make more sense.
Right, will post a fix.
>
>> + rcu_read_unlock();
>> + goto xdp_xmit;
>> default:
>> bpf_warn_invalid_xdp_action(act);
>> case XDP_ABORTED:
>> @@ -788,7 +829,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>> }
>>
> [...]
>
> Otherwise looks good to me thanks for doing this.
>
> .John
>
Thanks for the reviewing.
^ permalink raw reply
* Re: Regression in throughput between kvm guests over virtual bridge
From: Jason Wang @ 2017-09-22 4:03 UTC (permalink / raw)
To: Matthew Rosato, netdev; +Cc: davem, mst
In-Reply-To: <129a01d9-de9b-f3f1-935c-128e73153df6@linux.vnet.ibm.com>
On 2017年09月21日 03:38, Matthew Rosato wrote:
>> Seems to make some progress on wakeup mitigation. Previous patch tries
>> to reduce the unnecessary traversal of waitqueue during rx. Attached
>> patch goes even further which disables rx polling during processing tx.
>> Please try it to see if it has any difference.
> Unfortunately, this patch doesn't seem to have made a difference. I
> tried runs with both this patch and the previous patch applied, as well
> as only this patch applied for comparison (numbers from vhost thread of
> sending VM):
>
> 4.12 4.13 patch1 patch2 patch1+2
> 2.00% +3.69% +2.55% +2.81% +2.69% [...] __wake_up_sync_key
>
> In each case, the regression in throughput was still present.
This probably means some other cases of the wakeups were missed. Could
you please record the callers of __wake_up_sync_key()?
>
>> And two questions:
>> - Is the issue existed if you do uperf between 2VMs (instead of 4VMs)
> Verified that the second set of guests are not actually required, I can
> see the regression with only 2 VMs.
>
>> - Can enable batching in the tap of sending VM improve the performance
>> (ethtool -C $tap rx-frames 64)
> I tried this, but it did not help (actually seemed to make things a
> little worse)
>
I still can't see a reason that can lead more wakeups, will take more
time to look at this issue and keep you posted.
Thanks
^ permalink raw reply
* Re: [PATCH net-next 01/10] net: hns3: Support for dynamically assigning tx buffer to TC
From: David Miller @ 2017-09-22 3:43 UTC (permalink / raw)
To: linyunsheng
Cc: huangdaode, xuwei5, liguozhu, Yisen.Zhuang, gabriele.paoloni,
john.garry, linuxarm, salil.mehta, lipeng321, netdev,
linux-kernel
In-Reply-To: <8babe541-9409-7f66-e52b-922144c46fb0@huawei.com>
From: Yunsheng Lin <linyunsheng@huawei.com>
Date: Fri, 22 Sep 2017 09:57:31 +0800
> Hi, David
>
> On 2017/9/22 9:41, David Miller wrote:
>> From: Yunsheng Lin <linyunsheng@huawei.com>
>> Date: Thu, 21 Sep 2017 19:21:44 +0800
>>
>>> @@ -1324,23 +1324,28 @@ static int hclge_alloc_vport(struct hclge_dev *hdev)
>>> return 0;
>>> }
>>>
>>> -static int hclge_cmd_alloc_tx_buff(struct hclge_dev *hdev, u16 buf_size)
>>> +static int hclge_cmd_alloc_tx_buff(struct hclge_dev *hdev)
>>> {
>>> /* TX buffer size is unit by 128 byte */
>>> #define HCLGE_BUF_SIZE_UNIT_SHIFT 7
>>> #define HCLGE_BUF_SIZE_UPDATE_EN_MSK BIT(15)
>>> struct hclge_tx_buff_alloc *req;
>>> + struct hclge_priv_buf *priv;
>>> struct hclge_desc desc;
>>> + u32 buf_size;
>>> int ret;
>>> u8 i;
>>>
>>> req = (struct hclge_tx_buff_alloc *)desc.data;
>>>
>>> hclge_cmd_setup_basic_desc(&desc, HCLGE_OPC_TX_BUFF_ALLOC, 0);
>>> - for (i = 0; i < HCLGE_TC_NUM; i++)
>>> + for (i = 0; i < HCLGE_TC_NUM; i++) {
>>> + priv = &hdev->priv_buf[i];
>>> + buf_size = priv->tx_buf_size;
>>> req->tx_pkt_buff[i] =
>>> cpu_to_le16((buf_size >> HCLGE_BUF_SIZE_UNIT_SHIFT) |
>
> buf_size is used here
My bad, I misread the code.
Thanks.
^ permalink raw reply
* Re: [PATCH net] net: prevent dst uses after free
From: David Miller @ 2017-09-22 3:42 UTC (permalink / raw)
To: eric.dumazet; +Cc: pstaszewski, weiwan, xiyou.wangcong, netdev, edumazet
In-Reply-To: <1506010546.29839.148.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 21 Sep 2017 09:15:46 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> In linux-4.13, Wei worked hard to convert dst to a traditional
> refcounted model, removing GC.
>
> We now want to make sure a dst refcount can not transition from 0 back
> to 1.
>
> The problem here is that input path attached a not refcounted dst to an
> skb. Then later, because packet is forwarded and hits skb_dst_force()
> before exiting RCU section, we might try to take a refcount on one dst
> that is about to be freed, if another cpu saw 1 -> 0 transition in
> dst_release() and queued the dst for freeing after one RCU grace period.
>
> Lets unify skb_dst_force() and skb_dst_force_safe(), since we should
> always perform the complete check against dst refcount, and not assume
> it is not zero.
>
> Bugzilla : https://bugzilla.kernel.org/show_bug.cgi?id=197005
...
> Similarly dst_clone() can use dst_hold() helper to have additional
> debugging, as a follow up to commit 44ebe79149ff ("net: add debug
> atomic_inc_not_zero() in dst_hold()")
>
> In net-next we will convert dst atomic_t to refcount_t for peace of
> mind.
>
> Fixes: a4c2fd7f7891 ("net: remove DST_NOCACHE flag")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Wei Wang <weiwan@google.com>
> Reported-by: Paweł Staszewski <pstaszewski@itcare.pl>
> Bisected-by: Paweł Staszewski <pstaszewski@itcare.pl>
Applied and queued up for -stable, thanks Eric.
^ permalink raw reply
* Re: [PATCH net-next] test_rhashtable: remove initdata annotation
From: David Miller @ 2017-09-22 3:42 UTC (permalink / raw)
To: fw; +Cc: netdev
In-Reply-To: <20170921153608.8789-1-fw@strlen.de>
From: Florian Westphal <fw@strlen.de>
Date: Thu, 21 Sep 2017 17:36:08 +0200
> kbuild test robot reported a section mismatch warning w. gcc 4.x:
> WARNING: lib/test_rhashtable.o(.text+0x139e):
> Section mismatch in reference from the function rhltable_insert.clone.3() to the variable .init.data:rhlt
>
> so remove this annotation.
>
> Fixes: cdd4de372ea06 ("test_rhashtable: add test case for rhl_table interface")
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: vrf: remove skb_dst_force() after skb_dst_set()
From: David Miller @ 2017-09-22 3:36 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, dsa, shm
In-Reply-To: <1506005428.29839.129.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 21 Sep 2017 07:50:28 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> skb_dst_set(skb, dst) installs a normal (refcounted) dst, there is no
> point using skb_dst_force(skb)
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH] net: phy: Fix truncation of large IRQ numbers in phy_attached_print()
From: David Miller @ 2017-09-22 3:36 UTC (permalink / raw)
To: geert+renesas; +Cc: andrew, f.fainelli, romain.perier, netdev, linux-kernel
In-Reply-To: <1505993222-16721-1-git-send-email-geert+renesas@glider.be>
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Thu, 21 Sep 2017 13:27:02 +0200
> Given NR_IRQS is 2048 on sparc64, and even 32784 on alpha, 3 digits is
> not enough to represent interrupt numbers on all architectures. Hence
> PHY interrupt numbers may be truncated during printing.
>
> Increase the buffer size from 4 to 8 bytes to fix this.
>
> Fixes: 5e369aefdce4818c ("net: stmmac: Delete dead code for MDIO registration")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Applied.
^ permalink raw reply
* RE: [net-next 2/2] ifb: add device MTU validation check
From: 张胜举 @ 2017-09-22 3:35 UTC (permalink / raw)
To: 'Stephen Hemminger'; +Cc: davem, willemb, netdev
In-Reply-To: <20170921081010.4d6f5731@xeon-e3>
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: 2017年9月21日 23:10
> To: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
> Cc: davem@davemloft.net; willemb@google.com; netdev@vger.kernel.org
> Subject: Re: [net-next 2/2] ifb: add device MTU validation check
>
> On Thu, 21 Sep 2017 21:32:02 +0800
> Zhang Shengju <zhangshengju@cmss.chinamobile.com> wrote:
>
> > Currently, any mtu value can be assigned when adding a new ifb device:
> > [~]# ip link add name ifb2 mtu 100000 type ifb [~]# ip link show ifb2
> > 18: ifb2: <BROADCAST,NOARP> mtu 100000 qdisc noop state DOWN mode
> DEFAULT group default qlen 32
> > link/ether 7a:bf:f4:63:da:d1 brd ff:ff:ff:ff:ff:ff
> >
> > This patch adds device MTU validation check.
> >
> > Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
> > ---
> > drivers/net/ifb.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c index
> > 8870bd2..ce84ad2 100644
> > --- a/drivers/net/ifb.c
> > +++ b/drivers/net/ifb.c
> > @@ -282,6 +282,14 @@ static int ifb_validate(struct nlattr *tb[], struct
> nlattr *data[],
> > if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
> > return -EADDRNOTAVAIL;
> > }
> > +
> > + if (tb[IFLA_MTU]) {
> > + u32 mtu = nla_get_u32(tb[IFLA_MTU]);
> > +
> > + if (mtu < ETH_MIN_MTU || mtu > ETH_DATA_LEN)
> > + return -EINVAL;
> > + }
> > +
> > return 0;
> > }
> >
>
> What about running ifb with packets coming from devices with jumbo frames?
> Also since ifb is an input only device, MTU doesn't matter.
Actually ifb_setup() function setup ifb valid MTU range: [68, 1500], and
this will be check at dev_set_mtu() function.
You can't setup mtu out of this range after creation, but you can set any
value when adding new ifb device.
This is inconsistent, and I think we should add this check at creation time
BRs,
ZSJ
^ 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