* Re: [PATCH net 6/7] qlge: refactoring of ethtool stats.
From: Ben Hutchings @ 2012-07-03 1:24 UTC (permalink / raw)
To: Jitendra Kalsaria; +Cc: davem, netdev, ron.mercer, Dept_NX_Linux_NIC_Driver
In-Reply-To: <1341272514-5156-7-git-send-email-jitendra.kalsaria@qlogic.com>
This is hardly an urgent fix...
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH net 6/7] qlge: refactoring of ethtool stats.
From: David Miller @ 2012-07-03 1:26 UTC (permalink / raw)
To: bhutchings
Cc: jitendra.kalsaria, netdev, ron.mercer, Dept_NX_Linux_NIC_Driver
In-Reply-To: <1341278648.2590.31.camel@bwh-desktop.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 3 Jul 2012 02:24:08 +0100
> This is hardly an urgent fix...
This entire series is suspect, and largely inappropriate for 'net'
this late in the -rc.
^ permalink raw reply
* RE: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.
From: Jitendra Kalsaria @ 2012-07-03 1:30 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Ron Mercer, Dept-NX Linux NIC Driver
In-Reply-To: <20120702.171854.1585295090835924398.davem@davemloft.net>
As per your comments, TX ring full is not expected behavior? All I can think of increasing the TX queue to 1024 and clean-up in timer instead of interrupt?
Jiten
-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Monday, July 02, 2012 5:19 PM
To: Jitendra Kalsaria
Cc: netdev; Ron Mercer; Dept-NX Linux NIC Driver
Subject: Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.
From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
Date: Mon, 2 Jul 2012 19:41:48 -0400
> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
>
> qlge driver was acting wrongly when considering TX ring full
> as a TX error. TX ring full is expected behavior when NIC is
> overwhelmed and is expected to happen, as far as packets are
> not lost.
>
> Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
If your driver is properly coded, this code path should never trigger,
ever. So it is an error, and you need to fix whatever bug exists in
your driver which allows this to happen, rather than this change
here which attempts to sweep the issue under the rug.
^ permalink raw reply
* Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.
From: David Miller @ 2012-07-03 1:38 UTC (permalink / raw)
To: jitendra.kalsaria; +Cc: netdev, ron.mercer, Dept_NX_Linux_NIC_Driver
In-Reply-To: <5E4F49720D0BAD499EE1F01232234BA877435B266A@AVEXMB1.qlogic.org>
From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
Date: Mon, 2 Jul 2012 18:30:47 -0700
> As per your comments, TX ring full is not expected behavior? All I
> can think of increasing the TX queue to 1024 and clean-up in timer
> instead of interrupt?
Your transmit function should never be invoked when the queue is
full, logic elsewhere in your driver should have stopped the queue
therefore preventing further invocations of your transmit function
until you wake the queue when space is liberated in the TX ring.
^ permalink raw reply
* Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.
From: David Miller @ 2012-07-03 1:41 UTC (permalink / raw)
To: jitendra.kalsaria; +Cc: netdev, ron.mercer, Dept_NX_Linux_NIC_Driver
In-Reply-To: <20120702.183826.1521103644475572622.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Mon, 02 Jul 2012 18:38:26 -0700 (PDT)
> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> Date: Mon, 2 Jul 2012 18:30:47 -0700
>
>> As per your comments, TX ring full is not expected behavior? All I
>> can think of increasing the TX queue to 1024 and clean-up in timer
>> instead of interrupt?
>
> Your transmit function should never be invoked when the queue is
> full, logic elsewhere in your driver should have stopped the queue
> therefore preventing further invocations of your transmit function
> until you wake the queue when space is liberated in the TX ring.
BTW, did it even occur to you that there is a kernel log message here
in this code path for a reason?
That log message is there because this event is unexpected and a
driver error.
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the wireless tree
From: Stephen Rothwell @ 2012-07-03 1:44 UTC (permalink / raw)
To: David Miller, netdev
Cc: linux-next, linux-kernel, Eliad Peller, John W. Linville,
Johannes Berg
[-- Attachment #1: Type: text/plain, Size: 1532 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in
net/mac80211/mlme.c between commit 0030bc586591 ("mac80211: destroy
assoc_data correctly if assoc fails") from the wireless tree and commit
bdcbd8e0e3ff ("mac80211: clean up debugging") from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc net/mac80211/mlme.c
index 0db5d34,e6fe84a..0000000
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@@ -2170,17 -2148,19 +2148,17 @@@ ieee80211_rx_mgmt_assoc_resp(struct iee
*bss = assoc_data->bss;
if (status_code != WLAN_STATUS_SUCCESS) {
- printk(KERN_DEBUG "%s: %pM denied association (code=%d)\n",
- sdata->name, mgmt->sa, status_code);
+ sdata_info(sdata, "%pM denied association (code=%d)\n",
+ mgmt->sa, status_code);
ieee80211_destroy_assoc_data(sdata, false);
} else {
- sdata_info(sdata, "associated\n");
-
if (!ieee80211_assoc_success(sdata, *bss, mgmt, len)) {
/* oops -- internal error -- send timeout for now */
- ieee80211_destroy_assoc_data(sdata, true);
- sta_info_destroy_addr(sdata, mgmt->bssid);
+ ieee80211_destroy_assoc_data(sdata, false);
cfg80211_put_bss(*bss);
return RX_MGMT_CFG80211_ASSOC_TIMEOUT;
}
- printk(KERN_DEBUG "%s: associated\n", sdata->name);
++ sdata_info(sdata, "associated\n");
/*
* destroy assoc_data afterwards, as otherwise an idle
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 09/10] net/mlx4_en: Manage flow steering rules with ethtool
From: Ben Hutchings @ 2012-07-03 1:47 UTC (permalink / raw)
To: Or Gerlitz
Cc: davem, roland, Yevgeny Petrilin, Oren Duer, netdev,
Hadar Hen Zion, Amir Vadai
In-Reply-To: <4FF19ACD.6040602@mellanox.com>
On Mon, 2012-07-02 at 15:57 +0300, Or Gerlitz wrote:
> On 7/1/2012 7:00 PM, Ben Hutchings wrote:
[...]
> >> + spec_l3->id = MLX4_NET_TRANS_RULE_ID_IPV4;
> >> + spec_l3->ipv4.src_ip = cmd->fs.h_u.usr_ip4_spec.ip4src;
> >> + if (spec_l3->ipv4.src_ip)
> >> + spec_l3->ipv4.src_ip_msk = EN_ETHTOOL_WORD_MASK;
> >> + spec_l3->ipv4.dst_ip = cmd->fs.h_u.usr_ip4_spec.ip4dst;
> >> + if (spec_l3->ipv4.dst_ip)
> >> + spec_l3->ipv4.dst_ip_msk = EN_ETHTOOL_WORD_MASK;
> >
> > The conditions should be using the mask (cmd->fs.m_u) not the value.
>
> I'd like to clarify things here a bit - the way the code is written, is to
>
> 1st make sure that we can deal with the mask provided by the user in the
> ethtool
> command, e.g its all zeroes or all ones (leaving a side for a minute the
> other
> discussion on how would be best to impl. that check...) - this check is
> done
> in mlx4_en_validate_flow
>
> 2nd initialize mlx4 flow spec which is all empty - see calls to kzalloc
> spec_l2/l3/l4
>
> 3rd import the non-zero values (not masks) from the ethtool command into
> the mlx4
> flow specs, with FULL mask
>
>
> Under this logic, we can use the values and not the masks, isn't that?
No, it's perfectly valid to specify a filter that matches, for example,
a destination IP address of 0.0.0.0 with mask of 255.255.255.255. So
you really need to check the mask. If your filter hardware doesn't
support zero values for some fields then you'll need to reject them in
mlx4_en_validate_flow.
[...]
> >> +static int mlx4_en_ethtool_to_net_trans_rule(struct net_device *dev,
> >> + struct ethtool_rxnfc *cmd,
> >> + struct list_head *rule_list_h)
> >> +{
> >> + int err;
> >> + u64 mac;
> >> + __be64 be_mac;
> >> + struct ethhdr *eth_spec;
> >> + struct mlx4_en_priv *priv = netdev_priv(dev);
> >> + struct mlx4_spec_list *spec_l2;
> >> + __be64 mac_msk = cpu_to_be64(EN_ETHTOOL_MAC_MASK << 16);
> >> +
> >> + err = mlx4_en_validate_flow(dev, cmd);
> >> + if (err)
> >> + return err;
> >> +
> >> + spec_l2 = kzalloc(sizeof *spec_l2, GFP_KERNEL);
> >> + if (!spec_l2)
> >> + return -ENOMEM;
> >> +
> >> + mac = priv->mac & EN_ETHTOOL_MAC_MASK;
> >> + be_mac = cpu_to_be64(mac << 16);
> >> +
> >> + spec_l2->id = MLX4_NET_TRANS_RULE_ID_ETH;
> >> + memcpy(spec_l2->eth.dst_mac_msk, &mac_msk, ETH_ALEN);
> >> + if ((cmd->fs.flow_type & ~FLOW_EXT) != ETHER_FLOW)
> >> + memcpy(spec_l2->eth.dst_mac, &be_mac, ETH_ALEN);
> >
> > Does the hardware require filtering by MAC address and not only by layer 3/4 addresses?
>
> YES
That's a pity. Maybe the API should be extended so the driver can at
least report that the filter is narrower than requested.
> >> + if ((cmd->fs.flow_type & FLOW_EXT) && cmd->fs.m_ext.vlan_tci) {
> >> + spec_l2->eth.vlan_id = cmd->fs.h_ext.vlan_tci;
> >> + spec_l2->eth.vlan_id_msk = cpu_to_be16(0xfff);
> >
> > This doesn't match mlx4_en_validate_flow(); you are replacing a mask of
> > 0xffff with 0xfff.
>
> I need to check how exactly this should be done here, vlan ID is only 12
> bits in size, so this is
> probably the source for the 0xfff vs 0xffff
[...]
If the hardware can only match the VID then you need to validate that
the mask is either 0 or 0xfff instead of 0 or 0xffff.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH net-next 09/10] net/mlx4_en: Manage flow steering rules with ethtool
From: Ben Hutchings @ 2012-07-03 1:50 UTC (permalink / raw)
To: Or Gerlitz
Cc: davem, roland, yevgenyp, oren, netdev, Hadar Hen Zion, Amir Vadai
In-Reply-To: <4FF1A2DF.5090403@mellanox.com>
On Mon, 2012-07-02 at 16:32 +0300, Or Gerlitz wrote:
[...]
> > Also, are the rules really prioritised by location, so that if rule 0
> > and 1 match a packet then only rule 0 is applied? If they are actually
> > prioritised by the match type then you need to assign locations on the
> > driver side that reflect the actual priorities.
>
>
> Rules are prioritized by a scheme made of "domain" X location, see below
> and in patch #6
> the MLX4_DOMAIN_yyy entries, higher domain value means lower priority,
> so for instance rules
> placed by ethtool would have higher priority over rules added by the L2
> NIC or by future RFS
> patch. Within a domain, the location matters.
>
> You can see that simple L2 rules (e.g contain dest-mac, possibly vlan)
> added by the driver
> use the NIC domain, wheres those added to serve ethtool commands use the
> ETHTOOL one.
>
> Within the ethtool domain, we maintain the priority as the location
> specified by the user, hope this explains
> things.
[...]
Good, I didn't read the other patches but just wanted to make sure you
had thought about this.
Ben
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: linux-next: manual merge of the net-next tree with the wireless tree
From: John W. Linville @ 2012-07-03 2:33 UTC (permalink / raw)
To: Stephen Rothwell
Cc: David Miller, netdev, linux-next, linux-kernel, Eliad Peller,
Johannes Berg
In-Reply-To: <20120703114443.26e03246debd0652e8d1e92b@canb.auug.org.au>
On Tue, Jul 03, 2012 at 11:44:43AM +1000, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the net-next tree got a conflict in
> net/mac80211/mlme.c between commit 0030bc586591 ("mac80211: destroy
> assoc_data correctly if assoc fails") from the wireless tree and commit
> bdcbd8e0e3ff ("mac80211: clean up debugging") from the net-next tree.
>
> I fixed it up (see below) and can carry the fix as necessary.
> --
> Cheers,
> Stephen Rothwell sfr@canb.auug.org.au
>
> diff --cc net/mac80211/mlme.c
> index 0db5d34,e6fe84a..0000000
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@@ -2170,17 -2148,19 +2148,17 @@@ ieee80211_rx_mgmt_assoc_resp(struct iee
> *bss = assoc_data->bss;
>
> if (status_code != WLAN_STATUS_SUCCESS) {
> - printk(KERN_DEBUG "%s: %pM denied association (code=%d)\n",
> - sdata->name, mgmt->sa, status_code);
> + sdata_info(sdata, "%pM denied association (code=%d)\n",
> + mgmt->sa, status_code);
> ieee80211_destroy_assoc_data(sdata, false);
> } else {
> - sdata_info(sdata, "associated\n");
> -
> if (!ieee80211_assoc_success(sdata, *bss, mgmt, len)) {
> /* oops -- internal error -- send timeout for now */
> - ieee80211_destroy_assoc_data(sdata, true);
> - sta_info_destroy_addr(sdata, mgmt->bssid);
> + ieee80211_destroy_assoc_data(sdata, false);
> cfg80211_put_bss(*bss);
> return RX_MGMT_CFG80211_ASSOC_TIMEOUT;
> }
> - printk(KERN_DEBUG "%s: associated\n", sdata->name);
> ++ sdata_info(sdata, "associated\n");
>
> /*
> * destroy assoc_data afterwards, as otherwise an idle
Looks good to me.
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* BISECTED: Re: REGRESSION: 3.4.0->3.5.0-rc2 kernel WARNING on cable plug on Acer Aspire One, no network
From: Alex Villacís Lasso @ 2012-07-03 2:33 UTC (permalink / raw)
To: Francois Romieu; +Cc: netdev
In-Reply-To: <4FF05597.3040206@palosanto.com>
El 01/07/12 08:50, Alex Villacís Lasso escribió:
> El 11/06/12 16:38, Francois Romieu escribió:
>> Alex Villacís Lasso <a_villacis@palosanto.com> :
>> [...]
>>> $ grep XID dmesg-3.5.0-rc2.txt
>>> [ 15.873858] r8169 0000:02:00.0: eth0: RTL8102e at 0xf7c0e000,
>>> 00:1e:68:e5:5d:b1, XID 04a00000 IRQ 44
>> The 8102e has not been touched by that many suspect patches but I do
>> not see where the problem is :o(
>>
>> Can you peel off the r8169 patches between 3.4.0 and 3.5-rc ?
>>
> Still present in 3.5-rc5. Bisection still in progress.
>
> --
> 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
My full bisection points to this commit:
commit 0a2b9a6ea93650b8a00f9fd5ee8fdd25671e2df6
Author: Marek Szyprowski <m.szyprowski@samsung.com>
Date: Thu Dec 29 13:09:51 2011 +0100
X86: integrate CMA with DMA-mapping subsystem
This patch adds support for CMA to dma-mapping subsystem for x86
architecture that uses common pci-dma/pci-nommu implementation. This
allows to test CMA on KVM/QEMU and a lot of common x86 boxes.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Michal Nazarewicz <mina86@mina86.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Is this commit somehow messing with the network card DMA?
My full bisection log follows:
git bisect start
# good: [76e10d158efb6d4516018846f60c2ab5501900bc] Linux 3.4
git bisect good 76e10d158efb6d4516018846f60c2ab5501900bc
# bad: [cfaf025112d3856637ff34a767ef785ef5cf2ca9] Linux 3.5-rc2
git bisect bad cfaf025112d3856637ff34a767ef785ef5cf2ca9
# good: [3813d4024a75562baf77d3907fb6afbf8f9c8232] Merge tag
'ia64-3.5-merge' of git://git.kernel.org/pub/scm/linux/kernel/git/aegl/linux
git bisect good 3813d4024a75562baf77d3907fb6afbf8f9c8232
# bad: [5723aa993d83803157c22327e90cd59e3dcbe879] x86: use the new
generic strnlen_user() function
git bisect bad 5723aa993d83803157c22327e90cd59e3dcbe879
# good: [654443e20dfc0617231f28a07c96a979ee1a0239] Merge branch
'perf-uprobes-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good 654443e20dfc0617231f28a07c96a979ee1a0239
# good: [8c914028f5ddaa417b7d0f4b7fdc24caceaa8043] Merge branch
'drm-nouveau-next' of
git://anongit.freedesktop.org/git/nouveau/linux-2.6 into drm-core-next
git bisect good 8c914028f5ddaa417b7d0f4b7fdc24caceaa8043
# good: [0708500d49e8439d9fe5529795bdc1485f0f46c3] Merge tag
'devicetree-for-linus' of git://git.secretlab.ca/git/linux-2.6
git bisect good 0708500d49e8439d9fe5529795bdc1485f0f46c3
# good: [07acfc2a9349a8ce45b236c2624dad452001966b] Merge branch 'next'
of git://git.kernel.org/pub/scm/virt/kvm/kvm
git bisect good 07acfc2a9349a8ce45b236c2624dad452001966b
# good: [58823de9d2f1265030d0d06cb03cc2a551994398] Merge tag
'hda-switcheroo' of
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
git bisect good 58823de9d2f1265030d0d06cb03cc2a551994398
# bad: [0f51596bd39a5c928307ffcffc9ba07f90f42a8b] Merge branch
'for-next-arm-dma' into for-linus
git bisect bad 0f51596bd39a5c928307ffcffc9ba07f90f42a8b
# bad: [0a2b9a6ea93650b8a00f9fd5ee8fdd25671e2df6] X86: integrate CMA
with DMA-mapping subsystem
git bisect bad 0a2b9a6ea93650b8a00f9fd5ee8fdd25671e2df6
# good: [6d4a49160de2c684fb59fa627bce80e200224331] mm: page_alloc:
change fallbacks array handling
git bisect good 6d4a49160de2c684fb59fa627bce80e200224331
# good: [cfd3da1e49bb95c355c01c0f502d657deb3d34a4] mm: Serialize access
to min_free_kbytes
git bisect good cfd3da1e49bb95c355c01c0f502d657deb3d34a4
# good: [49f223a9cd96c7293d7258ff88c2bdf83065f69c] mm: trigger page
reclaim in alloc_contig_range() to stabilise watermarks
git bisect good 49f223a9cd96c7293d7258ff88c2bdf83065f69c
# good: [c64be2bb1c6eb43c838b2c6d57b074078be208dd] drivers: add
Contiguous Memory Allocator
git bisect good c64be2bb1c6eb43c838b2c6d57b074078be208dd
^ permalink raw reply
* Re: Deleting an alias causes rest to get deleted
From: Stephen Hemminger @ 2012-07-03 2:44 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Volkan Yazıcı, netdev
In-Reply-To: <1341277937.2590.25.camel@bwh-desktop.uk.solarflarecom.com>
On Tue, 3 Jul 2012 02:12:17 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Mon, 2012-07-02 at 22:54 +0300, Volkan Yazıcı wrote:
> > Hi!
> >
> > I observe an IP aliasing anomaly that occurs when I try to delete an IP
> > alias from an interface. That is, when I delete the first address in a
> > set of IP aliased addresses assigned according to a particular subnet,
> > rest of the aliases get deleted as well. Check out the below snippet.
> [...]
> > As a side note, when I first asked this question to Stephen Hemminger
> > (he forwarded me to this mailing list) he also told me that "/In Linux
> > the interface aliases are really a legacy from the BSD style addressing,
> > and don't act the same. It is not common practice to use them./" Is that
> > really the case?
> [...]
>
> If you didn't give him the full details shown above, it's possible he
> thought you meant alias interfaces such as 'eth0:0'.
Yes, that is what I was assuming by the word 'alias'
^ permalink raw reply
* Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.
From: Krishna Kumar2 @ 2012-07-03 3:28 UTC (permalink / raw)
To: David Miller
Cc: Dept_NX_Linux_NIC_Driver, jitendra.kalsaria, netdev, ron.mercer
In-Reply-To: <20120702.184134.1131493483786674336.davem@davemloft.net>
> David Miller <davem@davemloft.net> 07/03/2012 07:11 AM
>
> >> As per your comments, TX ring full is not expected behavior? All I
> >> can think of increasing the TX queue to 1024 and clean-up in timer
> >> instead of interrupt?
> >
> > Your transmit function should never be invoked when the queue is
> > full, logic elsewhere in your driver should have stopped the queue
> > therefore preventing further invocations of your transmit function
> > until you wake the queue when space is liberated in the TX ring.
>
> BTW, did it even occur to you that there is a kernel log message here
> in this code path for a reason?
>
> That log message is there because this event is unexpected and a
> driver error.
Correctly coded drivers check for this condition:
"atomic_read(&tx_ring->tx_count) < 2" (without the
unlikely) once again at the end of the xmit and stop
the queue if required. Also, the message should be changed
to show that reaching here is an error.
thanks,
- KK
^ permalink raw reply
* Hello Dear
From: grace_falli2674 @ 2012-07-03 3:42 UTC (permalink / raw)
Hello Dear
My Name is Mavis
^ permalink raw reply
* Re: [PATCH v6] bonding support for IPv6 transmit hashing
From: John Eaglesham @ 2012-07-03 5:01 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev
In-Reply-To: <18390.1341272010@death.nxdomain>
On 7/2/2012 4:33 PM, Jay Vosburgh wrote:
>> +
>> + (((hash >> 24) XOR (hash >> 16) XOR (hash >> 8) XOR hash)
>> + (source MAC XOR destination MAC))
>> + modulo slave count
>
> This seems to be missing an XOR, between the end of "XOR hash)"
> and the start of "(source MAC".
>
You're correct.
>> if (skb->protocol == htons(ETH_P_IP)) {
>> + iph = ip_hdr(skb);
>> if (!ip_is_fragment(iph) &&
>> - (iph->protocol == IPPROTO_TCP ||
>> - iph->protocol == IPPROTO_UDP)) {
>> + (iph->protocol == IPPROTO_TCP ||
>> + iph->protocol == IPPROTO_UDP)) {
>
> Why did these two lines change?
>
I replaced the mixed tabs and spaces with all tabs when I updated that
function, but in retrospect the tabs and spaces were likely intentional.
I will revert.
>> + layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
>> + if (iph->ihl * sizeof(u32) + sizeof(__be16) * 2 >
>> + skb_headlen(skb) - skb_network_offset(skb))
>> + goto short_header;
>> layer4_xor = ntohs((*layer4hdr ^ *(layer4hdr + 1)));
>> + } else if (skb_network_header_len(skb) < sizeof(struct iphdr)) {
>> + goto short_header;
>> }
>> - return (layer4_xor ^
>> - ((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
>> -
>> + return (layer4_xor ^ ((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
>
> This line runs past 80 columns. There are a few more of these
> further down.
>
I will double-check this.
>> + } else if (skb->protocol == htons(ETH_P_IPV6)) {
>> + ipv6h = ipv6_hdr(skb);
>> + if (ipv6h->nexthdr == IPPROTO_TCP || ipv6h->nexthdr == IPPROTO_UDP) {
>> + layer4hdr = (__be16 *)((u8 *)ipv6h + sizeof(struct ipv6hdr));
>
> Could this be written as
>
> layer4hdr = (__be16 *)(ipv6h + 1);
>
> instead?
>
> -J
>
Yes, I can make that change. Thanks.
John
^ permalink raw reply
* Re: [PATCH v6] bonding support for IPv6 transmit hashing
From: David Miller @ 2012-07-03 5:14 UTC (permalink / raw)
To: linux; +Cc: fubar, netdev
In-Reply-To: <4FF27CA0.7030708@8192.net>
From: John Eaglesham <linux@8192.net>
Date: Mon, 02 Jul 2012 22:01:20 -0700
> I replaced the mixed tabs and spaces with all tabs when I updated that
> function, but in retrospect the tabs and spaces were likely
> intentional. I will revert.
They were. Don't change existing coding style unless you are certain
it is wrong.
Besides any such changes are completely outside of the scope of
the changes you are making, so you should have left them out in
any event.
^ permalink raw reply
* Awaiting upsteam? - Re: [PATCH net-next v3] em_canid: Ematch rule to match CAN frames according to their identifiers
From: Oliver Hartkopp @ 2012-07-03 5:15 UTC (permalink / raw)
To: David Miller; +Cc: Marc Kleine-Budde, netdev, linux-can, lartc
In-Reply-To: <4FF1E26F.5000106@hartkopp.net>
Hello Dave,
i've seen that you tagged this patch as "Awaiting upstream" in Patchwork.
Does this mean, that *you* are waiting for another re-spin of the patch OR do
you expect this patch go through a sub-tree like Marcs can-next tree?
Who is committing these "awaiting upstream" patches?
Thanks for clarification,
Oliver
^ permalink raw reply
* Re: Awaiting upsteam? - Re: [PATCH net-next v3] em_canid: Ematch rule to match CAN frames according to their identifiers
From: David Miller @ 2012-07-03 5:22 UTC (permalink / raw)
To: socketcan; +Cc: mkl, netdev, linux-can, lartc
In-Reply-To: <4FF28005.4050108@hartkopp.net>
From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: Tue, 03 Jul 2012 07:15:49 +0200
> Hello Dave,
>
> i've seen that you tagged this patch as "Awaiting upstream" in Patchwork.
>
> Does this mean, that *you* are waiting for another re-spin of the patch OR do
> you expect this patch go through a sub-tree like Marcs can-next tree?
It should go through the CAN tree.
^ permalink raw reply
* Re: [PATCH v6] bonding support for IPv6 transmit hashing
From: John Eaglesham @ 2012-07-03 5:38 UTC (permalink / raw)
To: David Miller; +Cc: fubar, netdev
In-Reply-To: <20120702.221425.418792617776598841.davem@davemloft.net>
On 7/2/2012 10:14 PM, David Miller wrote:
> From: John Eaglesham <linux@8192.net>
> Date: Mon, 02 Jul 2012 22:01:20 -0700
>
>> I replaced the mixed tabs and spaces with all tabs when I updated that
>> function, but in retrospect the tabs and spaces were likely
>> intentional. I will revert.
>
> They were. Don't change existing coding style unless you are certain
> it is wrong.
>
> Besides any such changes are completely outside of the scope of
> the changes you are making, so you should have left them out in
> any event.
> --
> 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
>
Should the indents in the code I am adding follow the style guide and
use only tabs, or follow the style already present and mix spaces and
tabs for indentation?
Thanks,
John
^ permalink raw reply
* Re: [PATCH v6] bonding support for IPv6 transmit hashing
From: David Miller @ 2012-07-03 5:43 UTC (permalink / raw)
To: linux; +Cc: fubar, netdev
In-Reply-To: <4FF2853D.9070707@8192.net>
From: John Eaglesham <linux@8192.net>
Date: Mon, 02 Jul 2012 22:38:05 -0700
> On 7/2/2012 10:14 PM, David Miller wrote:
>> From: John Eaglesham <linux@8192.net>
>> Date: Mon, 02 Jul 2012 22:01:20 -0700
>>
>>> I replaced the mixed tabs and spaces with all tabs when I updated that
>>> function, but in retrospect the tabs and spaces were likely
>>> intentional. I will revert.
>>
>> They were. Don't change existing coding style unless you are certain
>> it is wrong.
>>
>> Besides any such changes are completely outside of the scope of
>> the changes you are making, so you should have left them out in
>> any event.
>> --
>> 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
>>
>
> Should the indents in the code I am adding follow the style guide and
> use only tabs, or follow the style already present and mix spaces and
> tabs for indentation?
You should use a mix of tabs, as necessary, to get things to line up
how I told you they need to line up.
^ permalink raw reply
* RE: [PATCH net 4/7] qlge: Fixed double pci free upon tx_ring->q allocation failure.
From: Jitendra Kalsaria @ 2012-07-03 5:56 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Miller, netdev, Ron Mercer, Dept-NX Linux NIC Driver
In-Reply-To: <1341278521.2590.30.camel@bwh-desktop.uk.solarflarecom.com>
-----Original Message-----
From: Ben Hutchings [mailto:bhutchings@solarflare.com]
Sent: Monday, July 02, 2012 6:22 PM
To: Jitendra Kalsaria
Cc: David Miller; netdev; Ron Mercer; Dept-NX Linux NIC Driver
Subject: Re: [PATCH net 4/7] qlge: Fixed double pci free upon tx_ring->q allocation failure.
On Mon, 2012-07-02 at 19:41 -0400, Jitendra Kalsaria wrote:
> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
>
> Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> ---
> drivers/net/ethernet/qlogic/qlge/qlge_main.c | 14 +++++++-------
> 1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> index cdbc860..9ecd15f 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> @@ -2701,20 +2701,20 @@ static int ql_alloc_tx_resources(struct ql_adapter *qdev,
> pci_alloc_consistent(qdev->pdev, tx_ring->wq_size,
> &tx_ring->wq_base_dma);
>
> - if ((tx_ring->wq_base == NULL) ||
> - tx_ring->wq_base_dma & WQ_ADDR_ALIGN) {
> - netif_err(qdev, ifup, qdev->ndev, "tx_ring alloc failed.\n");
> - return -ENOMEM;
> - }
> + if (!tx_ring->wq_base || tx_ring->wq_base_dma & WQ_ADDR_ALIGN)
> + goto err;
> +
So in case pci_alloc_consistent() fails, you now try to free anyway.
Not sure whether that's safe; do you feel lucky?
[JK] Ahh, my apology.
> tx_ring->q =
> kmalloc(tx_ring->wq_len * sizeof(struct tx_ring_desc), GFP_KERNEL);
> - if (tx_ring->q == NULL)
> + if (!tx_ring->q)
> goto err;
Unrelated change.
> return 0;
> err:
> pci_free_consistent(qdev->pdev, tx_ring->wq_size,
> - tx_ring->wq_base, tx_ring->wq_base_dma);
> + tx_ring->wq_base, tx_ring->wq_base_dma);
This was nicely indented before...
> + tx_ring->wq_base = NULL;
> + netif_err(qdev, ifup, qdev->ndev, "tx_ring alloc failed.\n");
> return -ENOMEM;
> }
>
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: Deleting an alias causes rest to get deleted
From: Julian Anastasov @ 2012-07-03 6:34 UTC (permalink / raw)
To: Volkan Yazıcı; +Cc: netdev
In-Reply-To: <4FF1FC74.8080401@gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 4139 bytes --]
Hello,
On Mon, 2 Jul 2012, Volkan Yazıcı wrote:
> Hi!
>
> I observe an IP aliasing anomaly that occurs when I try to delete an IP alias
> from an interface. That is, when I delete the first address in a set of IP
> aliased addresses assigned according to a particular subnet, rest of the
> aliases get deleted as well. Check out the below snippet.
This is in Linux may be from the 2.3/2.4 times
>
> $ *for I in `seq 1 6`; do sudo ip addr add 192.168.2.$I/29 dev eth0;
> done*
> $ ip addr list
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> inet 127.0.0.1/8 scope host lo
> inet6 ::1/128 scope host
> valid_lft forever preferred_lft forever
> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast
> state UP qlen 1000
> link/ether 00:24:54:b9:1c:f8 brd ff:ff:ff:ff:ff:ff
> *inet 192.168.1.200/24 brd 192.168.1.255 scope global eth0**
> inet 192.168.2.1/29 scope global eth0
> inet 192.168.2.2/29 scope global secondary eth0
> inet 192.168.2.3/29 scope global secondary eth0
> inet 192.168.2.4/29 scope global secondary eth0
> inet 192.168.2.5/29 scope global secondary eth0
> inet 192.168.2.6/29 scope global secondary eth0*
> inet6 fe80::224:54ff:feb9:1cf8/64 scope link
> valid_lft forever preferred_lft forever
> 3: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
> link/ether e8:39:df:6a:21:2a brd ff:ff:ff:ff:ff:ff
> $ *sudo ip addr del 192.168.2.1/29 dev eth0*
> $ ip addr list
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> inet 127.0.0.1/8 scope host lo
> inet6 ::1/128 scope host
> valid_lft forever preferred_lft forever
> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast
> state UP qlen 1000
> link/ether 00:24:54:b9:1c:f8 brd ff:ff:ff:ff:ff:ff
> *inet 192.168.1.200/24 brd 192.168.1.255 scope global eth0*
> inet6 fe80::224:54ff:feb9:1cf8/64 scope link
> valid_lft forever preferred_lft forever
> 3: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
> link/ether e8:39:df:6a:21:2a brd ff:ff:ff:ff:ff:ff
>
> Per see, deleting 192.168.2.1/29 causes the rest of the aliased interfaces get
> deleted as well. This is something that is slightly documented in the ifconfig
> manual: /for every scope (i.e. same net with address/netmask combination) all
> aliases are deleted, if you delete the first (primary)/. So what is the right
> way of just deleting the first (primary) alias without affecting the rest? If
> this is a scoping issue, is it possible to assign each alias as primary within
> its own dedicated scope?
There is (yet) undocumented feature for the
interfaces:
/proc/sys/net/ipv4/conf/*/promote_secondaries
You set it for specific interface _or_ for "all".
It defaults to 0. When you enable it, deleting a primary
address will not delete all secondary addresses but will
change the next secondary as primary. The term alias may
refer for addresses on same interface while here the
problem is for a subset of addresses - from same subnet.
Primary is the first address added for the configured
subnet, all next addresses in subnet are added as
secondaries as shown by ip addr list.
> As a side note, when I first asked this question to Stephen Hemminger (he
> forwarded me to this mailing list) he also told me that "/In Linux the
> interface aliases are really a legacy from the BSD style addressing, and don't
> act the same. It is not common practice to use them./" Is that really the
> case? Because, as you know, IP aliasing is the heart of a majority of the
> high-availability and clustering solutions in Linux. Is IP aliasing a really
> deprecated technology in Linux? Should we avoid using it? If so, what do you
> recommend as an alternative?
Please do not stop using aliases!
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [RFC PATCH net-next] ipvs: add missing lock in ip_vs_ftp_init_conn()
From: Julian Anastasov @ 2012-07-03 7:12 UTC (permalink / raw)
To: Xiaotian Feng
Cc: netdev, lvs-devel, netfilter-devel, netfilter, coreteam,
linux-kernel, Xiaotian Feng, Wensong Zhang, Simon Horman,
Pablo Neira Ayuso, Patrick McHardy, David S. Miller
In-Reply-To: <1340890587-8169-1-git-send-email-xtfeng@gmail.com>
Hello,
On Thu, 28 Jun 2012, Xiaotian Feng wrote:
> We met a kernel panic in 2.6.32.43 kernel:
>
> [2680191.848044] IPVS: ip_vs_conn_hash(): request for already hashed, called from run_timer_softirq+0x175/0x1d0
> <snip>
> [2680311.849009] general protection fault: 0000 [#1] SMP
> [2680311.853001] RIP: 0010:[<ffffffff815f155c>] [<ffffffff815f155c>] ip_vs_conn_expire+0xdc/0x2f0
> [2680311.853001] RSP: 0018:ffff880028303e70 EFLAGS: 00010202
> [2680311.853001] RAX: dead000000200200 RBX: ffff8801aad00b80 RCX: 0000000000001d90
> [2680311.853001] RDX: dead000000100100 RSI: 000000004fd59800 RDI: ffff8801aad00c08
> <snip>
> [2680311.853001] Call Trace:
> [2680311.853001] <IRQ>
> [2680311.853001] [<ffffffff815f1480>] ? ip_vs_conn_expire+0x0/0x2f0
> [2680311.853001] [<ffffffff8104e2a5>] run_timer_softirq+0x175/0x1d0
> [2680311.853001] [<ffffffff81021a48>] ? lapic_next_event+0x18/0x20
> [2680311.853001] [<ffffffff81049a13>] __do_softirq+0xb3/0x150
> [2680311.853001] [<ffffffff8100cc5c>] call_softirq+0x1c/0x30
> [2680311.853001] [<ffffffff8100ea9a>] do_softirq+0x4a/0x80
> [2680311.853001] [<ffffffff81049957>] irq_exit+0x77/0x80
> [2680311.853001] [<ffffffff81021f2c>] smp_apic_timer_interrupt+0x6c/0xa0
> [2680311.853001] [<ffffffff8100c633>] apic_timer_interrupt+0x13/0x20
> [2680311.853001] <EOI>
> [2680311.853001] [<ffffffff81013b52>] ? mwait_idle+0x52/0x70
> [2680311.853001] [<ffffffff8100a7b0>] ? enter_idle+0x20/0x30
> [2680311.853001] [<ffffffff8100ac62>] ? cpu_idle+0x52/0x80
> [2680311.853001] [<ffffffff816d504d>] ? start_secondary+0x19d/0x280
>
> rax and rdx is LIST_POISON1 and LIST_POISON2, so kernel is list_del() on an already deleted
> connection and result the general protect fault.
>
> The "request for already hashed" warning, told us someone might change the connection flags
> incorrectly, like described in commit aea9d711, it changes the connection flags, but doesn't
> put the connection back to the list. So ip_vs_conn_hash() throw a warning and return.
> Later, when ip_vs_conn_expire fire again, ip_vs_conn_unhash() will find the HASHED connection
> and list_del() it, then kernel panic happened.
>
> After code review, the only chance that kernel change connection flag without protection is
> in ip_vs_ftp_init_conn().
>
> Signed-off-by: Xiaotian Feng <dannyfeng@tencent.com>
> Cc: Wensong Zhang <wensong@linux-vs.org>
> Cc: Simon Horman <horms@verge.net.au>
> Cc: Julian Anastasov <ja@ssi.bg>
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: "David S. Miller" <davem@davemloft.net>
For the fix below:
Acked-by: Julian Anastasov <ja@ssi.bg>
Simon, the change looks ok. ip_vs_ftp_init_conn is called
from context where cp->lock is not locked (no double lock), so it
should be safe for the backup.
Only that the comment is not specifying that we
fix a problem in the backup server.
> ---
> net/netfilter/ipvs/ip_vs_ftp.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> index b20b29c..c2bc264 100644
> --- a/net/netfilter/ipvs/ip_vs_ftp.c
> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> @@ -65,8 +65,10 @@ static int ip_vs_ftp_pasv;
> static int
> ip_vs_ftp_init_conn(struct ip_vs_app *app, struct ip_vs_conn *cp)
> {
> + spin_lock(&cp->lock);
> /* We use connection tracking for the command connection */
> cp->flags |= IP_VS_CONN_F_NFCT;
> + spin_unlock(&cp->lock);
> return 0;
> }
>
> --
> 1.7.1
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Urgent Assitance Needed
From: Anita George @ 2012-07-03 7:15 UTC (permalink / raw)
To: Recipients
[-- Attachment #1: Mail message body --]
[-- Type: text/plain, Size: 36 bytes --]
OPEN AND READ THE ATTACHMENT FILE..
[-- Attachment #2: George.rtf --]
[-- Type: application/octet-stream, Size: 3447 bytes --]
^ permalink raw reply
* AW: AW: AW: RFC: replace packets already in queue
From: Erdt, Ralph @ 2012-07-03 7:29 UTC (permalink / raw)
To: Eric Dumazet, Nicolas de Pesloüan; +Cc: Rick Jones, netdev@vger.kernel.org
In-Reply-To: <1341266168.22621.466.camel@edumazet-glaptop>
> > If I were you, I would use a tun/tap interface and manage a private
> > packet queue in userspace. This way, you wouldn't have to manage the
> overhead of porting your kernel code to every new kernel versions.
> >
>
> This seems a good idea.
>
> Then you can do other coalescing stuff, like TCP ACK that could be
> aggregated to single ACK as well.
Thanks for the idea. But this is option when just doing the replace thing.
But the charm of the qdisc solution is the complete integration to TC. It's complete compatible to the other options, so that you can create a bigger TC rule set. And creating the rules is a standard operation for the administrators - they know TC.
In terms of the other coalescing stuff (we didn't use TCP, because it's not possible) - it's already done in the device "driver" I mentioned. Yes, we can extend the "driver", but the qdisc solution has the benefit that there is a clear separation.
Nevertheless we will discuss the idea internally. Maybe the group got another idea based on this.
^ permalink raw reply
* Re: [PATCH net-next 09/10] net/mlx4_en: Manage flow steering rules with ethtool
From: Or Gerlitz @ 2012-07-03 8:14 UTC (permalink / raw)
To: Andreas Schwab
Cc: David Laight, Joe Perches, Ben Hutchings, davem, roland, yevgenyp,
oren, netdev, Hadar Hen Zion, Amir Vadai
In-Reply-To: <m28vf2o0oa.fsf@igel.home>
On 7/2/2012 2:35 PM, Andreas Schwab wrote:
>
> field == 0 || field == (typeof field)~(typeof field)0
> You can avoid that by using (typeof field)-1.
>
OK, thanks everybody, we will take that path.
Or.
^ 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