* Re: [PATCH net-next v2 04/10] net: phy: add Broadcom BCM7xxx internal PHY driver
From: Florian Fainelli @ 2014-02-13 18:41 UTC (permalink / raw)
To: Francois Romieu
Cc: netdev, David Miller, Kevin Cernekee, devicetree@vger.kernel.org
In-Reply-To: <20140213103454.GA14941@electric-eye.fr.zoreil.com>
Hi Francois,
2014-02-13 2:34 GMT-08:00 Francois Romieu <romieu@fr.zoreil.com>:
> Florian Fainelli <f.fainelli@gmail.com> :
> [...]
>> diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
>> new file mode 100644
>> index 0000000..6aea6e2
>> --- /dev/null
>> +++ b/drivers/net/phy/bcm7xxx.c
> [...]
>> +static int bcm7445_config_init(struct phy_device *phydev)
>> +{
>> + int ret;
>
> It could be declared after 'i' below.
>
>> + const struct bcm7445_regs {
>
> static const
>
>> + int reg;
>> + u16 value;
>> + } bcm7445_regs_cfg[] = {
>> + /* increases ADC latency by 24ns */
>> + { 0x17, 0x0038 },
>> + { 0x15, 0xAB95 },
>> + /* increases internal 1V LDO voltage by 5% */
>> + { 0x17, 0x2038 },
>> + { 0x15, 0xBB22 },
>> + /* reduce RX low pass filter corner frequency */
>> + { 0x17, 0x6038 },
>> + { 0x15, 0xFFC5 },
>> + /* reduce RX high pass filter corner frequency */
>> + { 0x17, 0x003a },
>> + { 0x15, 0x2002 },
>> + };
>> + unsigned int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(bcm7445_regs_cfg); i++) {
>> + ret = phy_write(phydev,
>> + bcm7445_regs_cfg[i].reg,
>> + bcm7445_regs_cfg[i].value);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void phy_write_exp(struct phy_device *phydev,
>> + u16 reg, u16 value)
>
> static void phy_write_exp(struct phy_device *phydev, u16 reg, u16 value)
>
>> +{
>> + phy_write(phydev, 0x17, 0xf00 | reg);
>> + phy_write(phydev, 0x15, value);
>> +}
>> +
>> +static void phy_write_misc(struct phy_device *phydev,
>> + u16 reg, u16 chl, u16 value)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ all tabs that don't line up
>
> static void phy_write_misc(struct phy_device *phydev,
> u16 reg, u16 chl, u16 value)
>
> static void phy_write_misc(struct phy_device *phydev, u16 reg, u16 chl,
> u16 value)
>
> static void phy_write_misc(struct phy_device *phydev, u16 reg, u16 chl, u16 val)
>
>
>> +{
>> + int tmp;
>> +
>> + phy_write(phydev, 0x18, 0x7);
>> +
>> + tmp = phy_read(phydev, 0x18);
>> + tmp |= 0x800;
>> + phy_write(phydev, 0x18, tmp);
>> +
>> + tmp = (chl * 0x2000) | reg;
>> + phy_write(phydev, 0x17, tmp);
>> +
>> + phy_write(phydev, 0x15, value);
>
> You may use some #define for the 0x15, 0x17 and 0x18 values.
Right, those are actually inherited from the BCM54xx PHY driver, I
will move those register to a common location e.g: brcmphy.h
>
>> +}
>> +
>> +static int bcm7xxx_28nm_afe_config_init(struct phy_device *phydev)
>> +{
>> + /* write AFE_RXCONFIG_0 */
>> + phy_write_misc(phydev, 0x38, 0x0000, 0xeb19);
>> +
>> + /* write AFE_RXCONFIG_1 */
>> + phy_write_misc(phydev, 0x38, 0x0001, 0x9a3f);
>> +
>> + /* write AFE_RX_LP_COUNTER */
>> + phy_write_misc(phydev, 0x38, 0x0003, 0x7fc7);
>> +
>> + /* write AFE_HPF_TRIM_OTHERS */
>> + phy_write_misc(phydev, 0x3A, 0x0000, 0x000b);
>> +
>> + /* write AFTE_TX_CONFIG */
>> + phy_write_misc(phydev, 0x39, 0x0000, 0x0800);
>
> Some #define may be welcome to replace the comments.
I would rather keep those as comments as they might change over time
if I get to incorporate a new workaround sequence.
>
> [...]
>> +static int bcm7xxx_28nm_config_init(struct phy_device *phydev)
>> +{
>> + int ret;
>> +
>> + ret = bcm7445_config_init(phydev);
>> + if (ret)
>> + return ret;
>> +
>> + return bcm7xxx_28nm_afe_config_init(phydev);
>> +}
>> +
>> +static int phy_set_clr_bits(struct phy_device *dev, int location,
>> + int set_mask, int clr_mask)
>> +{
>> + int v, ret;
>> +
>> + v = phy_read(dev, location);
>> + if (v < 0)
>> + return v;
>> +
>> + v &= ~clr_mask;
>> + v |= set_mask;
>> +
>> + ret = phy_write(dev, location, v);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return v;
>> +}
>> +
>> +static int bcm7xxx_config_init(struct phy_device *phydev)
>> +{
>> + /* Enable 64 clock MDIO */
>> + phy_write(phydev, 0x1d, 0x1000);
>> + phy_read(phydev, 0x1d);
>> +
>> + /* Workaround only required for 100Mbits/sec */
>> + if (!(phydev->dev_flags & PHY_BRCM_100MBPS_WAR))
>> + return 0;
>> +
>> + /* set shadow mode 2 */
>> + phy_set_clr_bits(phydev, 0x1f, 0x0004, 0x0004);
>
> phy_set_clr_bits returned status code is not checked.
>
>> +
>> + /* set iddq_clkbias */
>> + phy_write(phydev, 0x14, 0x0F00);
>> + udelay(10);
>> +
>> + /* reset iddq_clkbias */
>> + phy_write(phydev, 0x14, 0x0C00);
>> +
>> + phy_write(phydev, 0x13, 0x7555);
>> +
>> + /* reset shadow mode 2 */
>> + phy_set_clr_bits(phydev, 0x1f, 0x0004, 0);
>
> phy_set_clr_bits returned status code is not checked.
>
>> +
>> + return 0;
>> +}
>> +
>> +/* Workaround for putting the PHY in IDDQ mode, required
>> + * for all BCM7XXX PHYs
>> + */
>> +static int bcm7xxx_suspend(struct phy_device *phydev)
>
> Factor out with bcm7445_config_init and some helper ?
I would rather keep this function simple like it is today since this
really is only required for entering suspend mode properly while
bcm7445_config_init() as the name suggests is specific to 7445 only.
Thanks for the review!
>
>> +{
>> + int ret;
>> + const struct bcm7xxx_regs {
>> + int reg;
>> + u16 value;
>> + } bcm7xxx_suspend_cfg[] = {
>> + { 0x1f, 0x008b },
>> + { 0x10, 0x01c0 },
>> + { 0x14, 0x7000 },
>> + { 0x1f, 0x000f },
>> + { 0x10, 0x20d0 },
>> + { 0x1f, 0x000b },
>> + };
>> + unsigned int i;
>
> --
> Ueimor
--
Florian
^ permalink raw reply
* Re: large degradation in ip netns add/exec performance in 3.13?
From: Eric Dumazet @ 2014-02-13 18:38 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev
In-Reply-To: <52FAC6D1.50702@hp.com>
On Tue, 2014-02-11 at 16:56 -0800, Rick Jones wrote:
> On 02/11/2014 02:26 PM, Rick Jones wrote:
> > I did have the system lockup once at 16 concurrent streams on the 3.13.0
> > kernel. Didn't happen the next two times I tried.
>
> I've had it happen again with a slightly tweaked ("heavier") fake router
> creation. I'm guessing the attached soft lockup messages are related
> and may help point somewhere.
>
> happy benchmarking,
>
> rick jones
>
It doesnt seem to be network related but vfs, you might copy lkml to
your report ...
[ 200.884083] BUG: soft lockup - CPU#12 stuck for 23s! [ip:34798]
[ 200.911571] Modules linked in: veth ipmi_devintf 8021q garp xt_CHECKSUM iptable_mangle bridge stp llc kvm_intel kvm bonding ipt_REJECT xt_LOG xt_limit xt_tcpudp xt_addrtype nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack ip6table_filter psmouse ip6_tables iptable_filter ip_tables x_tables gpio_ich igb mpt2sas i2c_algo_bit sb_edac scsi_transport_sas ptp ioatdma ipmi_si serio_raw hpwdt edac_core lpc_ich pps_core hpilo be2net dca ipmi_msghandler raid_class mac_hid acpi_power_meter lp parport shpchp hpsa
[ 200.911599] CPU: 12 PID: 34798 Comm: ip Not tainted 3.13.0 #1
[ 200.911600] Hardware name: HP ProLiant SL230s Gen8/, BIOS P75 08/20/2012
[ 200.911602] task: ffff880fe4d3ae60 ti: ffff880fe4c9e000 task.ti: ffff880fe4c9e000
[ 200.911603] RIP: 0010:[<ffffffff811ad1e8>] [<ffffffff811ad1e8>] __lookup_mnt+0x68/0x80
[ 200.911609] RSP: 0018:ffff880fe4c9fcd8 EFLAGS: 00000282
[ 200.911610] RAX: ffff880fe7151400 RBX: ffffffff8119859b RCX: ffff881ff9464370
[ 200.911612] RDX: ffff880fe7151420 RSI: ffff8800b985e9c0 RDI: ffff880fe94cd1a0
[ 200.911613] RBP: ffff880fe4c9fce8 R08: 326f6d7500737973 R09: ffff8800b985e9c0
[ 200.911614] R10: 003f010100030103 R11: ffffc90000002000 R12: ffff880fe4c9fe00
[ 200.911615] R13: ffff880fe244bc08 R14: 0000000000000000 R15: ffff881fe9ce21f0
[ 200.911616] FS: 00007fe7048e0700(0000) GS:ffff88203f880000(0000) knlGS:0000000000000000
[ 200.911617] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 200.911618] CR2: 000000000043e408 CR3: 0000001fe94bf000 CR4: 00000000000407e0
[ 200.911620] Stack:
[ 200.911621] ffff881fe87c62fc ffff880fe7151400 ffff880fe4c9fd18 ffffffff811ad2b0
[ 200.911624] ffff880fe4c9fdb8 ffff880fe4c9ff28 ffff881ff65949c0 ffff880fe4c9ff28
[ 200.911626] ffff880fe4c9fd38 ffffffff8119855e ffff880fe4c9fda8 ffff881ff65949c0
[ 200.911629] Call Trace:
[ 200.911632] [<ffffffff811ad2b0>] lookup_mnt+0x30/0x70
[ 200.911636] [<ffffffff8119855e>] follow_mount+0x5e/0x70
[ 200.911639] [<ffffffff81199070>] mountpoint_last+0xc0/0x1d0
[ 200.911641] [<ffffffff8119ad47>] path_mountpoint+0xd7/0x440
[ 200.911646] [<ffffffff810953c9>] ? __rwsem_do_wake+0x119/0x170
[ 200.911651] [<ffffffff81352e17>] ? call_rwsem_wake+0x17/0x30
[ 200.911655] [<ffffffff8117f551>] ? kmem_cache_alloc+0x31/0x160
[ 200.911657] [<ffffffff8119922c>] ? getname_flags+0x5c/0x190
[ 200.911659] [<ffffffff8119b0e4>] filename_mountpoint+0x34/0xc0
[ 200.911661] [<ffffffff8119e65a>] user_path_mountpoint_at+0x4a/0x70
[ 200.911664] [<ffffffff811adcef>] SyS_umount+0x7f/0x3b0
[ 200.911665] [<ffffffff811af548>] ? SyS_mount+0xb8/0xe0
[ 200.911669] [<ffffffff816c3552>] system_call_fastpath+0x16/0x1b
[ 200.911670] Code: 48 8b 55 f8 31 c0 48 39 ca 74 2b 48 89 d0 eb 13 0f 1f 00 48 8b 00 48 89 45 f8 48 8b 45 f8 48 39 c8 74 18 48 8b 50 10 48 83 c2 20 <48> 39 d7 75 e3 48 39 70 18 75 dd c9 c3 0f 1f 00 31 c0 c9 c3 0f
^ permalink raw reply
* Re: ip_set: protocol %u message -- useful?
From: Cong Wang @ 2014-02-13 18:32 UTC (permalink / raw)
To: Jozsef Kadlecsik
Cc: Ilia Mirkin, netdev, linux-kernel@vger.kernel.org,
Patrick McHardy, Vitaly Lavrov
In-Reply-To: <alpine.DEB.2.10.1402131117120.16734@blackhole.kfki.hu>
On Thu, Feb 13, 2014 at 2:30 AM, Jozsef Kadlecsik
<kadlec@blackhole.kfki.hu> wrote:
> On Thu, 13 Feb 2014, Ilia Mirkin wrote:
>> messages in my dmesg. This might be because of some local
>> configuration changes I've made, or perhaps a kernel upgrade. Either
>> way, it appears this message has been a pr_notice since the original
>> code added it in a7b4f989a62 ("netfilter: ipset: IP set core
>> support").
>>
>> Does this message provide a lot of value? Or could it be made into a pr_debug?
>
> That's a report message on the protocol version used by the ipset
> subsystem. There was (and possibly will be) multiple protocols, so it
> helps to catch basic userpsace/kernelspace communication issues.
But still it doesn't deserve a pr_notice()... pr_info() should be enough.
diff --git a/net/netfilter/ipset/ip_set_core.c
b/net/netfilter/ipset/ip_set_core.c
index de770ec..5ea063f 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1945,7 +1945,7 @@ ip_set_net_init(struct net *net)
return -ENOMEM;
inst->is_deleted = 0;
rcu_assign_pointer(inst->ip_set_list, list);
- pr_notice("ip_set: protocol %u\n", IPSET_PROTOCOL);
+ pr_info("ip_set: protocol %u\n", IPSET_PROTOCOL);
return 0;
}
^ permalink raw reply related
* [PATCH Resend] net: asix: add missing flag to struct driver_info
From: Emil Goode @ 2014-02-13 18:30 UTC (permalink / raw)
To: David S. Miller, Ming Lei, Mark Brown, Jeff Kirsher, Glen Turner
Cc: linux-usb, netdev, linux-kernel, Emil Goode
The struct driver_info ax88178_info is assigned the function
asix_rx_fixup_common as it's rx_fixup callback. This means that
FLAG_MULTI_PACKET must be set as this function is cloning the
data and calling usbnet_skb_return. Not setting this flag leads
to usbnet_skb_return beeing called a second time from within
the rx_process function in the usbnet module.
Signed-off-by: Emil Goode <emilgoode@gmail.com>
Reported-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/asix_devices.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 9765a7d..5d19409 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -917,7 +917,8 @@ static const struct driver_info ax88178_info = {
.status = asix_status,
.link_reset = ax88178_link_reset,
.reset = ax88178_reset,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
+ FLAG_MULTI_PACKET,
.rx_fixup = asix_rx_fixup_common,
.tx_fixup = asix_tx_fixup,
};
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: remove useless if check from register_netdevice()
From: Cong Wang @ 2014-02-13 18:28 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: Alexei Starovoitov, David S. Miller, netdev
In-Reply-To: <CAOJe8K2iTsv-HUEotjwpc7s9LMEU6+nMfK5VWWF0tcMLhYBS6Q@mail.gmail.com>
On Thu, Feb 13, 2014 at 9:15 AM, Denis Kirjanov <kda@linux-powerpc.org> wrote:
> All of the users of ndo_init return negative error code on error path.
> Only staging stuff tries to use positive codes like octeon ethernet
> which does request_irq inside ndo_open...
>
You should put this explanation in the changelog.
^ permalink raw reply
* Re: ovs inconsistent lock state
From: Jiri Pirko @ 2014-02-13 18:12 UTC (permalink / raw)
To: Pravin Shelar; +Cc: netdev, Jesse Gross, dev@openvswitch.org
In-Reply-To: <CALnjE+oKrqMVWCrS3kSomxnsSJMFDpFsK8KY+oK1kVTd0Lb8Wg@mail.gmail.com>
Thu, Feb 13, 2014 at 06:42:03PM CET, pshelar@nicira.com wrote:
>On Thu, Feb 13, 2014 at 9:13 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Hi.
>>
>> On current net-next I'm getting following message once I add a dp:
>>
>> [ 3014.523665] =================================
>> [ 3014.524118] [ INFO: inconsistent lock state ]
>> [ 3014.524118] 3.14.0-rc2+ #1 Not tainted
>> [ 3014.524118] ---------------------------------
>> [ 3014.524118] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
>> [ 3014.524118] swapper/1/0 [HC0[0]:SC1[5]:HE1:SE0] takes:
>> [ 3014.524118] (&(&flow->stats.stat->lock)->rlock){+.?...}, at: [<ffffffffa021b6ef>] ovs_flow_stats_update+0x4f/0xd0 [openvswitch]
>> [ 3014.524118] {SOFTIRQ-ON-W} state was registered at:
>> [ 3014.524118] [<ffffffff810d6843>] __lock_acquire+0x5a3/0x1c30
>> [ 3014.524118] [<ffffffff810d7f80>] lock_acquire+0xb0/0x150
>> [ 3014.524118] [<ffffffff81705c99>] _raw_spin_lock+0x39/0x50
>> [ 3014.524118] [<ffffffffa021b8d3>] ovs_flow_stats_get+0x163/0x1b0 [openvswitch]
>> [ 3014.524118] [<ffffffffa0218ce5>] ovs_flow_cmd_fill_info+0x165/0x2b0 [openvswitch]
>> [ 3014.524118] [<ffffffffa021904c>] ovs_flow_cmd_build_info.constprop.27+0x6c/0xa0 [openvswitch]
>> [ 3014.524118] [<ffffffffa0219519>] ovs_flow_cmd_new_or_set+0x499/0x4f0 [openvswitch]
>> [ 3014.524118] [<ffffffff8161bf09>] genl_family_rcv_msg+0x199/0x390
>> [ 3014.524118] [<ffffffff8161c18e>] genl_rcv_msg+0x8e/0xd0
>> [ 3014.524118] [<ffffffff8161a189>] netlink_rcv_skb+0xa9/0xc0
>> [ 3014.524118] [<ffffffff8161a6a8>] genl_rcv+0x28/0x40
>> [ 3014.524118] [<ffffffff816197d0>] netlink_unicast+0xf0/0x1b0
>> [ 3014.524118] [<ffffffff81619b8f>] netlink_sendmsg+0x2ff/0x740
>> [ 3014.524118] [<ffffffff815ce86b>] sock_sendmsg+0x8b/0xc0
>> [ 3014.524118] [<ffffffff815cf369>] ___sys_sendmsg+0x389/0x3a0
>> [ 3014.524118] [<ffffffff815cfa62>] __sys_sendmsg+0x42/0x80
>> [ 3014.524118] [<ffffffff815cfab2>] SyS_sendmsg+0x12/0x20
>> [ 3014.524118] [<ffffffff8170f829>] system_call_fastpath+0x16/0x1b
>> [ 3014.524118] irq event stamp: 6163288
>> [ 3014.524118] hardirqs last enabled at (6163288): [<ffffffff8169f8a0>] ip6_finish_output2+0x380/0x670
>> [ 3014.524118] hardirqs last disabled at (6163287): [<ffffffff8169f85b>] ip6_finish_output2+0x33b/0x670
>> [ 3014.524118] softirqs last enabled at (6163266): [<ffffffff810888a2>] _local_bh_enable+0x22/0x50
>> [ 3014.524118] softirqs last disabled at (6163267): [<ffffffff81089f15>] irq_exit+0xc5/0xd0
>> [ 3014.524118]
>> other info that might help us debug this:
>> [ 3014.524118] Possible unsafe locking scenario:
>>
>> [ 3014.524118] CPU0
>> [ 3014.524118] ----
>> [ 3014.524118] lock(&(&flow->stats.stat->lock)->rlock);
>> [ 3014.524118] <Interrupt>
>> [ 3014.524118] lock(&(&flow->stats.stat->lock)->rlock);
>> [ 3014.524118]
>> *** DEADLOCK ***
>>
>> [ 3014.524118] 5 locks held by swapper/1/0:
>> [ 3014.524118] #0: (((&idev->mc_ifc_timer))){+.-...}, at: [<ffffffff81090095>] call_timer_fn+0x5/0x180
>> [ 3014.524118] #1: (rcu_read_lock){.+.+..}, at: [<ffffffff816c5755>] mld_sendpack+0x5/0x330
>> [ 3014.524118] #2: (rcu_read_lock_bh){.+....}, at: [<ffffffff8169f579>] ip6_finish_output2+0x59/0x670
>> [ 3014.524118] #3: (rcu_read_lock_bh){.+....}, at: [<ffffffff815eb585>] __dev_queue_xmit+0x5/0x6a0
>> [ 3014.524118] #4: (rcu_read_lock){.+.+..}, at: [<ffffffffa0221995>] internal_dev_xmit+0x5/0x90 [openvswitch]
>> [ 3014.524118]
>> stack backtrace:
>> [ 3014.524118] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.14.0-rc2+ #1
>> [ 3014.524118] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
>> [ 3014.524118] ffffffff82338940 ffff8800bb2838f0 ffffffff816fd1ac ffff8800b6b33570
>> [ 3014.524118] ffff8800bb283940 ffffffff816f9144 0000000000000005 ffff880000000001
>> [ 3014.524118] ffff880000000000 0000000000000006 ffff8800b6b33570 ffffffff810d4380
>> [ 3014.524118] Call Trace:
>> [ 3014.524118] <IRQ> [<ffffffff816fd1ac>] dump_stack+0x4d/0x66
>> [ 3014.524118] [<ffffffff816f9144>] print_usage_bug+0x1f3/0x204
>> [ 3014.524118] [<ffffffff810d4380>] ? check_usage_backwards+0x130/0x130
>> [ 3014.524118] [<ffffffff810d4da0>] mark_lock+0x270/0x300
>> [ 3014.524118] [<ffffffff810d6718>] __lock_acquire+0x478/0x1c30
>> [ 3014.524118] [<ffffffff810d4ede>] ? mark_held_locks+0xae/0x130
>> [ 3014.524118] [<ffffffffa0220047>] ? find_bucket.isra.1+0x67/0x70 [openvswitch]
>> [ 3014.524118] [<ffffffffa0220228>] ? masked_flow_lookup+0x78/0x140 [openvswitch]
>> [ 3014.524118] [<ffffffff810d669a>] ? __lock_acquire+0x3fa/0x1c30
>> [ 3014.524118] [<ffffffff810d7f80>] lock_acquire+0xb0/0x150
>> [ 3014.524118] [<ffffffffa021b6ef>] ? ovs_flow_stats_update+0x4f/0xd0 [openvswitch]
>> [ 3014.524118] [<ffffffff81705c99>] _raw_spin_lock+0x39/0x50
>> [ 3014.524118] [<ffffffffa021b6ef>] ? ovs_flow_stats_update+0x4f/0xd0 [openvswitch]
>> [ 3014.524118] [<ffffffffa021b6ef>] ovs_flow_stats_update+0x4f/0xd0 [openvswitch]
>> [ 3014.524118] [<ffffffffa021ac75>] ovs_dp_process_received_packet+0x75/0x100 [openvswitch]
>> [ 3014.524118] [<ffffffffa02213ea>] ovs_vport_receive+0x2a/0x30 [openvswitch]
>> [ 3014.524118] [<ffffffffa02219e6>] internal_dev_xmit+0x56/0x90 [openvswitch]
>> [ 3014.524118] [<ffffffffa0221995>] ? internal_dev_xmit+0x5/0x90 [openvswitch]
>> [ 3014.524118] [<ffffffff815eb276>] dev_hard_start_xmit+0x316/0x620
>> [ 3014.524118] [<ffffffff815eb8ff>] __dev_queue_xmit+0x37f/0x6a0
>> [ 3014.524118] [<ffffffff815eb585>] ? __dev_queue_xmit+0x5/0x6a0
>> [ 3014.524118] [<ffffffff8169f8a0>] ? ip6_finish_output2+0x380/0x670
>> [ 3014.524118] [<ffffffff815ebc30>] dev_queue_xmit+0x10/0x20
>> [ 3014.524118] [<ffffffff8169f900>] ip6_finish_output2+0x3e0/0x670
>> [ 3014.524118] [<ffffffff816a31aa>] ? ip6_finish_output+0x9a/0x200
>> [ 3014.524118] [<ffffffff816a31aa>] ip6_finish_output+0x9a/0x200
>> [ 3014.524118] [<ffffffff816a335f>] ip6_output+0x4f/0x1d0
>> [ 3014.524118] [<ffffffff816c58e6>] mld_sendpack+0x196/0x330
>> [ 3014.524118] [<ffffffff816c5755>] ? mld_sendpack+0x5/0x330
>> [ 3014.524118] [<ffffffff816c63cd>] mld_ifc_timer_expire+0x19d/0x2d0
>> [ 3014.524118] [<ffffffff816c6230>] ? mld_dad_timer_expire+0x70/0x70
>> [ 3014.524118] [<ffffffff8109010a>] call_timer_fn+0x7a/0x180
>> [ 3014.524118] [<ffffffff81090095>] ? call_timer_fn+0x5/0x180
>> [ 3014.524118] [<ffffffff816c6230>] ? mld_dad_timer_expire+0x70/0x70
>> [ 3014.524118] [<ffffffff8109054c>] run_timer_softirq+0x20c/0x2c0
>> [ 3014.524118] [<ffffffff810899fd>] __do_softirq+0x12d/0x310
>> [ 3014.524118] [<ffffffff81089f15>] irq_exit+0xc5/0xd0
>> [ 3014.524118] [<ffffffff81711b75>] smp_apic_timer_interrupt+0x45/0x60
>> [ 3014.524118] [<ffffffff817104b2>] apic_timer_interrupt+0x72/0x80
>> [ 3014.524118] <EOI> [<ffffffff81052486>] ? native_safe_halt+0x6/0x10
>> [ 3014.524118] [<ffffffff8101ec04>] default_idle+0x24/0xe0
>> [ 3014.524118] [<ffffffff8101f54e>] arch_cpu_idle+0x2e/0x40
>> [ 3014.524118] [<ffffffff810e96ae>] cpu_startup_entry+0x9e/0x280
>> [ 3014.524118] [<ffffffff81044929>] start_secondary+0x1d9/0x280
>> --
>
>Since ovs disable bh while reading stats from local cpu, deadlock can
>not occur. This is false positive.
I fail to see where bh is disabled. ovs_dp_process_received_packet() is
called with rcu_read_lock...
>
>> To unsubscribe from this list: send the line "unsubscribe netdev" 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
* [PATCH net-next] net-sysfs: get_netdev_queue_index() cleanup
From: Eric Dumazet @ 2014-02-13 18:07 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
Remove one inline keyword, and no need for a loop to find
an index into a table.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/core/net-sysfs.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 93886246a0b4..73aa594674ef 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -996,15 +996,12 @@ static struct attribute_group dql_group = {
#endif /* CONFIG_BQL */
#ifdef CONFIG_XPS
-static inline unsigned int get_netdev_queue_index(struct netdev_queue *queue)
+static unsigned int get_netdev_queue_index(struct netdev_queue *queue)
{
struct net_device *dev = queue->dev;
- int i;
-
- for (i = 0; i < dev->num_tx_queues; i++)
- if (queue == &dev->_tx[i])
- break;
+ unsigned int i;
+ i = queue - dev->_tx;
BUG_ON(i >= dev->num_tx_queues);
return i;
^ permalink raw reply related
* Intermittent lockups creating/manipulating network namespaces (was Re: large degradation in ip netns add/exec performance in 3.13?)
From: Rick Jones @ 2014-02-13 18:02 UTC (permalink / raw)
To: netdev
In-Reply-To: <52FAC6D1.50702@hp.com>
On 02/11/2014 04:56 PM, Rick Jones wrote:
> On 02/11/2014 02:26 PM, Rick Jones wrote:
>> I did have the system lockup once at 16 concurrent streams on the 3.13.0
>> kernel. Didn't happen the next two times I tried.
>
> I've had it happen again with a slightly tweaked ("heavier") fake router
> creation. I'm guessing the attached soft lockup messages are related
> and may help point somewhere.
The 16 concurrent streams lockup. The "rest of the system" keeps going.
I've had the same hang happen with 3.14.0-rc2 as well. I was able to
take a crash dump via "echo c > /proc/sysrq-trigger" but I have no clue
how to look at it. More than willing to upload it to netperf.org.
rick jones
^ permalink raw reply
* Re: ovs inconsistent lock state
From: Pravin Shelar @ 2014-02-13 17:42 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, Jesse Gross, dev@openvswitch.org
In-Reply-To: <20140213171307.GC2829@minipsycho.orion>
On Thu, Feb 13, 2014 at 9:13 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Hi.
>
> On current net-next I'm getting following message once I add a dp:
>
> [ 3014.523665] =================================
> [ 3014.524118] [ INFO: inconsistent lock state ]
> [ 3014.524118] 3.14.0-rc2+ #1 Not tainted
> [ 3014.524118] ---------------------------------
> [ 3014.524118] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
> [ 3014.524118] swapper/1/0 [HC0[0]:SC1[5]:HE1:SE0] takes:
> [ 3014.524118] (&(&flow->stats.stat->lock)->rlock){+.?...}, at: [<ffffffffa021b6ef>] ovs_flow_stats_update+0x4f/0xd0 [openvswitch]
> [ 3014.524118] {SOFTIRQ-ON-W} state was registered at:
> [ 3014.524118] [<ffffffff810d6843>] __lock_acquire+0x5a3/0x1c30
> [ 3014.524118] [<ffffffff810d7f80>] lock_acquire+0xb0/0x150
> [ 3014.524118] [<ffffffff81705c99>] _raw_spin_lock+0x39/0x50
> [ 3014.524118] [<ffffffffa021b8d3>] ovs_flow_stats_get+0x163/0x1b0 [openvswitch]
> [ 3014.524118] [<ffffffffa0218ce5>] ovs_flow_cmd_fill_info+0x165/0x2b0 [openvswitch]
> [ 3014.524118] [<ffffffffa021904c>] ovs_flow_cmd_build_info.constprop.27+0x6c/0xa0 [openvswitch]
> [ 3014.524118] [<ffffffffa0219519>] ovs_flow_cmd_new_or_set+0x499/0x4f0 [openvswitch]
> [ 3014.524118] [<ffffffff8161bf09>] genl_family_rcv_msg+0x199/0x390
> [ 3014.524118] [<ffffffff8161c18e>] genl_rcv_msg+0x8e/0xd0
> [ 3014.524118] [<ffffffff8161a189>] netlink_rcv_skb+0xa9/0xc0
> [ 3014.524118] [<ffffffff8161a6a8>] genl_rcv+0x28/0x40
> [ 3014.524118] [<ffffffff816197d0>] netlink_unicast+0xf0/0x1b0
> [ 3014.524118] [<ffffffff81619b8f>] netlink_sendmsg+0x2ff/0x740
> [ 3014.524118] [<ffffffff815ce86b>] sock_sendmsg+0x8b/0xc0
> [ 3014.524118] [<ffffffff815cf369>] ___sys_sendmsg+0x389/0x3a0
> [ 3014.524118] [<ffffffff815cfa62>] __sys_sendmsg+0x42/0x80
> [ 3014.524118] [<ffffffff815cfab2>] SyS_sendmsg+0x12/0x20
> [ 3014.524118] [<ffffffff8170f829>] system_call_fastpath+0x16/0x1b
> [ 3014.524118] irq event stamp: 6163288
> [ 3014.524118] hardirqs last enabled at (6163288): [<ffffffff8169f8a0>] ip6_finish_output2+0x380/0x670
> [ 3014.524118] hardirqs last disabled at (6163287): [<ffffffff8169f85b>] ip6_finish_output2+0x33b/0x670
> [ 3014.524118] softirqs last enabled at (6163266): [<ffffffff810888a2>] _local_bh_enable+0x22/0x50
> [ 3014.524118] softirqs last disabled at (6163267): [<ffffffff81089f15>] irq_exit+0xc5/0xd0
> [ 3014.524118]
> other info that might help us debug this:
> [ 3014.524118] Possible unsafe locking scenario:
>
> [ 3014.524118] CPU0
> [ 3014.524118] ----
> [ 3014.524118] lock(&(&flow->stats.stat->lock)->rlock);
> [ 3014.524118] <Interrupt>
> [ 3014.524118] lock(&(&flow->stats.stat->lock)->rlock);
> [ 3014.524118]
> *** DEADLOCK ***
>
> [ 3014.524118] 5 locks held by swapper/1/0:
> [ 3014.524118] #0: (((&idev->mc_ifc_timer))){+.-...}, at: [<ffffffff81090095>] call_timer_fn+0x5/0x180
> [ 3014.524118] #1: (rcu_read_lock){.+.+..}, at: [<ffffffff816c5755>] mld_sendpack+0x5/0x330
> [ 3014.524118] #2: (rcu_read_lock_bh){.+....}, at: [<ffffffff8169f579>] ip6_finish_output2+0x59/0x670
> [ 3014.524118] #3: (rcu_read_lock_bh){.+....}, at: [<ffffffff815eb585>] __dev_queue_xmit+0x5/0x6a0
> [ 3014.524118] #4: (rcu_read_lock){.+.+..}, at: [<ffffffffa0221995>] internal_dev_xmit+0x5/0x90 [openvswitch]
> [ 3014.524118]
> stack backtrace:
> [ 3014.524118] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.14.0-rc2+ #1
> [ 3014.524118] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [ 3014.524118] ffffffff82338940 ffff8800bb2838f0 ffffffff816fd1ac ffff8800b6b33570
> [ 3014.524118] ffff8800bb283940 ffffffff816f9144 0000000000000005 ffff880000000001
> [ 3014.524118] ffff880000000000 0000000000000006 ffff8800b6b33570 ffffffff810d4380
> [ 3014.524118] Call Trace:
> [ 3014.524118] <IRQ> [<ffffffff816fd1ac>] dump_stack+0x4d/0x66
> [ 3014.524118] [<ffffffff816f9144>] print_usage_bug+0x1f3/0x204
> [ 3014.524118] [<ffffffff810d4380>] ? check_usage_backwards+0x130/0x130
> [ 3014.524118] [<ffffffff810d4da0>] mark_lock+0x270/0x300
> [ 3014.524118] [<ffffffff810d6718>] __lock_acquire+0x478/0x1c30
> [ 3014.524118] [<ffffffff810d4ede>] ? mark_held_locks+0xae/0x130
> [ 3014.524118] [<ffffffffa0220047>] ? find_bucket.isra.1+0x67/0x70 [openvswitch]
> [ 3014.524118] [<ffffffffa0220228>] ? masked_flow_lookup+0x78/0x140 [openvswitch]
> [ 3014.524118] [<ffffffff810d669a>] ? __lock_acquire+0x3fa/0x1c30
> [ 3014.524118] [<ffffffff810d7f80>] lock_acquire+0xb0/0x150
> [ 3014.524118] [<ffffffffa021b6ef>] ? ovs_flow_stats_update+0x4f/0xd0 [openvswitch]
> [ 3014.524118] [<ffffffff81705c99>] _raw_spin_lock+0x39/0x50
> [ 3014.524118] [<ffffffffa021b6ef>] ? ovs_flow_stats_update+0x4f/0xd0 [openvswitch]
> [ 3014.524118] [<ffffffffa021b6ef>] ovs_flow_stats_update+0x4f/0xd0 [openvswitch]
> [ 3014.524118] [<ffffffffa021ac75>] ovs_dp_process_received_packet+0x75/0x100 [openvswitch]
> [ 3014.524118] [<ffffffffa02213ea>] ovs_vport_receive+0x2a/0x30 [openvswitch]
> [ 3014.524118] [<ffffffffa02219e6>] internal_dev_xmit+0x56/0x90 [openvswitch]
> [ 3014.524118] [<ffffffffa0221995>] ? internal_dev_xmit+0x5/0x90 [openvswitch]
> [ 3014.524118] [<ffffffff815eb276>] dev_hard_start_xmit+0x316/0x620
> [ 3014.524118] [<ffffffff815eb8ff>] __dev_queue_xmit+0x37f/0x6a0
> [ 3014.524118] [<ffffffff815eb585>] ? __dev_queue_xmit+0x5/0x6a0
> [ 3014.524118] [<ffffffff8169f8a0>] ? ip6_finish_output2+0x380/0x670
> [ 3014.524118] [<ffffffff815ebc30>] dev_queue_xmit+0x10/0x20
> [ 3014.524118] [<ffffffff8169f900>] ip6_finish_output2+0x3e0/0x670
> [ 3014.524118] [<ffffffff816a31aa>] ? ip6_finish_output+0x9a/0x200
> [ 3014.524118] [<ffffffff816a31aa>] ip6_finish_output+0x9a/0x200
> [ 3014.524118] [<ffffffff816a335f>] ip6_output+0x4f/0x1d0
> [ 3014.524118] [<ffffffff816c58e6>] mld_sendpack+0x196/0x330
> [ 3014.524118] [<ffffffff816c5755>] ? mld_sendpack+0x5/0x330
> [ 3014.524118] [<ffffffff816c63cd>] mld_ifc_timer_expire+0x19d/0x2d0
> [ 3014.524118] [<ffffffff816c6230>] ? mld_dad_timer_expire+0x70/0x70
> [ 3014.524118] [<ffffffff8109010a>] call_timer_fn+0x7a/0x180
> [ 3014.524118] [<ffffffff81090095>] ? call_timer_fn+0x5/0x180
> [ 3014.524118] [<ffffffff816c6230>] ? mld_dad_timer_expire+0x70/0x70
> [ 3014.524118] [<ffffffff8109054c>] run_timer_softirq+0x20c/0x2c0
> [ 3014.524118] [<ffffffff810899fd>] __do_softirq+0x12d/0x310
> [ 3014.524118] [<ffffffff81089f15>] irq_exit+0xc5/0xd0
> [ 3014.524118] [<ffffffff81711b75>] smp_apic_timer_interrupt+0x45/0x60
> [ 3014.524118] [<ffffffff817104b2>] apic_timer_interrupt+0x72/0x80
> [ 3014.524118] <EOI> [<ffffffff81052486>] ? native_safe_halt+0x6/0x10
> [ 3014.524118] [<ffffffff8101ec04>] default_idle+0x24/0xe0
> [ 3014.524118] [<ffffffff8101f54e>] arch_cpu_idle+0x2e/0x40
> [ 3014.524118] [<ffffffff810e96ae>] cpu_startup_entry+0x9e/0x280
> [ 3014.524118] [<ffffffff81044929>] start_secondary+0x1d9/0x280
> --
Since ovs disable bh while reading stats from local cpu, deadlock can
not occur. This is false positive.
> To unsubscribe from this list: send the line "unsubscribe netdev" 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: [PATCH v2] usbnet: remove generic hard_header_len check
From: Emil Goode @ 2014-02-13 17:29 UTC (permalink / raw)
To: David Laight
Cc: Steve Glendinning, Oliver Neukum, Bjørn Mork,
David S. Miller, Freddy Xin, Eric Dumazet, Ming Lei,
Paul Gortmaker, Jeff Kirsher, Liu Junliang, Octavian Purdila,
linux-usb@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D0F6BEF3A@AcuExch.aculab.com>
On Thu, Feb 13, 2014 at 04:56:58PM +0000, David Laight wrote:
> From: Of Emil Goode
> > This patch removes a generic hard_header_len check from the usbnet
> > module that is causing dropped packages under certain circumstances
> > for devices that send rx packets that cross urb boundaries.
> >
> > One example is the AX88772B which occasionally send rx packets that
> > cross urb boundaries where the remaining partial packet is sent with
> > no hardware header. When the buffer with a partial packet is of less
> > number of octets than the value of hard_header_len the buffer is
> > discarded by the usbnet module.
> ...
> > diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
> > index d6f64da..955df81 100644
> > --- a/drivers/net/usb/ax88179_178a.c
> > +++ b/drivers/net/usb/ax88179_178a.c
> > @@ -1118,6 +1118,10 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> > u16 hdr_off;
> > u32 *pkt_hdr;
> >
> > + /* This check is no longer done by usbnet */
> > + if (skb->len < dev->net->hard_header_len)
> > + return 0;
> > +
>
> The ax88179 driver can also receive ethernet frames that cross the
> end of rx URB.
> It should have the code to save the last fragment (of a urb) until
> the next data arrives, and then correctly merge the fragments.
>
> It is likely that any sub-driver that sets the receive urb length
> to a multiple of 1k (rather than leaving it at hard_hdr+mtu) can defrag
> rx data that crosses urb boundaries.
Yes probably the check is unnecessary to many of the devices but
it should be tested by someone who has the hardware.
If you have a ax88179 device to test this patch but without the
hard_header_len check added to the ax88179_rx_fixup function that
would be great. Then we don't need to introduce that check.
Best regards,
Emil Goode
^ permalink raw reply
* Re: [PATCH v2] net: sctp: Fix a_rwnd/rwnd management to reflect real state of the receiver's buffer
From: Vlad Yasevich @ 2014-02-13 17:14 UTC (permalink / raw)
To: Matija Glavinic Pecotic
Cc: linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
Alexander Sverdlin
In-Reply-To: <52FA8073.2060607@nsn.com>
On 02/11/2014 02:56 PM, Matija Glavinic Pecotic wrote:
> Hello Vlad,
>
> On 02/11/2014 03:52 PM, ext Vlad Yasevich wrote:
>> Hi Matija
>>
>> On 02/09/2014 02:15 AM, Matija Glavinic Pecotic wrote:
>>>
>>> --- net-next.orig/net/sctp/ulpevent.c
>>> +++ net-next/net/sctp/ulpevent.c
>>> @@ -989,7 +989,7 @@ static void sctp_ulpevent_receive_data(s
>>> skb = sctp_event2skb(event);
>>> /* Set the owner and charge rwnd for bytes received. */
>>> sctp_ulpevent_set_owner(event, asoc);
>>> - sctp_assoc_rwnd_decrease(asoc, skb_headlen(skb));
>>> + sctp_assoc_rwnd_update(asoc, false);
>>>
>>> if (!skb->data_len)
>>> return;
>>> @@ -1035,8 +1035,9 @@ static void sctp_ulpevent_release_data(s
>>> }
>>>
>>> done:
>>> - sctp_assoc_rwnd_increase(event->asoc, len);
>>> - sctp_ulpevent_release_owner(event);
>>> + atomic_sub(event->rmem_len, &event->asoc->rmem_alloc);
>>> + sctp_assoc_rwnd_update(event->asoc, true);
>>> + sctp_association_put(event->asoc)
>>
>> Can't we simply change the order of window update and release instead
>> of open coding it like this?
>
> that was the initial idea, but sctp_ulpevent_release_owner puts
> the association and calls sctp_association_destroy if its time to
> do so. IMHO, in the case if we would switch it, we would open a
> potential race condition.
On the tx side, I agree that there would be race. One the recieve side,
I don't think you could ever be in the codition where the last reference
on the asoc is held by a data chunk you are feeing.
However, to be completely safe we could do:
asoc = event->asoc;
sctp_association_hold(asoc);
sctp_ulpevent_release_owner(event);
sctp_assoc_rwnd_update(asoc, true);
sctp_assocition_put(asoc);
The reason I don't like to open-code release owner is that if it
ever changes, we'll have to rememeber to change this open-coded
implementation as well.
Thanks
-vlad
>
> I agree this doesn't look the best. But since we should call
> sctp_assoc_rwnd_update after accounting and before put, we have only
> option to move sctp_assoc_rwnd_update to _ulpevent_release_owner. As on
> this path we wish to update peer and generate sack, but we for sure do not
> want it on all paths where ulpevent_release_owner is used, I see no
> alternative but to add additional parameter to ulpevent_release_owner
> which would be just passed to rwnd_update - bool update_peer. On the
> other hand, I wonder whether ulpevent_release_owner would do more then
> it should in that case?
>
>>
>> -vlad
>>
>>> }
>>>
>>> static void sctp_ulpevent_release_frag_data(struct sctp_ulpevent
*event)
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" 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
* [PATCH net] packet: check for ndo_select_queue during queue selection
From: Daniel Borkmann @ 2014-02-13 17:18 UTC (permalink / raw)
To: davem; +Cc: mathias.kretschmer, netdev, Jesper Dangaard Brouer
Mathias reported that on an AMD Geode LX embedded board (ALiX)
with ath9k driver PACKET_QDISC_BYPASS triggers a WARN_ON()
coming from the driver itself via 066dae93bdf ("ath9k: rework
tx queue selection and fix queue stopping/waking"). The reason
why this happened is that ndo_select_queue() call is not
invoked from direct xmit path i.e. for ieee80211 subsystem
that sets queue and TID (similar to 802.1d tag) which is being
put into the frame through 802.11e (WMM, QoS). If that is not
set, pending frame counter for e.g. ath9k can get messed up.
So the WARN_ON() in ath9k is absolutely legitimate. Generally,
the hw queue selection in ieee80211 depends on the type of
traffic, and priorities are set according to ieee80211_ac_numbers
mapping; working in a similar way as DiffServ only on a lower
layer, so that the AP can favour frames that have "real-time"
requirements like voice or video data frames. We were evaluating
this and concluded that it's best for PACKET_QDISC_BYPASS to
just probe for existance of this function and invoke it if
available so that drivers that implement it can make an
informed decision to which hw queue the frame should be
dispatched to. Only few drivers implement this function, so
for the majority of drivers nothing changes. The original
pktgen scenario for which PACKET_QDISC_BYPASS was designed
for is still intact with this change anyway. We think
restricting this feature only to ethernet drivers would be an
artificial barrier where we would instead only need to call
ndo_select_queue(). Besides, checking for ARPHRD_ETHER as pktgen
does wouldn't be sufficient as also many ieee80211 drivers
use it as dev->type. If side-effects that we documented are
undesired or use case involves buffering, then people just go
with the normal dev_queue_xmit() that is on default enabled.
Reported-by: Mathias Kretschmer <mathias.kretschmer@fokus.fraunhofer.de>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
---
Intended for 3.14.
include/linux/netdevice.h | 20 ++++++++++++++++++++
net/core/flow_dissector.c | 13 +------------
net/packet/af_packet.c | 20 ++++++++++++++++----
3 files changed, 37 insertions(+), 16 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 440a02e..5f8c860 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2276,6 +2276,26 @@ static inline void netdev_reset_queue(struct net_device *dev_queue)
}
/**
+ * netdev_cap_txqueue - check if selected tx queue exceeds device queues
+ * @dev: network device
+ * @queue_index: given tx queue index
+ *
+ * Returns 0 if given tx queue index >= number of device tx queues,
+ * otherwise returns the originally passed tx queue index.
+ */
+static inline u16 netdev_cap_txqueue(struct net_device *dev, u16 queue_index)
+{
+ if (unlikely(queue_index >= dev->real_num_tx_queues)) {
+ net_warn_ratelimited("%s selects TX queue %d, but real number of TX queues is %d\n",
+ dev->name, queue_index,
+ dev->real_num_tx_queues);
+ return 0;
+ }
+
+ return queue_index;
+}
+
+/**
* netif_running - test if up
* @dev: network device
*
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 87577d4..d0e1fb1 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -323,17 +323,6 @@ u32 __skb_get_poff(const struct sk_buff *skb)
return poff;
}
-static inline u16 dev_cap_txqueue(struct net_device *dev, u16 queue_index)
-{
- if (unlikely(queue_index >= dev->real_num_tx_queues)) {
- net_warn_ratelimited("%s selects TX queue %d, but real number of TX queues is %d\n",
- dev->name, queue_index,
- dev->real_num_tx_queues);
- return 0;
- }
- return queue_index;
-}
-
static inline int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
{
#ifdef CONFIG_XPS
@@ -409,7 +398,7 @@ struct netdev_queue *netdev_pick_tx(struct net_device *dev,
queue_index = __netdev_pick_tx(dev, skb);
if (!accel_priv)
- queue_index = dev_cap_txqueue(dev, queue_index);
+ queue_index = netdev_cap_txqueue(dev, queue_index);
}
skb_set_queue_mapping(skb, queue_index);
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 6a2bb37..562ced9 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -308,9 +308,19 @@ static bool packet_use_direct_xmit(const struct packet_sock *po)
return po->xmit == packet_direct_xmit;
}
-static u16 packet_pick_tx_queue(struct net_device *dev)
+static void packet_pick_tx_queue(struct net_device *dev, struct sk_buff *skb)
{
- return (u16) raw_smp_processor_id() % dev->real_num_tx_queues;
+ const struct net_device_ops *ops = dev->netdev_ops;
+ u16 queue_index;
+
+ if (ops->ndo_select_queue) {
+ queue_index = ops->ndo_select_queue(dev, skb, NULL);
+ queue_index = netdev_cap_txqueue(dev, queue_index);
+ } else {
+ queue_index = raw_smp_processor_id() % dev->real_num_tx_queues;
+ }
+
+ skb_set_queue_mapping(skb, queue_index);
}
/* register_prot_hook must be invoked with the po->bind_lock held,
@@ -2285,7 +2295,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
}
}
- skb_set_queue_mapping(skb, packet_pick_tx_queue(dev));
+ packet_pick_tx_queue(dev, skb);
+
skb->destructor = tpacket_destruct_skb;
__packet_set_status(po, ph, TP_STATUS_SENDING);
packet_inc_pending(&po->tx_ring);
@@ -2499,7 +2510,8 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
skb->dev = dev;
skb->priority = sk->sk_priority;
skb->mark = sk->sk_mark;
- skb_set_queue_mapping(skb, packet_pick_tx_queue(dev));
+
+ packet_pick_tx_queue(dev, skb);
if (po->has_vnet_hdr) {
if (vnet_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH next resend] tcp: use zero-window when free_space is low
From: Rick Jones @ 2014-02-13 17:18 UTC (permalink / raw)
To: Florian Westphal, netdev; +Cc: Neal Cardwell, Eric Dumazet, Yuchung Cheng
In-Reply-To: <1392292350-28800-1-git-send-email-fw@strlen.de>
On 02/13/2014 03:52 AM, Florian Westphal wrote:
> Maybe someone has better idea?
Does tcp_rmem have to be a "hard" limit, or could it allowed to be fuzzy?
rick jones
^ permalink raw reply
* Re: [PATCH v2 net-next] net: remove useless if check from register_netdevice()
From: Denis Kirjanov @ 2014-02-13 17:15 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: David S. Miller, netdev
In-Reply-To: <CAADnVQJwP+ThozpeKOEvzoJRkYQvZgWyM9U+ixyeKKH9nkNb6w@mail.gmail.com>
All of the users of ndo_init return negative error code on error path.
Only staging stuff tries to use positive codes like octeon ethernet
which does request_irq inside ndo_open...
On 2/13/14, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Wed, Feb 12, 2014 at 8:47 PM, Denis Kirjanov <kda@linux-powerpc.org>
> wrote:
>> remove useless if check from register_netdevice()
>>
>> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
>> ---
>> v1 -> v2: Fixed identation
>> ---
>> net/core/dev.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 4ad1b78..21a72ad 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -5876,8 +5876,7 @@ int register_netdevice(struct net_device *dev)
>> if (dev->netdev_ops->ndo_init) {
>> ret = dev->netdev_ops->ndo_init(dev);
>> if (ret) {
>> - if (ret > 0)
>> - ret = -EIO;
>> + ret = -EIO;
>> goto out;
>
> why is it a useless check?
> seems perfectly valid to me.
> most of the time ndo_init() returns negative error code like -ENOMEM
> which we want
> to propagate further down and want to override it to -EIO if it's > 0
>
^ permalink raw reply
* ovs inconsistent lock state
From: Jiri Pirko @ 2014-02-13 17:13 UTC (permalink / raw)
To: netdev; +Cc: jesse, dev
Hi.
On current net-next I'm getting following message once I add a dp:
[ 3014.523665] =================================
[ 3014.524118] [ INFO: inconsistent lock state ]
[ 3014.524118] 3.14.0-rc2+ #1 Not tainted
[ 3014.524118] ---------------------------------
[ 3014.524118] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
[ 3014.524118] swapper/1/0 [HC0[0]:SC1[5]:HE1:SE0] takes:
[ 3014.524118] (&(&flow->stats.stat->lock)->rlock){+.?...}, at: [<ffffffffa021b6ef>] ovs_flow_stats_update+0x4f/0xd0 [openvswitch]
[ 3014.524118] {SOFTIRQ-ON-W} state was registered at:
[ 3014.524118] [<ffffffff810d6843>] __lock_acquire+0x5a3/0x1c30
[ 3014.524118] [<ffffffff810d7f80>] lock_acquire+0xb0/0x150
[ 3014.524118] [<ffffffff81705c99>] _raw_spin_lock+0x39/0x50
[ 3014.524118] [<ffffffffa021b8d3>] ovs_flow_stats_get+0x163/0x1b0 [openvswitch]
[ 3014.524118] [<ffffffffa0218ce5>] ovs_flow_cmd_fill_info+0x165/0x2b0 [openvswitch]
[ 3014.524118] [<ffffffffa021904c>] ovs_flow_cmd_build_info.constprop.27+0x6c/0xa0 [openvswitch]
[ 3014.524118] [<ffffffffa0219519>] ovs_flow_cmd_new_or_set+0x499/0x4f0 [openvswitch]
[ 3014.524118] [<ffffffff8161bf09>] genl_family_rcv_msg+0x199/0x390
[ 3014.524118] [<ffffffff8161c18e>] genl_rcv_msg+0x8e/0xd0
[ 3014.524118] [<ffffffff8161a189>] netlink_rcv_skb+0xa9/0xc0
[ 3014.524118] [<ffffffff8161a6a8>] genl_rcv+0x28/0x40
[ 3014.524118] [<ffffffff816197d0>] netlink_unicast+0xf0/0x1b0
[ 3014.524118] [<ffffffff81619b8f>] netlink_sendmsg+0x2ff/0x740
[ 3014.524118] [<ffffffff815ce86b>] sock_sendmsg+0x8b/0xc0
[ 3014.524118] [<ffffffff815cf369>] ___sys_sendmsg+0x389/0x3a0
[ 3014.524118] [<ffffffff815cfa62>] __sys_sendmsg+0x42/0x80
[ 3014.524118] [<ffffffff815cfab2>] SyS_sendmsg+0x12/0x20
[ 3014.524118] [<ffffffff8170f829>] system_call_fastpath+0x16/0x1b
[ 3014.524118] irq event stamp: 6163288
[ 3014.524118] hardirqs last enabled at (6163288): [<ffffffff8169f8a0>] ip6_finish_output2+0x380/0x670
[ 3014.524118] hardirqs last disabled at (6163287): [<ffffffff8169f85b>] ip6_finish_output2+0x33b/0x670
[ 3014.524118] softirqs last enabled at (6163266): [<ffffffff810888a2>] _local_bh_enable+0x22/0x50
[ 3014.524118] softirqs last disabled at (6163267): [<ffffffff81089f15>] irq_exit+0xc5/0xd0
[ 3014.524118]
other info that might help us debug this:
[ 3014.524118] Possible unsafe locking scenario:
[ 3014.524118] CPU0
[ 3014.524118] ----
[ 3014.524118] lock(&(&flow->stats.stat->lock)->rlock);
[ 3014.524118] <Interrupt>
[ 3014.524118] lock(&(&flow->stats.stat->lock)->rlock);
[ 3014.524118]
*** DEADLOCK ***
[ 3014.524118] 5 locks held by swapper/1/0:
[ 3014.524118] #0: (((&idev->mc_ifc_timer))){+.-...}, at: [<ffffffff81090095>] call_timer_fn+0x5/0x180
[ 3014.524118] #1: (rcu_read_lock){.+.+..}, at: [<ffffffff816c5755>] mld_sendpack+0x5/0x330
[ 3014.524118] #2: (rcu_read_lock_bh){.+....}, at: [<ffffffff8169f579>] ip6_finish_output2+0x59/0x670
[ 3014.524118] #3: (rcu_read_lock_bh){.+....}, at: [<ffffffff815eb585>] __dev_queue_xmit+0x5/0x6a0
[ 3014.524118] #4: (rcu_read_lock){.+.+..}, at: [<ffffffffa0221995>] internal_dev_xmit+0x5/0x90 [openvswitch]
[ 3014.524118]
stack backtrace:
[ 3014.524118] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.14.0-rc2+ #1
[ 3014.524118] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 3014.524118] ffffffff82338940 ffff8800bb2838f0 ffffffff816fd1ac ffff8800b6b33570
[ 3014.524118] ffff8800bb283940 ffffffff816f9144 0000000000000005 ffff880000000001
[ 3014.524118] ffff880000000000 0000000000000006 ffff8800b6b33570 ffffffff810d4380
[ 3014.524118] Call Trace:
[ 3014.524118] <IRQ> [<ffffffff816fd1ac>] dump_stack+0x4d/0x66
[ 3014.524118] [<ffffffff816f9144>] print_usage_bug+0x1f3/0x204
[ 3014.524118] [<ffffffff810d4380>] ? check_usage_backwards+0x130/0x130
[ 3014.524118] [<ffffffff810d4da0>] mark_lock+0x270/0x300
[ 3014.524118] [<ffffffff810d6718>] __lock_acquire+0x478/0x1c30
[ 3014.524118] [<ffffffff810d4ede>] ? mark_held_locks+0xae/0x130
[ 3014.524118] [<ffffffffa0220047>] ? find_bucket.isra.1+0x67/0x70 [openvswitch]
[ 3014.524118] [<ffffffffa0220228>] ? masked_flow_lookup+0x78/0x140 [openvswitch]
[ 3014.524118] [<ffffffff810d669a>] ? __lock_acquire+0x3fa/0x1c30
[ 3014.524118] [<ffffffff810d7f80>] lock_acquire+0xb0/0x150
[ 3014.524118] [<ffffffffa021b6ef>] ? ovs_flow_stats_update+0x4f/0xd0 [openvswitch]
[ 3014.524118] [<ffffffff81705c99>] _raw_spin_lock+0x39/0x50
[ 3014.524118] [<ffffffffa021b6ef>] ? ovs_flow_stats_update+0x4f/0xd0 [openvswitch]
[ 3014.524118] [<ffffffffa021b6ef>] ovs_flow_stats_update+0x4f/0xd0 [openvswitch]
[ 3014.524118] [<ffffffffa021ac75>] ovs_dp_process_received_packet+0x75/0x100 [openvswitch]
[ 3014.524118] [<ffffffffa02213ea>] ovs_vport_receive+0x2a/0x30 [openvswitch]
[ 3014.524118] [<ffffffffa02219e6>] internal_dev_xmit+0x56/0x90 [openvswitch]
[ 3014.524118] [<ffffffffa0221995>] ? internal_dev_xmit+0x5/0x90 [openvswitch]
[ 3014.524118] [<ffffffff815eb276>] dev_hard_start_xmit+0x316/0x620
[ 3014.524118] [<ffffffff815eb8ff>] __dev_queue_xmit+0x37f/0x6a0
[ 3014.524118] [<ffffffff815eb585>] ? __dev_queue_xmit+0x5/0x6a0
[ 3014.524118] [<ffffffff8169f8a0>] ? ip6_finish_output2+0x380/0x670
[ 3014.524118] [<ffffffff815ebc30>] dev_queue_xmit+0x10/0x20
[ 3014.524118] [<ffffffff8169f900>] ip6_finish_output2+0x3e0/0x670
[ 3014.524118] [<ffffffff816a31aa>] ? ip6_finish_output+0x9a/0x200
[ 3014.524118] [<ffffffff816a31aa>] ip6_finish_output+0x9a/0x200
[ 3014.524118] [<ffffffff816a335f>] ip6_output+0x4f/0x1d0
[ 3014.524118] [<ffffffff816c58e6>] mld_sendpack+0x196/0x330
[ 3014.524118] [<ffffffff816c5755>] ? mld_sendpack+0x5/0x330
[ 3014.524118] [<ffffffff816c63cd>] mld_ifc_timer_expire+0x19d/0x2d0
[ 3014.524118] [<ffffffff816c6230>] ? mld_dad_timer_expire+0x70/0x70
[ 3014.524118] [<ffffffff8109010a>] call_timer_fn+0x7a/0x180
[ 3014.524118] [<ffffffff81090095>] ? call_timer_fn+0x5/0x180
[ 3014.524118] [<ffffffff816c6230>] ? mld_dad_timer_expire+0x70/0x70
[ 3014.524118] [<ffffffff8109054c>] run_timer_softirq+0x20c/0x2c0
[ 3014.524118] [<ffffffff810899fd>] __do_softirq+0x12d/0x310
[ 3014.524118] [<ffffffff81089f15>] irq_exit+0xc5/0xd0
[ 3014.524118] [<ffffffff81711b75>] smp_apic_timer_interrupt+0x45/0x60
[ 3014.524118] [<ffffffff817104b2>] apic_timer_interrupt+0x72/0x80
[ 3014.524118] <EOI> [<ffffffff81052486>] ? native_safe_halt+0x6/0x10
[ 3014.524118] [<ffffffff8101ec04>] default_idle+0x24/0xe0
[ 3014.524118] [<ffffffff8101f54e>] arch_cpu_idle+0x2e/0x40
[ 3014.524118] [<ffffffff810e96ae>] cpu_startup_entry+0x9e/0x280
[ 3014.524118] [<ffffffff81044929>] start_secondary+0x1d9/0x280
^ permalink raw reply
* Re: [PATCH net-next v2 07/10] net: bcmgenet: add MDIO routines
From: Florian Fainelli @ 2014-02-13 17:00 UTC (permalink / raw)
To: Mark Rutland
Cc: netdev@vger.kernel.org, davem@davemloft.net, cernekee@gmail.com,
devicetree@vger.kernel.org
In-Reply-To: <20140213115008.GB5871@e106331-lin.cambridge.arm.com>
Hi Mark,
2014-02-13 3:50 GMT-08:00 Mark Rutland <mark.rutland@arm.com>:
> On Thu, Feb 13, 2014 at 05:29:52AM +0000, Florian Fainelli wrote:
>> This patch adds support for configuring the port multiplexer hardware
>> which resides in front of the GENET Ethernet MAC controller. This allows
>> us to support:
>>
>> - internal PHYs (using drivers/net/phy/bcm7xxx.c)
>> - MoCA PHYs which are an entirely separate hardware block not covered
>> here
>> - external PHYs and switches
>>
>> Note that MoCA and switches are currently supported using the emulated
>> "fixed PHY" driver.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> Changes since v1:
>> - fixed MDIO crash/warning when Device Tree probing fails
>> - removed the use of priv->phy_type and use priv->phy_interface
>> directly
>>
>> drivers/net/ethernet/broadcom/genet/bcmmii.c | 481 +++++++++++++++++++++++++++
>> 1 file changed, 481 insertions(+)
>> create mode 100644 drivers/net/ethernet/broadcom/genet/bcmmii.c
>
> [...]
>
>> +static int bcmgenet_mii_of_init(struct bcmgenet_priv *priv)
>> +{
>> + struct device_node *dn = priv->pdev->dev.of_node;
>> + struct device *kdev = &priv->pdev->dev;
>> + struct device_node *mdio_dn;
>> + const __be32 *fixed_link;
>
> This looks a bit odd. Could we not have a common parser for fixed-link
> properties?
Do you remember the fixed-link revamp that Thomas Petazzoni submitted
a while ago? I was planning on using it, but there were still some
disagreements on how to do it, so I ended up using the good old
"fixed-link" property as-is.
>
>> + u32 propval;
>> + int ret, sz;
>> +
>> + mdio_dn = of_get_next_child(dn, NULL);
>> + if (!mdio_dn) {
>> + dev_err(kdev, "unable to find MDIO bus node\n");
>> + return -ENODEV;
>> + }
>
> Could you please check that this is the node you expect (by looking at
> the compatible string list).
Makes sense.
>
>> +
>> + ret = of_mdiobus_register(priv->mii_bus, mdio_dn);
>> + if (ret) {
>> + dev_err(kdev, "failed to register MDIO bus\n");
>> + return ret;
>> + }
>> +
>> + /* Check if we have an internal or external PHY */
>> + priv->phy_dn = of_parse_phandle(dn, "phy-handle", 0);
>> + if (priv->phy_dn) {
>> + if (!of_property_read_u32(priv->phy_dn, "max-speed", &propval))
>> + priv->phy_speed = propval;
>
> Is there no way to find this out without reading values directly off of
> the PHY? It seems like something we should have an abstraction for.
In fact, this is a remnant from when I did not submit the patch doing
this in of_mdiobus_register(), so this is now useless.
>
>> + } else {
>> + /* Read the link speed from the fixed-link property */
>> + fixed_link = of_get_property(dn, "fixed-link", &sz);
>> + if (!fixed_link || sz < sizeof(*fixed_link)) {
>> + ret = -ENODEV;
>> + goto out;
>> + }
>> +
>> + priv->phy_speed = be32_to_cpu(fixed_link[2]);
>
> Similarly can we not have a common fixed-link parser? Or abstraction
> such that you query the phy regardless of what it is and how its binding
> represents this?
I do not think I need this line anymore. I will do some more testing
and will remove this parsing. I do agree that we need a common
fixed-link parser, but that is part of the discussion initiated by
Thomas Petazzoni.
Thanks for the review!
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v2 09/10] Documentation: add Device tree bindings for Broadcom GENET
From: Florian Fainelli @ 2014-02-13 16:57 UTC (permalink / raw)
To: Mark Rutland
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20140213111328.GB30705-NuALmloUBlrZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>
Hi Mark,
2014-02-13 3:13 GMT-08:00 Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>:
> On Thu, Feb 13, 2014 at 05:29:54AM +0000, Florian Fainelli wrote:
>> This patch adds the Device Tree bindings for the Broadcom GENET Gigabit
>> Ethernet controller. A bunch of examples are provided to illustrate the
>> versatile aspect of the hardare.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> Changes since v1:
>> - rebased
>>
>> .../devicetree/bindings/net/broadcom-bcmgenet.txt | 111 +++++++++++++++++++++
>> 1 file changed, 111 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
>> new file mode 100644
>> index 0000000..93c58e9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
>> @@ -0,0 +1,111 @@
>> +* Broadcom BCM7xxx Ethernet Controller (GENET)
>> +
>> +Required properties:
>> +- compatible: should be "brcm,genet-v1", "brcm,genet-v2", "brcm,genet-v3",
>> + "brcm,genet-v4".
>
> Presumably "should contain one of" is a better description than "should
> be"?
>
> Are the newer revisions compatible with older revisions?
Not entirely, the driver has internal macros: GENET_IS_V<N>() to help
figuring out which parts are different.
>
>> +- reg: address and length of the register set for the device.
>> +- interrupts: interrupt for the device
>> +- mdio bus node: this node should always be present regarless of the PHY
>> + configuration of the GENET instance
>
> Nit: a node is not a property, list it after properties.
>
>> +- phy-mode: The interface between the SoC and the PHY (a string that
>> + of_get_phy_mode() can understand).
>
> Do we not have a document under bindings listing these? I really don't
> like referring to code in bindings docs.
Sergei has been working on a patch that centralizes the Ethernet DT
binding, but I am targetting the net-next/master tree in which this is
not yet present. I could probably pro-actively mention it though.
>
>> +
>> +MDIO bus node required properties:
>> +
>> +- compatible: should be "brcm,genet-v<N>-mdio"
>
> Where N is? Could this not be an explicit list as above? It helps when
> searching for bindings.
Yes, this should match the genet-v<N> compatible string.
>
>> +- reg: address and length relative to the parent node base register address
>
> The parent node will require #address-cells and #size-cells too then.
Correct.
>
>> +- address-cells: address cell for MDIO bus addressing, should be 1
>> +- size-cells: size of the cells for MDIO bus addressing, should be 0
>> +
>> +Optional properties:
>> +- phy-handle: A phandle to a phy node defining the PHY address (as the reg
>> + property, a single integer), used to describe configurations where a PHY
>> + (internal or external) is used.
>
> Is there not a phy binding you could refer to instead?
This is ePAPR, but once again, Sergei's document centralizes that nicely.
>
>> +
>> +- fixed-link: When the GENET interface is connected to a MoCA hardware block
>> + or when operating in a RGMII to RGMII type of connection, or when the
>> + MDIO bus is voluntarily disabled, this property should be used to describe
>> + the "fixed link", the property is described as follows:
>> +
>> + fixed-link: <a b c d e> where a is emulated phy id - choose any,
>> + but unique to the all specified fixed-links, b is duplex - 0 half,
>> + 1 full, c is link speed - d#10/d#100/d#1000, d is pause - 0 no
>> + pause, 1 pause, e is asym_pause - 0 no asym_pause, 1 asym_pause.
>
> Is this not documented elsewhere such that it can be referred to?
The Freescale FEC driver is the one holding most of the documentation
for fixed-link, I can refer to it until Sergei's patch which
centralizes the Ethernet DT bindings gets merged.
>
>> +
>> +Internal Gigabit PHY example:
>> +
>> +ethernet@f0b60000 {
>> + phy-mode = "internal";
>> + phy-handle = <&phy1>;
>> + mac-address = [ 00 10 18 36 23 1a ];
>> + compatible = "brcm,genet-v4";
>> + #address-cells = <0x1>;
>> + #size-cells = <0x1>;
>> + device_type = "ethernet";
>
> What's this needed by? I can't see any other devices with this
> device_type, and I was under the impression that we didn't want new
> device_type properties cropping up.
This is just an oversight, and is not required per-se, I will get it removed.
>
>> + reg = <0xf0b60000 0xfc4c>;
>> + interrupts = <0x0 0x14 0x0 0x0 0x15 0x0>;
>
> How many? The binding implied only one, and I'm not away of any
> interrupt controller bindings with #interrupt-cells = <6>.
In fact, only two interrupts, cells, this is a bad copy-pasting from
the bootloader providing the DTB.
Thanks for the review!
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH v2] usbnet: remove generic hard_header_len check
From: David Laight @ 2014-02-13 16:56 UTC (permalink / raw)
To: 'Emil Goode', Steve Glendinning, Oliver Neukum,
Bjørn Mork, David S. Miller, Freddy Xin, Eric Dumazet,
Ming Lei, Paul Gortmaker, Jeff Kirsher, Liu Junliang,
Octavian Purdila
Cc: linux-usb@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1392310219-5719-1-git-send-email-emilgoode@gmail.com>
From: Of Emil Goode
> This patch removes a generic hard_header_len check from the usbnet
> module that is causing dropped packages under certain circumstances
> for devices that send rx packets that cross urb boundaries.
>
> One example is the AX88772B which occasionally send rx packets that
> cross urb boundaries where the remaining partial packet is sent with
> no hardware header. When the buffer with a partial packet is of less
> number of octets than the value of hard_header_len the buffer is
> discarded by the usbnet module.
...
> diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
> index d6f64da..955df81 100644
> --- a/drivers/net/usb/ax88179_178a.c
> +++ b/drivers/net/usb/ax88179_178a.c
> @@ -1118,6 +1118,10 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> u16 hdr_off;
> u32 *pkt_hdr;
>
> + /* This check is no longer done by usbnet */
> + if (skb->len < dev->net->hard_header_len)
> + return 0;
> +
The ax88179 driver can also receive ethernet frames that cross the
end of rx URB.
It should have the code to save the last fragment (of a urb) until
the next data arrives, and then correctly merge the fragments.
It is likely that any sub-driver that sets the receive urb length
to a multiple of 1k (rather than leaving it at hard_hdr+mtu) can defrag
rx data that crosses urb boundaries.
David
^ permalink raw reply
* [PATCH v2] usbnet: remove generic hard_header_len check
From: Emil Goode @ 2014-02-13 16:50 UTC (permalink / raw)
To: Steve Glendinning, Oliver Neukum, Bjørn Mork,
David S. Miller, Freddy Xin, Eric Dumazet, Ming Lei,
Paul Gortmaker, Jeff Kirsher, Liu Junliang, Octavian Purdila
Cc: linux-usb, netdev, linux-kernel, Emil Goode
This patch removes a generic hard_header_len check from the usbnet
module that is causing dropped packages under certain circumstances
for devices that send rx packets that cross urb boundaries.
One example is the AX88772B which occasionally send rx packets that
cross urb boundaries where the remaining partial packet is sent with
no hardware header. When the buffer with a partial packet is of less
number of octets than the value of hard_header_len the buffer is
discarded by the usbnet module.
With AX88772B this can be reproduced by using ping with a packet
size between 1965-1976.
The bug has been reported here:
https://bugzilla.kernel.org/show_bug.cgi?id=29082
This patch introduces the following changes:
- Removes the generic hard_header_len check in the rx_complete
function in the usbnet module.
- Introduces a ETH_HLEN check for skbs that are not cloned from
within a rx_fixup callback.
- For safety a hard_header_len check is added to each rx_fixup
callback function that could be affected by this change.
These extra checks could possibly be removed by someone
who has the hardware to test.
- Removes a call to dev_kfree_skb_any() and instead utilizes the
dev->done list to queue skbs for cleanup.
The changes place full responsibility on the rx_fixup callback
functions that clone skbs to only pass valid skbs to the
usbnet_skb_return function.
Signed-off-by: Emil Goode <emilgoode@gmail.com>
Reported-by: Igor Gnatenko <i.gnatenko.brain@gmail.com>
---
v2: Added a comment in the changelog about the removal of a
call to dev_kfree_skb_any().
drivers/net/usb/ax88179_178a.c | 4 ++++
drivers/net/usb/gl620a.c | 4 ++++
drivers/net/usb/mcs7830.c | 5 +++--
drivers/net/usb/net1080.c | 4 ++++
drivers/net/usb/qmi_wwan.c | 8 ++++----
drivers/net/usb/rndis_host.c | 4 ++++
drivers/net/usb/smsc75xx.c | 4 ++++
drivers/net/usb/smsc95xx.c | 4 ++++
drivers/net/usb/sr9800.c | 4 ++++
drivers/net/usb/usbnet.c | 25 ++++++++++---------------
10 files changed, 45 insertions(+), 21 deletions(-)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index d6f64da..955df81 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1118,6 +1118,10 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
u16 hdr_off;
u32 *pkt_hdr;
+ /* This check is no longer done by usbnet */
+ if (skb->len < dev->net->hard_header_len)
+ return 0;
+
skb_trim(skb, skb->len - 4);
memcpy(&rx_hdr, skb_tail_pointer(skb), 4);
le32_to_cpus(&rx_hdr);
diff --git a/drivers/net/usb/gl620a.c b/drivers/net/usb/gl620a.c
index e4a8a93..1cc24e6 100644
--- a/drivers/net/usb/gl620a.c
+++ b/drivers/net/usb/gl620a.c
@@ -84,6 +84,10 @@ static int genelink_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
u32 size;
u32 count;
+ /* This check is no longer done by usbnet */
+ if (skb->len < dev->net->hard_header_len)
+ return 0;
+
header = (struct gl_header *) skb->data;
// get the packet count of the received skb
diff --git a/drivers/net/usb/mcs7830.c b/drivers/net/usb/mcs7830.c
index a305a7b..82d844a 100644
--- a/drivers/net/usb/mcs7830.c
+++ b/drivers/net/usb/mcs7830.c
@@ -526,8 +526,9 @@ static int mcs7830_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
{
u8 status;
- if (skb->len == 0) {
- dev_err(&dev->udev->dev, "unexpected empty rx frame\n");
+ /* This check is no longer done by usbnet */
+ if (skb->len < dev->net->hard_header_len) {
+ dev_err(&dev->udev->dev, "unexpected tiny rx frame\n");
return 0;
}
diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
index 0a85d92..4cbdb13 100644
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c
@@ -364,6 +364,10 @@ static int net1080_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
struct nc_trailer *trailer;
u16 hdr_len, packet_len;
+ /* This check is no longer done by usbnet */
+ if (skb->len < dev->net->hard_header_len)
+ return 0;
+
if (!(skb->len & 0x01)) {
netdev_dbg(dev->net, "rx framesize %d range %d..%d mtu %d\n",
skb->len, dev->net->hard_header_len, dev->hard_mtu,
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index ff5c871..b2e2aee 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -80,10 +80,10 @@ static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
{
__be16 proto;
- /* usbnet rx_complete guarantees that skb->len is at least
- * hard_header_len, so we can inspect the dest address without
- * checking skb->len
- */
+ /* This check is no longer done by usbnet */
+ if (skb->len < dev->net->hard_header_len)
+ return 0;
+
switch (skb->data[0] & 0xf0) {
case 0x40:
proto = htons(ETH_P_IP);
diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
index a48bc0f..524a47a 100644
--- a/drivers/net/usb/rndis_host.c
+++ b/drivers/net/usb/rndis_host.c
@@ -492,6 +492,10 @@ EXPORT_SYMBOL_GPL(rndis_unbind);
*/
int rndis_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
{
+ /* This check is no longer done by usbnet */
+ if (skb->len < dev->net->hard_header_len)
+ return 0;
+
/* peripheral may have batched packets to us... */
while (likely(skb->len)) {
struct rndis_data_hdr *hdr = (void *)skb->data;
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index f17b9e0..d9e7892 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -2106,6 +2106,10 @@ static void smsc75xx_rx_csum_offload(struct usbnet *dev, struct sk_buff *skb,
static int smsc75xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
{
+ /* This check is no longer done by usbnet */
+ if (skb->len < dev->net->hard_header_len)
+ return 0;
+
while (skb->len > 0) {
u32 rx_cmd_a, rx_cmd_b, align_count, size;
struct sk_buff *ax_skb;
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 8dd54a0..424db65e 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1723,6 +1723,10 @@ static void smsc95xx_rx_csum_offload(struct sk_buff *skb)
static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
{
+ /* This check is no longer done by usbnet */
+ if (skb->len < dev->net->hard_header_len)
+ return 0;
+
while (skb->len > 0) {
u32 header, align_count;
struct sk_buff *ax_skb;
diff --git a/drivers/net/usb/sr9800.c b/drivers/net/usb/sr9800.c
index 4175eb9..575be80 100644
--- a/drivers/net/usb/sr9800.c
+++ b/drivers/net/usb/sr9800.c
@@ -63,6 +63,10 @@ static int sr_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
{
int offset = 0;
+ /* This check is no longer done by usbnet */
+ if (skb->len < dev->net->hard_header_len)
+ return 0;
+
while (offset + sizeof(u32) < skb->len) {
struct sk_buff *sr_skb;
u16 size;
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 4671da7..dd10d58 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -542,17 +542,19 @@ static inline void rx_process (struct usbnet *dev, struct sk_buff *skb)
}
// else network stack removes extra byte if we forced a short packet
- if (skb->len) {
- /* all data was already cloned from skb inside the driver */
- if (dev->driver_info->flags & FLAG_MULTI_PACKET)
- dev_kfree_skb_any(skb);
- else
- usbnet_skb_return(dev, skb);
+ /* all data was already cloned from skb inside the driver */
+ if (dev->driver_info->flags & FLAG_MULTI_PACKET)
+ goto done;
+
+ if (skb->len < ETH_HLEN) {
+ dev->net->stats.rx_errors++;
+ dev->net->stats.rx_length_errors++;
+ netif_dbg(dev, rx_err, dev->net, "rx length %d\n", skb->len);
+ } else {
+ usbnet_skb_return(dev, skb);
return;
}
- netif_dbg(dev, rx_err, dev->net, "drop\n");
- dev->net->stats.rx_errors++;
done:
skb_queue_tail(&dev->done, skb);
}
@@ -574,13 +576,6 @@ static void rx_complete (struct urb *urb)
switch (urb_status) {
/* success */
case 0:
- if (skb->len < dev->net->hard_header_len) {
- state = rx_cleanup;
- dev->net->stats.rx_errors++;
- dev->net->stats.rx_length_errors++;
- netif_dbg(dev, rx_err, dev->net,
- "rx length %d\n", skb->len);
- }
break;
/* stalls need manual reset. this is rare ... except that
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 v2] iplink_bond: fix parameter value matching
From: Michal Kubecek @ 2014-02-13 16:31 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Ding Tianhong
In-Reply-To: <52F448AF.3060604@huawei.com>
Lookup function get_index() compares argument with table entries
only up to the length of the table entry so that if an entry
with lower index is a substring of a later one, earlier entry is
used even if the argument is equal to the other. For example,
ip link set bond0 type bond xmit_hash_policy layer2+3
sets xmit_hash_policy to 0 (layer2) as this is found before
"layer2+3" can be checked.
Use strcmp() to compare whole strings instead.
v2: look for an exact match only
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
ip/iplink_bond.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index f22151e..7a950df 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -106,7 +106,7 @@ static int get_index(const char **tbl, char *name)
return i;
for (i = 0; tbl[i]; i++)
- if (strncmp(tbl[i], name, strlen(tbl[i])) == 0)
+ if (strcmp(tbl[i], name) == 0)
return i;
return -1;
--
1.8.1.4
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: remove useless if check from register_netdevice()
From: Alexei Starovoitov @ 2014-02-13 16:23 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: David S. Miller, netdev
In-Reply-To: <1392266829-2365-1-git-send-email-kda@linux-powerpc.org>
On Wed, Feb 12, 2014 at 8:47 PM, Denis Kirjanov <kda@linux-powerpc.org> wrote:
> remove useless if check from register_netdevice()
>
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
> ---
> v1 -> v2: Fixed identation
> ---
> net/core/dev.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 4ad1b78..21a72ad 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5876,8 +5876,7 @@ int register_netdevice(struct net_device *dev)
> if (dev->netdev_ops->ndo_init) {
> ret = dev->netdev_ops->ndo_init(dev);
> if (ret) {
> - if (ret > 0)
> - ret = -EIO;
> + ret = -EIO;
> goto out;
why is it a useless check?
seems perfectly valid to me.
most of the time ndo_init() returns negative error code like -ENOMEM
which we want
to propagate further down and want to override it to -EIO if it's > 0
^ permalink raw reply
* Re: [PATCH next resend] tcp: use zero-window when free_space is low
From: Eric Dumazet @ 2014-02-13 16:19 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev, Neal Cardwell, Yuchung Cheng
In-Reply-To: <20140213153441.GJ25153@breakpoint.cc>
On Thu, 2014-02-13 at 16:34 +0100, Florian Westphal wrote:
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Would you mind waiting we run our packetdrill tests before acknowledging
> > this patch, because I suspect this might have some impact ?
>
> Of course not. I am very happy that you folks have these kinds of tests
> and are willing to double-check. Take all time you need, there is no
> need to haste.
>
We have today 348 different packetdrill tests, and this number keeps
increasing ;)
> Many Thanks Eric.
>
> Do you think it makes sense to add counters for this?
>
> The caveat is that decision to send 0 window doesn't mean we end
> up sending one, since we cannot shrink already offered window.
>
> static u16 tcp_select_window(struct sock *sk)
> {
> struct tcp_sock *tp = tcp_sk(sk);
> u32 cur_win = tcp_receive_window(tp);
> u32 new_win = __tcp_select_window(sk);
>
> /* Never shrink the offered window */
> if (new_win < cur_win) {
>
> Would you add SNMP counter for '__tcp_select_window() wants 0 window'
> or for 'tcp_select_window() does pick 0 window' ?
>
> [ or even different counters for both ? ]
Ideally, having counters of transitions would be nice.
This would help us to spot regressions in TCP stacks or network drivers,
or wrong application tunings.
One counter tracking number of times a socket went from a non zero
window to a zero window (as you said, I am referring to effective window
being sent)
One counter tracking the reverse.
If it proves being too complex or expensive, a single counter tracking
"win 0" sent to the peers.
^ permalink raw reply
* FROM WORLD BANK/UNITED, READ THE ATTACHMENT!!!!
From: WORLD BANK/UNITED NATIONS @ 2014-02-13 16:13 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 48 bytes --]
FROM WORLD BANK/UNITED, READ THE ATTACHMENT!!!!
[-- Attachment #2: WORLD BANK UNITED NATIONS.doc --]
[-- Type: application/msword, Size: 28672 bytes --]
^ permalink raw reply
* Re: RFC: bridge get fdb by bridge device
From: John Fastabend @ 2014-02-13 16:03 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: vyasevic, netdev@vger.kernel.org, Stephen Hemminger,
Scott Feldman
In-Reply-To: <52FCE69F.5080602@mojatatu.com>
On 2/13/2014 7:37 AM, Jamal Hadi Salim wrote:
> On 02/12/14 13:50, John Fastabend wrote:
>> On 2/11/2014 1:04 PM, Jamal Hadi Salim wrote:
>>
>> Because it is not the same type of object as the software bridge.
>> Most notably it doesn't do learning.
>
> This kept nagging at me.
> Learning is optional for a bridge. So is flooding.
> I think we got this right in recent kernels.
>
> cheers,
> jamal
Yeah I remember now Vlad added them. The real distinction
is macvlan devices only have _one_ uplink port and support
other forwarding modes, VEPA, etc.
^ permalink raw reply
* Re: [PATCH net-next] net: remove unnecessary return's
From: Stephen Hemminger @ 2014-02-13 16:02 UTC (permalink / raw)
To: Joe Perches; +Cc: David Miller, netdev, Julia Lawall
In-Reply-To: <1392273125.2214.25.camel@joe-AO722>
On Wed, 12 Feb 2014 22:32:05 -0800
Joe Perches <joe@perches.com> wrote:
> On Wed, 2014-02-12 at 20:51 -0800, Stephen Hemminger wrote:
> > One of my pet coding style peeves is the practice of
> > adding extra return; at the end of function.
> > Kill several instances of this in network code.
> > I suppose some coccinelle wizardy could do this automatically.
>
> Maybe, but grep version 2.5.4 will show most of them.
>
> $ grep-2.5.4 -rP --include=*.[ch] "return;\n}" *
> [...]
>
> Fixing them has to make sure that there's no
> label before the close brace.
>
> gcc has to have a statement before the close brace
> of a void return after a label.
>
> label:
> }
>
> must be:
>
> label:
> ;
> }
>
> to compile.
>
My method was to use:
find . -name '*.c' | xargs grep -Pzo '(?s)^(\s*)\Nreturn;.}'
Then ignore cases where it was done for final label and where return
was alone in stub function.
^ 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