* [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
From: Rick Jones @ 2014-11-20 18:58 UTC (permalink / raw)
To: netdev; +Cc: davem
From: Rick Jones <rick.jones2@hp.com>
When a system is the passive accepter of many connections, for example
when the target of a netperf TCP_CC or TCP_CRR test, or as say a web
server, the discard of the skb containing the TCP SYN being processed
for the LISTEN endpoint should be a consume_skb() rather than a kfree_skb()
to avoid cluttering a dropped packet profile.
Signed-off-by: Rick Jones <rick.jones2@hp.com>
---
perf top -a -g -e skb:kfree_sk output from a system which is the target
of a netperf TCP_CC test
before:
- 100.00% 100.00% [kernel] [k] kfree_skb
- kfree_skb
+ 68.68% sk_stream_kill_queues
+ 31.32% tcp_rcv_state_process
after:
- 100.00% 100.00% [kernel] [k] kfree_skb
- kfree_skb
99.89% sk_stream_kill_queues
Presumably, the consume_skb() versus kfree_skb() could be made conditional
on there actually being data in the TCP SYN segment, but the odds of there
actually being data seemed low enough to not warrant it.
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index d91436b..999b0a4 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5624,8 +5624,12 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
* against this problem. So, we drop the data
* in the interest of security over speed unless
* it's still in use.
+ *
+ * 99 times out of 10, there won't actually be any
+ * data and so we aren't really dropping anything.
+ * So, we consume_skb() rather than kfree_skb().
*/
- kfree_skb(skb);
+ consume_skb(skb);
return 0;
}
goto discard;
^ permalink raw reply related
* RE: [PATCH 1/1] net: Hyper-V: Deletion of an unnecessary check before the function call "vfree"
From: Haiyang Zhang @ 2014-11-20 18:58 UTC (permalink / raw)
To: SF Markus Elfring, KY Srinivasan, devel@linuxdriverproject.org,
netdev@vger.kernel.org
Cc: LKML, kernel-janitors@vger.kernel.org, Julia Lawall
In-Reply-To: <546DF9D7.6080801@users.sourceforge.net>
> -----Original Message-----
> From: SF Markus Elfring [mailto:elfring@users.sourceforge.net]
> Sent: Thursday, November 20, 2014 9:25 AM
> To: Haiyang Zhang; KY Srinivasan; devel@linuxdriverproject.org;
> netdev@vger.kernel.org
> Cc: LKML; kernel-janitors@vger.kernel.org; Julia Lawall
> Subject: [PATCH 1/1] net: Hyper-V: Deletion of an unnecessary check
> before the function call "vfree"
>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 20 Nov 2014 15:15:21 +0100
>
> The vfree() function performs also input parameter validation. Thus the
> test
> around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Thanks!
^ permalink raw reply
* Re: [PATCHv10 ovs 12/15] datapath: Add support for unique flow identifiers.
From: Joe Stringer @ 2014-11-20 18:57 UTC (permalink / raw)
To: Pravin Shelar; +Cc: dev@openvswitch.org, netdev
In-Reply-To: <CALnjE+qwnKa-s1GoZRvh-RgjRO5eKvrf+XnvD_meGES1L=fe0w@mail.gmail.com>
On Wednesday, November 19, 2014 16:39:20 Pravin Shelar wrote:
> On Wed, Nov 19, 2014 at 4:20 PM, Joe Stringer <joestringer@nicira.com>
wrote:
> > On Wednesday, November 19, 2014 15:34:24 Pravin Shelar wrote:
> >> On Thu, Nov 13, 2014 at 11:17 AM, Joe Stringer <joestringer@nicira.com>
> >
> > wrote:
> >> > @@ -684,33 +691,43 @@ static size_t ovs_flow_cmd_msg_size(const struct
> >> > sw_flow_actions *acts)
> >> >
> >> > /* Called with ovs_mutex or RCU read lock. */
> >> > static int ovs_flow_cmd_fill_match(const struct sw_flow *flow,
> >> >
> >> > - struct sk_buff *skb)
> >> > + struct sk_buff *skb, u32
> >> > ufid_flags)
> >> >
> >> > {
> >> >
> >> > struct nlattr *nla;
> >> > int err;
> >> >
> >> > - /* Fill flow key. */
> >> > - nla = nla_nest_start(skb, OVS_FLOW_ATTR_KEY);
> >> > - if (!nla)
> >> > - return -EMSGSIZE;
> >> > -
> >> > - err = ovs_nla_put_flow(&flow->unmasked_key,
> >> > &flow->unmasked_key, skb, - false);
> >> > - if (err)
> >> > - return err;
> >> > -
> >> > - nla_nest_end(skb, nla);
> >> > + /* Fill flow key. If userspace didn't specify a UFID, then
> >> > ignore the + * OMIT_KEY flag. */
> >> > + if (!(ufid_flags & OVS_UFID_F_OMIT_KEY) ||
> >> > + !flow->index_by_ufid) {
> >>
> >> I am not sure about this check, userspace needs to send atleast ufid
> >> or the unmasked key as id for flow. otherwise we shld flag error. Here
> >> we can serialize flow->key.
> >> There could be another function which takes care of flow-id
> >> serialization where we serialize use ufid or unmasked key as flow id.
> >> Lets group ufid and unmasked key together rather than masked key and
> >> unmasked key which are not related.
> >
> > Right, at flow setup time the flow key is always required, but the UFID
> > is optional. For most other cases, one of the two most be specified. For
> > flow dump, neither is required from userspace, but OMIT_KEY flag may be
> > raised. That's the particular case that this logic is trying to catch
> > (dump all flows, including those that were set up without UFID - in
> > which case the OMIT_KEY flag doesn't make sense, so treat the flag like
> > a request rather than a command).
>
> How do you handle overlapping flows without the flow id in dump operation?
In userspace? revalidators will dump flows, then if the flow-key is exactly the
same as another flow then they will be hashed to the same udpif_key and the
second flow will be ignored each dump. So, stats may get messed up. If they
differ but overlap, they will hash to different udpif_keys and stats tracked
separately. Revalidator won't check the validity of overlapping flows until the
next change to ofproto-dpif.
^ permalink raw reply
* Re: [PATCH net 2/5] i40e: Implement ndo_gso_check()
From: Joe Stringer @ 2014-11-20 19:16 UTC (permalink / raw)
To: Jesse Gross
Cc: netdev, Jeff Kirsher, linux.nics, Tom Herbert,
Linux Kernel Mailing List, Shannon Nelson, jesse.brandeburg
In-Reply-To: <CAEP_g=8rbJF5z92LV_c14KooU=i3x=9WpikocctcEUOdw5-oLA@mail.gmail.com>
On Tuesday, November 04, 2014 15:45:22 Jesse Gross wrote:
> On Tue, Nov 4, 2014 at 1:56 PM, Joe Stringer <joestringer@nicira.com> wrote:
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > b/drivers/net/ethernet/intel/i40e/i40e_main.c index c3a7f4a..21829b5
> > 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > +static bool i40e_gso_check(struct sk_buff *skb, struct net_device *dev)
> > +{
> > + if ((skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL) &&
> > + (skb->inner_protocol_type != ENCAP_TYPE_ETHER ||
> > + skb->inner_protocol != htons(ETH_P_TEB) ||
> > + skb_inner_mac_header(skb) - skb_transport_header(skb) > 64))
> > + return false;
>
> I think it may be possible to even support a few more things here.
> According to the datasheet here:
> http://www.intel.com/content/dam/www/public/us/en/documents/datasheets/xl71
> 0-10-40-controller-datasheet.pdf
>
> This can actually support 64 bytes beyond the tunnel header, which
> would make for a total of 80 bytes. It looks like it can also support
> IPv4 or IPv6 beyond just Ethernet as the encapsulated protocol.
>
> Intel guys, can you confirm that this is correct?
I'm just respinning this for v4/6 beyond GRE/UDP tunnel and IPIP, and I found
the description of max protocol parsing size of 480B (with individual header
limit of 255B). I couldn't find where you get this 64/80 number or which
headers it maps to. Could you (or one of the intel guys) expand on this?
^ permalink raw reply
* i40e/i40e_ethtool.c weirdness?
From: Valdis Kletnieks @ 2014-11-20 19:21 UTC (permalink / raw)
To: Mitch Williams, Jeff Kirsher
Cc: Catherine Sullivan, Shannon Nelson, netdev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1861 bytes --]
(spotted while looking at a 'git bisect visualize' for something else)
After this commit:
Author: Mitch Williams <mitch.a.williams@intel.com> 2014-09-13 03:40:47
Committer: Jeff Kirsher <jeffrey.t.kirsher@intel.com> 2014-10-23 23:38:04
i40e: Add 10GBaseT support
Add driver support for 10GBaseT device.
we have the following chunk of code in i40e_ethtool.c:
case I40E_PHY_TYPE_10GBASE_SFPP_CU:
ecmd->supported = SUPPORTED_10000baseT_Full;
break;
case I40E_PHY_TYPE_1000BASE_KX:
case I40E_PHY_TYPE_1000BASE_T:
ecmd->supported = SUPPORTED_Autoneg |
SUPPORTED_10000baseT_Full |
SUPPORTED_1000baseT_Full |
SUPPORTED_100baseT_Full;
ecmd->advertising = ADVERTISED_Autoneg |
ADVERTISED_10000baseT_Full |
ADVERTISED_1000baseT_Full |
ADVERTISED_100baseT_Full;
break;
case I40E_PHY_TYPE_100BASE_TX:
ecmd->supported = SUPPORTED_Autoneg |
SUPPORTED_10000baseT_Full |
SUPPORTED_1000baseT_Full |
SUPPORTED_100baseT_Full;
ecmd->advertising = ADVERTISED_Autoneg |
ADVERTISED_10000baseT_Full |
ADVERTISED_1000baseT_Full |
ADVERTISED_100baseT_Full;
break;
case I40E_PHY_TYPE_SGMII:
ecmd->supported = SUPPORTED_Autoneg |
I'm confused by the fact that 2 cases that by name are 100M and 1G parts
got bits saying that 10G is "supported" - was that intentional?
[-- Attachment #2: Type: application/pgp-signature, Size: 848 bytes --]
^ permalink raw reply
* [PATCH net-next] filter: add bpf_optimize_div()
From: Denis Kirjanov @ 2014-11-20 19:42 UTC (permalink / raw)
To: netdev; +Cc: Denis Kirjanov, Alexei Starovoitov
optimize_div() found in mips bpf jit is really usefull
for other arches. So let's put it in filter.h
CC: Alexei Starovoitov <ast@plumgrid.com>
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
include/linux/filter.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index ca95abd..b385637 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -414,6 +414,17 @@ static inline void bpf_jit_dump(unsigned int flen, unsigned int proglen,
print_hex_dump(KERN_ERR, "JIT code: ", DUMP_PREFIX_OFFSET,
16, 1, image, proglen, false);
}
+
+static inline int bpf_optimize_div(u32 *k)
+{
+ /* power of 2 divides can be implemented with right shift */
+ if (!(*k & (*k-1))) {
+ *k = ilog2(*k);
+ return 1;
+ }
+
+ return 0;
+}
#else
static inline void bpf_jit_compile(struct bpf_prog *fp)
{
--
2.1.0
^ permalink raw reply related
* Re: [PATCH net 2/5] i40e: Implement ndo_gso_check()
From: Jesse Gross @ 2014-11-20 20:14 UTC (permalink / raw)
To: Joe Stringer
Cc: netdev, Jeff Kirsher, linux.nics, Tom Herbert,
Linux Kernel Mailing List, Shannon Nelson, Brandeburg, Jesse
In-Reply-To: <201411201116.04406.joestringer@nicira.com>
On Thu, Nov 20, 2014 at 11:16 AM, Joe Stringer <joestringer@nicira.com> wrote:
> On Tuesday, November 04, 2014 15:45:22 Jesse Gross wrote:
>> On Tue, Nov 4, 2014 at 1:56 PM, Joe Stringer <joestringer@nicira.com> wrote:
>> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> > b/drivers/net/ethernet/intel/i40e/i40e_main.c index c3a7f4a..21829b5
>> > 100644
>> > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> > +static bool i40e_gso_check(struct sk_buff *skb, struct net_device *dev)
>> > +{
>> > + if ((skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL) &&
>> > + (skb->inner_protocol_type != ENCAP_TYPE_ETHER ||
>> > + skb->inner_protocol != htons(ETH_P_TEB) ||
>> > + skb_inner_mac_header(skb) - skb_transport_header(skb) > 64))
>> > + return false;
>>
>> I think it may be possible to even support a few more things here.
>> According to the datasheet here:
>> http://www.intel.com/content/dam/www/public/us/en/documents/datasheets/xl71
>> 0-10-40-controller-datasheet.pdf
>>
>> This can actually support 64 bytes beyond the tunnel header, which
>> would make for a total of 80 bytes. It looks like it can also support
>> IPv4 or IPv6 beyond just Ethernet as the encapsulated protocol.
>>
>> Intel guys, can you confirm that this is correct?
>
> I'm just respinning this for v4/6 beyond GRE/UDP tunnel and IPIP, and I found
> the description of max protocol parsing size of 480B (with individual header
> limit of 255B). I couldn't find where you get this 64/80 number or which
> headers it maps to. Could you (or one of the intel guys) expand on this?
The number that I gave was from the section on Geneve support (on page
708), which says that it can support up to 64 bytes of options (this
was also my understanding from previous conversations with Intel
guys). I searched for 480 byte limit and it seems like it for receive
instead of transmit, which could conceivably be different.
^ permalink raw reply
* Re: [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
From: Vijay Subramanian @ 2014-11-20 20:15 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev, David Miller
In-Reply-To: <20141120185829.986CB290095D@tardy>
> + * 99 times out of 10, there won't actually be any
Minor typo here.. s/10/100.
Vijay
^ permalink raw reply
* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Eric Dumazet @ 2014-11-20 20:41 UTC (permalink / raw)
To: Cong Wang; +Cc: Wengang, netdev
In-Reply-To: <CAHA+R7PKRbBNwfUWRAsVHQreKsGXBc8CHorZRuyCfvwL4O=y+g@mail.gmail.com>
On Thu, 2014-11-20 at 09:34 -0800, Cong Wang wrote:
> I didn't look into ipoib_header_ops, thought it might have some dependency
> on symbols.
I did look before answering and suggesting this, you really should do
the same instead of giving advices of over engineering the stack.
Best is the enemy of the good.
Its hard to find some networking function trivial than this one.
static int ipoib_hard_header(struct sk_buff *skb,
struct net_device *dev,
unsigned short type,
const void *daddr, const void *saddr, unsigned len)
{
struct ipoib_header *header;
struct ipoib_cb *cb = ipoib_skb_cb(skb);
header = (struct ipoib_header *) skb_push(skb, sizeof *header);
header->proto = htons(type);
header->reserved = 0;
/*
* we don't rely on dst_entry structure, always stuff the
* destination address into skb->cb so we can figure out where
* to send the packet later.
*/
memcpy(cb->hwaddr, daddr, INFINIBAND_ALEN);
return sizeof *header;
}
^ permalink raw reply
* Re: [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
From: Eric Dumazet @ 2014-11-20 20:47 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev, davem
In-Reply-To: <20141120185829.986CB290095D@tardy>
On Thu, 2014-11-20 at 10:58 -0800, Rick Jones wrote:
> From: Rick Jones <rick.jones2@hp.com>
>
> When a system is the passive accepter of many connections, for example
> when the target of a netperf TCP_CC or TCP_CRR test, or as say a web
> server, the discard of the skb containing the TCP SYN being processed
> for the LISTEN endpoint should be a consume_skb() rather than a kfree_skb()
> to avoid cluttering a dropped packet profile.
>
> Signed-off-by: Rick Jones <rick.jones2@hp.com>
>
> ---
So what happens if we really drop the packet ?
TCP stack at this point owns the packet, it is possible to mark a bit in
it to either call consume_skb() or kfree_skb()
I attempted this once but gave up because it was a quite intrusive
patch...
^ permalink raw reply
* Re: [PATCH net-next] filter: add bpf_optimize_div()
From: Alexei Starovoitov @ 2014-11-20 21:07 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: Network Development
In-Reply-To: <1416512551-22252-1-git-send-email-kda@linux-powerpc.org>
On Thu, Nov 20, 2014 at 11:42 AM, Denis Kirjanov <kda@linux-powerpc.org> wrote:
> optimize_div() found in mips bpf jit is really usefull
> for other arches. So let's put it in filter.h
>
> CC: Alexei Starovoitov <ast@plumgrid.com>
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
> ---
> include/linux/filter.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index ca95abd..b385637 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> +
> +static inline int bpf_optimize_div(u32 *k)
'inline' is unnecessary
> +{
> + /* power of 2 divides can be implemented with right shift */
> + if (!(*k & (*k-1))) {
> + *k = ilog2(*k);
> + return 1;
> + }
> +
> + return 0;
> +}
I don't think it makes sense to add a helper function
without first user. If you really want to use it
in ppc, make this change as part of the series.
In the 1st patch move it out of mips and use it in
mips jit and 2nd patch use it in ppc.
Also since you mentioned mips...
if (k == 1 || optimize_div(&k)) {
ctx->flags |= SEEN_A;
emit_jit_reg_move(r_A, r_zero, ctx);
this is definitely wrong in there.
Also with such helper function we'd need to be
careful not to modify 'k' in place, otherwise
multi-pass jit algorithm would be broken.
In general I don't think such optimizations
belong in kernel at all. User space should be doing this.
^ permalink raw reply
* pull request: wireless 2014-11-20
From: John W. Linville @ 2014-11-20 21:00 UTC (permalink / raw)
To: davem; +Cc: linux-wireless, netdev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 6471 bytes --]
Dave,
Please full this little batch of fixes intended for the 3.18 stream!
For the mac80211 patch, Johannes says:
"Here's another last minute fix, for minstrel HT crashing
depending on the value of some uninitialised stack."
On top of that...
Ben Greear fixes an ath9k regression in which a BSSID mask is
miscalculated.
Dmitry Torokhov corrects an error handling routing in brcmfmac which
was checking an unsigned variable for a negative value.
Johannes Berg avoids a build problem in brcmfmac for arches where
linux/unaligned/access_ok.h and asm/unaligned.h conflict.
Mathy Vanhoef addresses another brcmfmac issue so as to eliminate a
use-after-free of the URB transfer buffer if a timeout occurs.
Please let me know if there are problems!
Thanks,
John
---
The following changes since commit 4e6ce4dc7ce71d0886908d55129d5d6482a27ff9:
ath9k: Fix RTC_DERIVED_CLK usage (2014-11-11 16:24:18 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git tags/master-2014-11-20
for you to fetch changes up to a1d69c60c44134f64945bbf6a6dfda22eaf4a214:
brcmfmac: don't include linux/unaligned/access_ok.h (2014-11-20 14:46:45 -0500)
----------------------------------------------------------------
Ben Greear (1):
ath9k: fix regression in bssidmask calculation
Dmitry Torokhov (1):
brcmfmac: fix error handling of irq_of_parse_and_map
Felix Fietkau (1):
mac80211: minstrel_ht: fix a crash in rate sorting
Johannes Berg (1):
brcmfmac: don't include linux/unaligned/access_ok.h
John W. Linville (1):
Merge tag 'mac80211-for-john-2014-11-18' of git://git.kernel.org/.../jberg/mac80211
Mathy Vanhoef (1):
brcmfmac: kill URB when request timed out
drivers/net/wireless/ath/ath9k/main.c | 9 ++++++---
drivers/net/wireless/brcm80211/brcmfmac/of.c | 4 ++--
drivers/net/wireless/brcm80211/brcmfmac/pcie.c | 2 +-
drivers/net/wireless/brcm80211/brcmfmac/usb.c | 6 ++++--
net/mac80211/rc80211_minstrel_ht.c | 15 ++++++---------
5 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 30c66dfcd7a0..4f18a6be0c7d 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -974,9 +974,8 @@ void ath9k_calculate_iter_data(struct ath_softc *sc,
struct ath_vif *avp;
/*
- * Pick the MAC address of the first interface as the new hardware
- * MAC address. The hardware will use it together with the BSSID mask
- * when matching addresses.
+ * The hardware will use primary station addr together with the
+ * BSSID mask when matching addresses.
*/
memset(iter_data, 0, sizeof(*iter_data));
memset(&iter_data->mask, 0xff, ETH_ALEN);
@@ -1205,6 +1204,8 @@ static int ath9k_add_interface(struct ieee80211_hw *hw,
list_add_tail(&avp->list, &avp->chanctx->vifs);
}
+ ath9k_calculate_summary_state(sc, avp->chanctx);
+
ath9k_assign_hw_queues(hw, vif);
an->sc = sc;
@@ -1274,6 +1275,8 @@ static void ath9k_remove_interface(struct ieee80211_hw *hw,
ath_tx_node_cleanup(sc, &avp->mcast_node);
+ ath9k_calculate_summary_state(sc, avp->chanctx);
+
mutex_unlock(&sc->mutex);
}
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/of.c b/drivers/net/wireless/brcm80211/brcmfmac/of.c
index f05f5270fec1..927bffd5be64 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/of.c
@@ -40,8 +40,8 @@ void brcmf_of_probe(struct brcmf_sdio_dev *sdiodev)
return;
irq = irq_of_parse_and_map(np, 0);
- if (irq < 0) {
- brcmf_err("interrupt could not be mapped: err=%d\n", irq);
+ if (!irq) {
+ brcmf_err("interrupt could not be mapped\n");
devm_kfree(dev, sdiodev->pdata);
return;
}
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/brcm80211/brcmfmac/pcie.c
index 8c0632ec9f7a..16fef3382019 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/pcie.c
@@ -19,10 +19,10 @@
#include <linux/pci.h>
#include <linux/vmalloc.h>
#include <linux/delay.h>
-#include <linux/unaligned/access_ok.h>
#include <linux/interrupt.h>
#include <linux/bcma/bcma.h>
#include <linux/sched.h>
+#include <asm/unaligned.h>
#include <soc.h>
#include <chipcommon.h>
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
index dc135915470d..875d1142c8b0 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
@@ -669,10 +669,12 @@ static int brcmf_usb_dl_cmd(struct brcmf_usbdev_info *devinfo, u8 cmd,
goto finalize;
}
- if (!brcmf_usb_ioctl_resp_wait(devinfo))
+ if (!brcmf_usb_ioctl_resp_wait(devinfo)) {
+ usb_kill_urb(devinfo->ctl_urb);
ret = -ETIMEDOUT;
- else
+ } else {
memcpy(buffer, tmpbuf, buflen);
+ }
finalize:
kfree(tmpbuf);
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index df90ce2db00c..408fd8ab4eef 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -252,19 +252,16 @@ minstrel_ht_sort_best_tp_rates(struct minstrel_ht_sta *mi, u8 index,
cur_thr = mi->groups[cur_group].rates[cur_idx].cur_tp;
cur_prob = mi->groups[cur_group].rates[cur_idx].probability;
- tmp_group = tp_list[j - 1] / MCS_GROUP_RATES;
- tmp_idx = tp_list[j - 1] % MCS_GROUP_RATES;
- tmp_thr = mi->groups[tmp_group].rates[tmp_idx].cur_tp;
- tmp_prob = mi->groups[tmp_group].rates[tmp_idx].probability;
-
- while (j > 0 && (cur_thr > tmp_thr ||
- (cur_thr == tmp_thr && cur_prob > tmp_prob))) {
- j--;
+ do {
tmp_group = tp_list[j - 1] / MCS_GROUP_RATES;
tmp_idx = tp_list[j - 1] % MCS_GROUP_RATES;
tmp_thr = mi->groups[tmp_group].rates[tmp_idx].cur_tp;
tmp_prob = mi->groups[tmp_group].rates[tmp_idx].probability;
- }
+ if (cur_thr < tmp_thr ||
+ (cur_thr == tmp_thr && cur_prob <= tmp_prob))
+ break;
+ j--;
+ } while (j > 0);
if (j < MAX_THR_RATES - 1) {
memmove(&tp_list[j + 1], &tp_list[j], (sizeof(*tp_list) *
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply related
* Re: [PATCH net-next 3/4] igb: enable internal PPS for the i210.
From: Keller, Jacob E @ 2014-11-20 21:28 UTC (permalink / raw)
To: richardcochran@gmail.com
Cc: netdev@vger.kernel.org, davem@davemloft.net, Allan, Bruce W,
Ronciak, John, Kirsher, Jeffrey T, Vick, Matthew
In-Reply-To: <20141120091804.GA4587@localhost.localdomain>
On Thu, 2014-11-20 at 10:18 +0100, Richard Cochran wrote:
> On Wed, Nov 19, 2014 at 09:06:19PM +0000, Keller, Jacob E wrote:
> > On Wed, 2014-11-19 at 21:26 +0100, Richard Cochran wrote:
> > > On Wed, Nov 19, 2014 at 07:32:33PM +0000, Keller, Jacob E wrote:
> > > > Good catch :)
>
> I have not been able to reproduce the crash, and so the cause is not
> what I thought it was. Maybe it was my patch that preserved the
> enabled interrupts in igb_ptp_reset(). I didn't notice that the driver
> frees and reallocates the ptp_clock. I would never do that, myself.
>
Yea, I think that was a design I tried in the ixgbe driver, but it
really isn't good.
> > I think you need something here, but it should be clearing that register
> > after a MAC reset, so it needs to be re-initialized. I'm not sure if
> > that reset path was used in the same place in the past.
>
> Okay, lets figure this out. Why is there a PTP reset function at all?
> I don't know, lets see who calls it...
>
> Finding functions calling: igb_ptp_reset
> ----------------------------------------
> *** drivers/net/ethernet/intel/igb/igb_main.c:
> igb_reset[2033] igb_ptp_reset(adapter);
>
> Easy enough to understand. But who is calling igb_reset?
>
> Finding functions calling: igb_reset
> ------------------------------------
> *** drivers/net/ethernet/intel/igb/igb_ethtool.c:
> igb_set_settings[345] igb_reset(adapter);
> igb_set_pauseparam[409] igb_reset(adapter);
> igb_diag_test[2016] igb_reset(adapter);
> igb_set_eee[2729] igb_reset(adapter);
> *** drivers/net/ethernet/intel/igb/igb_main.c:
> igb_down[1814] igb_reset(adapter);
> igb_set_features[2069] igb_reset(adapter);
> igb_probe[2526] igb_reset(adapter);
> __igb_open[3110] igb_reset(adapter);
> igb_watchdog_task[4231] igb_reset(adapter);
> igb_change_mtu[5189] igb_reset(adapter);
> igb_resume[7460] igb_reset(adapter);
> igb_sriov_reinit[7545] igb_reset(adapter);
> igb_io_slot_reset[7678] igb_reset(adapter);
>
> Wow, that is quite much. So, whenever any random parameter is changed,
> we reset the PTP clock. Great.
Here is the problem. In igb_reset, we call e1000_reset_hw() which will
eventually perform a hardware MAC reset. Yes maybe we shouldn't be
calling igb_reset everywhere.. I cannot answer that, however...
>
> Really, wouldn't better to reset the clock functions only when
> absolutely necessary?
>
We *are*. If e1000_reset_hw(hw) is called, the MAC registers will be
reset to their initial values, which includes not having SYSTIME setup,
and not having the outputs configured. There is not much that can be
done to avoid that besides major refactoring of the igb driver to avoid
resetting as much as it does. In most of those cases, I think the reset
is fine, and we actually aren't going to reset that many times during
nominal operation.
Some of the ethtool settings maybe could avoid resets, but I am not
certain during exactly which path they end up calling resets.
Regards,
Jake
> Thanks,
> Richard
^ permalink raw reply
* Re: [RFC] net: ipv4: drop unicast encapsulated in L2 multicast
From: Johannes Berg @ 2014-11-20 21:31 UTC (permalink / raw)
To: David Miller
Cc: ja-FgGsKACvmQM, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140822.105405.1982870131653082781.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Fri, 2014-08-22 at 10:54 -0700, David Miller wrote:
> >> if (res.type == RTN_BROADCAST)
> >> goto brd_input;
> >
> > Is this place better, after checking for RTN_BROADCAST?
> >
> > /* ARP link-layer broadcasts are acceptable here */
> > if ((skb->pkt_type == PACKET_BROADCAST ||
> > skb->pkt_type == PACKET_MULTICAST) &&
> > skb->protocol == htons(ETH_P_IP))
> > goto e_inval;
>
> Indeed, this would make ARP happier, but that still leaves open the
> issue of CLUSTERIP.
I'm back looking at this, but must admit I'm completely confused now :-)
I could add an IPv4 sysctl to control this behaviour:
0 - off
1 - RFC 1122 "SHOULD"
2 - also drop unicast-in-multicast (for wireless)
But I guess due to cluster-IP it would have to default to 0.
However, talk about ip_local_deliver_finish() in this thread has me
wondering if we could just implement it using iptables? I guess
ipt_addrtype and ip6t_addrtype would let me do that?
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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 net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
From: Rick Jones @ 2014-11-20 21:31 UTC (permalink / raw)
To: Eric Dumazet, Rick Jones; +Cc: netdev, davem
In-Reply-To: <1416516423.8629.43.camel@edumazet-glaptop2.roam.corp.google.com>
On 11/20/2014 12:47 PM, Eric Dumazet wrote:
> On Thu, 2014-11-20 at 10:58 -0800, Rick Jones wrote:
>> From: Rick Jones <rick.jones2@hp.com>
>>
>> When a system is the passive accepter of many connections, for example
>> when the target of a netperf TCP_CC or TCP_CRR test, or as say a web
>> server, the discard of the skb containing the TCP SYN being processed
>> for the LISTEN endpoint should be a consume_skb() rather than a kfree_skb()
>> to avoid cluttering a dropped packet profile.
>>
>> Signed-off-by: Rick Jones <rick.jones2@hp.com>
>>
>> ---
>
> So what happens if we really drop the packet ?
Do you mean when there is actually data in the SYN?
rick
>
> TCP stack at this point owns the packet, it is possible to mark a bit in
> it to either call consume_skb() or kfree_skb()
>
> I attempted this once but gave up because it was a quite intrusive
> patch...
^ permalink raw reply
* Re: [PATCH net-next] filter: add bpf_optimize_div()
From: Daniel Borkmann @ 2014-11-20 21:32 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: netdev, Alexei Starovoitov
In-Reply-To: <1416512551-22252-1-git-send-email-kda@linux-powerpc.org>
On 11/20/2014 08:42 PM, Denis Kirjanov wrote:
> optimize_div() found in mips bpf jit is really usefull
> for other arches. So let's put it in filter.h
>
> CC: Alexei Starovoitov <ast@plumgrid.com>
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
NAK
1) There's no user of this function (you name MIPS JIT, but
don't remove it from there) ...
2) It's not really specific or tied to the BPF API in particular,
so doesn't really belong here, rather some more generic kernel
header, iff anything.
> ---
> include/linux/filter.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index ca95abd..b385637 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -414,6 +414,17 @@ static inline void bpf_jit_dump(unsigned int flen, unsigned int proglen,
> print_hex_dump(KERN_ERR, "JIT code: ", DUMP_PREFIX_OFFSET,
> 16, 1, image, proglen, false);
> }
> +
> +static inline int bpf_optimize_div(u32 *k)
> +{
> + /* power of 2 divides can be implemented with right shift */
> + if (!(*k & (*k-1))) {
> + *k = ilog2(*k);
> + return 1;
> + }
> +
> + return 0;
> +}
> #else
> static inline void bpf_jit_compile(struct bpf_prog *fp)
> {
>
^ permalink raw reply
* Re: [RFC] situation with csum_and_copy_... API
From: Al Viro @ 2014-11-20 21:47 UTC (permalink / raw)
To: David Miller
Cc: torvalds, netdev, linux-kernel, target-devel,
Nicholas A. Bellinger, Christoph Hellwig
In-Reply-To: <20141119.165340.2162829993279387495.davem@davemloft.net>
On Wed, Nov 19, 2014 at 04:53:40PM -0500, David Miller wrote:
> Pulled, thanks Al.
Umm... Not in net-next.git#master... Anyway, the next portion is in
vfs.git#iov_iter-net right now; I'll post it on netdev once I get some
sleep.
It's getting close to really interesting parts. Right now the main obstacle
is in iscsit_do_rx_data/iscsit_do_tx_data; what happens there is reuse of
iovec if kernel_sendmsg() gives a short write - it tries to send again, with
the same iovec and decremented length. Ditto on RX side (with kernel_recvmsg(),
obviously).
As far as I can see, these retries on the send side are simply broken -
normally we are talking to TCP sockets there and tcp_sendmsg() does *not*
modify iovec in normal case. IOW, if you get 8K sent out of 80K, the next
time it'll try to send 72K - already sent piece + 64K following it, etc.
Could target-devel folks tell how realistic those resends are, in the
first place? Both with TX and RX sides... Is there any sane limit on
iovec size there, etc.
Note that while conversion to iov_iter will provide a very simple solution
(iovec remains unchanged, iterator advances and we just need to avoid
reinitializing it for subsequent iterations in those loops), it won't solve
the problem in older kernels; that code had been there since 2011 and
iov_iter conversion is far too invasive for -stable.
^ permalink raw reply
* Re: [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
From: Eric Dumazet @ 2014-11-20 21:48 UTC (permalink / raw)
To: Rick Jones; +Cc: Rick Jones, netdev, davem
In-Reply-To: <546E5DAF.5090907@hp.com>
On Thu, 2014-11-20 at 13:31 -0800, Rick Jones wrote:
> Do you mean when there is actually data in the SYN?
No, I mean the packet can be _dropped_ here, really.
SYN flood for example.
^ permalink raw reply
* Re: [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
From: Rick Jones @ 2014-11-20 21:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, davem
In-Reply-To: <1416520109.8629.44.camel@edumazet-glaptop2.roam.corp.google.com>
On 11/20/2014 01:48 PM, Eric Dumazet wrote:
> On Thu, 2014-11-20 at 13:31 -0800, Rick Jones wrote:
>
>> Do you mean when there is actually data in the SYN?
>
> No, I mean the packet can be _dropped_ here, really.
>
> SYN flood for example.
Ah. I did not pick-up on that. Seeing the kfree_skb() in the original
code I thought it was the end of the line for the segment.
rick
^ permalink raw reply
* Re: [RFC] situation with csum_and_copy_... API
From: Eric Dumazet @ 2014-11-20 21:55 UTC (permalink / raw)
To: Al Viro
Cc: David Miller, torvalds, netdev, linux-kernel, target-devel,
Nicholas A. Bellinger, Christoph Hellwig
In-Reply-To: <20141120214753.GR7996@ZenIV.linux.org.uk>
On Thu, 2014-11-20 at 21:47 +0000, Al Viro wrote:
> As far as I can see, these retries on the send side are simply broken -
> normally we are talking to TCP sockets there and tcp_sendmsg() does *not*
> modify iovec in normal case.
Arg... I sent this morning something doing this (against net-next tree)
Is it a problem ?
Or can we consider FASTOPEN being not normal case ? ;)
https://patchwork.ozlabs.org/patch/412776/
^ permalink raw reply
* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Cong Wang @ 2014-11-20 21:57 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Wengang, netdev
In-Reply-To: <1416516104.8629.39.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, Nov 20, 2014 at 12:41 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2014-11-20 at 09:34 -0800, Cong Wang wrote:
>
>> I didn't look into ipoib_header_ops, thought it might have some dependency
>> on symbols.
>
> I did look before answering and suggesting this, you really should do
> the same instead of giving advices of over engineering the stack.
>
> Best is the enemy of the good.
>
> Its hard to find some networking function trivial than this one.
What about other modules defining *header_ops? Don't they
need to move to vmlinux as well?
I still don't like this workaround even just for stable. Although
definitely a real fix could be harder to backport, for me it is normal
backport 8+ patches to stable:
http://www.spinics.net/lists/stable/msg66122.html
http://www.spinics.net/lists/linux-fsdevel/msg79967.html
I know you disagree, I don't even want to waste time on arguing it.
^ permalink raw reply
* Re: [PATCH net-next] filter: add bpf_optimize_div()
From: Daniel Borkmann @ 2014-11-20 21:57 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Denis Kirjanov, Network Development
In-Reply-To: <CAMEtUuw1q_=CuZOHqd1WDd8ZE+bo-CNsbQ4rFPgRRVy7H1Q4uA@mail.gmail.com>
On 11/20/2014 10:07 PM, Alexei Starovoitov wrote:
> On Thu, Nov 20, 2014 at 11:42 AM, Denis Kirjanov <kda@linux-powerpc.org> wrote:
...
>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>> index ca95abd..b385637 100644
>> --- a/include/linux/filter.h
>> +++ b/include/linux/filter.h
>> +
>> +static inline int bpf_optimize_div(u32 *k)
>
> 'inline' is unnecessary
Btw, it's a header file here, so it's necessary here.
...
> In general I don't think such optimizations
> belong in kernel at all. User space should be doing this.
+1
^ permalink raw reply
* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Eric Dumazet @ 2014-11-20 22:03 UTC (permalink / raw)
To: Cong Wang; +Cc: Wengang, netdev
In-Reply-To: <CAHA+R7Me4zPdGpeQVHFLLGVq=4cVar09jQFyjA81nxF_Fmuppw@mail.gmail.com>
On Thu, 2014-11-20 at 13:57 -0800, Cong Wang wrote:
> On Thu, Nov 20, 2014 at 12:41 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Thu, 2014-11-20 at 09:34 -0800, Cong Wang wrote:
> >
> >> I didn't look into ipoib_header_ops, thought it might have some dependency
> >> on symbols.
> >
> > I did look before answering and suggesting this, you really should do
> > the same instead of giving advices of over engineering the stack.
> >
> > Best is the enemy of the good.
> >
> > Its hard to find some networking function trivial than this one.
>
> What about other modules defining *header_ops? Don't they
> need to move to vmlinux as well?
Yep, if they can be in a bonding device, for practical reasons, not to
prove your point.
>
> I still don't like this workaround even just for stable. Although
> definitely a real fix could be harder to backport, for me it is normal
> backport 8+ patches to stable:
>
> http://www.spinics.net/lists/stable/msg66122.html
> http://www.spinics.net/lists/linux-fsdevel/msg79967.html
>
> I know you disagree, I don't even want to waste time on arguing it.
Whatever, I really don't care.
Do your stuff, but don't ask people asking for an easy fix to do the
heart surgery.
Provide a patch, please.
^ permalink raw reply
* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Cong Wang @ 2014-11-20 22:13 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Wengang, netdev
In-Reply-To: <1416521035.8629.49.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, Nov 20, 2014 at 2:03 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> Provide a patch, please.
>
>
Don't blame me.
I want to provide a real fix, you want a minimum fix for stable.
We agree that we disagree on this point, right? What's
more, according to your rule, I should yield to you when I
touch something you want to touch.
Also, no one seems to care about my previous question:
why only bonding has the problem?
^ permalink raw reply
* Re: [PATCH 0/4] move pci_assivned_vfs() check (while disabling VFs) to pci sub-system
From: Bjorn Helgaas @ 2014-11-20 22:21 UTC (permalink / raw)
To: Sathya Perla
Cc: linux-pci, netdev, ariel.elior, linux.nics, shahed.shaikh,
ddutile, Jeff Kirsher
In-Reply-To: <1415620410-4937-1-git-send-email-sathya.perla@emulex.com>
[+cc Jeff]
On Mon, Nov 10, 2014 at 05:23:26PM +0530, Sathya Perla wrote:
> A user must not be allowed to disable VFs while they are already assigned to
> a guest. This check is being made in each individual driver that implements
> the sriov_configure PCI method.
> This patch-set fixes this code duplication by moving this check from
> drivers to the sriov_nuvfs_store() routine just before invoking
> sriov_configure() when num_vfs is equal to 0.
>
> Vasundhara Volam (4):
> pci: move pci_assivned_vfs() check while disabling VFs to pci
> sub-system
> bnx2x: remove pci_assigned_vfs() check while disabling VFs
> i40e: remove pci_assigned_vfs() check while disabling VFs
> qlcnic: remove pci_assigned_vfs() check while disabling VFs
>
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 2 +-
> drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 7 +------
> .../net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c | 10 ----------
> drivers/pci/pci-sysfs.c | 5 +++++
> 4 files changed, 7 insertions(+), 17 deletions(-)
I'm dropping these for the reasons Don & Alex outlined -- they don't fix
the problem for VFIO, so this amounts to shuffling around code that's known
to be broken, which seems more confusing than worthwhile.
Jeff, if I were you I would drop the i40e patch. I don't think it makes
sense to remove the check from i40e before adding it to the PCI core.
Bjorn
^ 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