* Re: [PATCH v3] net/irda: fix lockdep annotation
From: Dmitry Vyukov @ 2017-01-19 10:05 UTC (permalink / raw)
To: David Miller
Cc: Dave Jones, Samuel Ortiz, Alexander Potapenko, andreyknvl, netdev
In-Reply-To: <20170117.151901.998994334183973013.davem@davemloft.net>
On Tue, Jan 17, 2017 at 9:19 PM, David Miller <davem@davemloft.net> wrote:
> From: Dmitry Vyukov <dvyukov@google.com>
> Date: Mon, 16 Jan 2017 22:10:52 +0100
>
>> The current annotation uses a global variable as recursion counter.
>> The variable is not atomic nor protected with a mutex, but mutated
>> by multiple threads. This causes lockdep bug reports episodically:
>>
>> BUG: looking up invalid subclass: 4294967295
>> ...
>> _raw_spin_lock_irqsave_nested+0x120/0x180
>> hashbin_delete+0x4fe/0x750
>> __irias_delete_object+0xab/0x170
>> irias_delete_object+0x5f/0xc0
>> ircomm_tty_detach_cable+0x1d5/0x3f0
>> ...
>>
>> Make the hashbin_lock_depth variable atomic to prevent bug reports.
>>
>> As is this causes "unused variable 'depth'" warning without LOCKDEP.
>> So also change raw_spin_lock_irqsave_nested() macro to not cause
>> the warning without LOCKDEP. Similar to what raw_spin_lock_nested()
>> already does.
>>
>> Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
>> Cc: Dave Jones <davej@redhat.com>
>> Cc: Samuel Ortiz <samuel@sortiz.org>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: netdev@vger.kernel.org
>> Fixes: c7630a4b932af ("[IrDA]: irda lockdep annotation")
>
> I took a closer look at this, and really this whole lockdep thing is
> more than a hack.
>
> The basic problem is that the free_func() passed into hashbin_delete()
> can trigger hashbin operations on other hashtables.
>
> This function is destroying a hash table completely, and is doing so
> in a context where no new hash table entries can be inserted. The
> last thing we do is kfree() the hashtable so this must be true.
>
> Therefore, it is much better to just kill off all of this lockdep
> stuff, and recode the teardown loop into something like:
>
> if (hashbin->hb_type & HB_LOCK)
> spin_lock_irqsave(&hashbin->hb_lock, flags);
> for (i = 0; i < HASHBIN_SIZE; i++) {
> while (1) {
> queue = dequeue_first((irda_queue_t **) &hashbin->hb_queue[i]);
>
> if (!queue)
> break;
> if (free_func) {
> if (hashbin->hb_type & HB_LOCK)
> spin_unlock_irqrestore(&hashbin->hb_lock, flags);
> free_func(queue);
> if (hashbin->hb_type & HB_LOCK)
> spin_lock_irqsave(&hashbin->hb_lock, flags);
> }
>
> }
> }
> hashbin->hb_current = NULL;
> hashbin->magic = ~HB_MAGIC;
> if (hashbin->hb_type & HB_LOCK)
> spin_unlock_irqrestore(&hashbin->hb_lock, flags);
>
> At which point the recursive locking becomes impossible.
Thanks for looking into it! This particular issue bothers my fuzzers
considerably. I agree that removing recursion is better.
So do how we proceed? Will you mail this as a real patch?
^ permalink raw reply
* RE: [PATCH net-next] macb: Common code to enable ptp support for SAMA5Dx platforms.
From: Andrei.Pistirica @ 2017-01-19 10:21 UTC (permalink / raw)
To: nicolas.ferre, netdev, linux-kernel, linux-arm-kernel, davem,
harinikatakamlinux, harini.katakam, richardcochran, rafalo
Cc: boris.brezillon, alexandre.belloni, michals, tbultel, anirudh,
punnaia
In-Reply-To: <1c534b5a-25cb-a5d6-f87c-db9ed958a606@atmel.com>
> Subject: Re: [PATCH net-next] macb: Common code to enable ptp support
> for SAMA5Dx platforms.
>
> Le 18/01/2017 à 09:57, Andrei Pistirica a écrit :
> > This patch does the following:
> > - add GEM-PTP interface
> > - registers and bitfields for TSU are named according to SAMA5Dx data
> > sheet
> > - PTP support based on platform capability
>
> The $subject will certainly never match reality, sadly "enable ptp support
> for SAMA5Dx platforms". So, you'd better change it.
> (no "." at the end BTW).
I will change it to: " Common code to enable ptp support for MACB/GEM"
> > +2518,7 @@ static void macb_configure_caps(struct macb *bp,
> > dcfg = gem_readl(bp, DCFG2);
> > if ((dcfg & (GEM_BIT(RX_PKT_BUFF) |
> GEM_BIT(TX_PKT_BUFF))) == 0)
> > bp->caps |= MACB_CAPS_FIFO_MODE;
> > +
>
> Nitpicking, just because other issue exists: this white line doesn't belong to
> the patch.
Ok I'll remove it. I missed it because checkpatch didn't report any warning.
[...]
> > #define MACB_CAPS_GIGABIT_MODE_AVAILABLE 0x20000000
> > #define MACB_CAPS_SG_DISABLED 0x40000000
> > #define MACB_CAPS_MACB_IS_GEM 0x80000000
> > +#define MACB_CAPS_GEM_HAS_PTP 0x00000020
>
> No, this mask already exists a couple of lines above:
> #define MACB_CAPS_JUMBO 0x00000020
>
> That leads to a NACK, sorry (I didn't spotted earlier, BTW).
Yes... you are right... sorry.
[...]
> Otherwise, I'm okay with the rest.
>
> I suggest to people that will keep the ball rolling on this topic to take
> advantage of the chunks of code that Andrei developed with the help of
> Richard and the best practices discussed. I think particularly, if it makes
> sense with HW, about:
> - gem_ptp_do_[rt]xstamp(bp, skb) dereference scheme
> - gem_ptp_adjfine() rationale
> - gem_get_ptp_peer() if needed
Just mind that in case of an implementation with buffer rings and irqs
a different mechanism have to be used.
Regards,
Andrei
>
> Regards,
> --
> Nicolas Ferre
^ permalink raw reply
* [PATCH] rtlwifi: rtl8192x: Enabling and disabling hardware interrupts after enabling local irq flags
From: Bharat Kumar Gogada @ 2017-01-19 10:14 UTC (permalink / raw)
To: Larry.Finger, chaoming_li, linux-wireless, linux-kernel
Cc: kvalo, netdev, rgummal, Bharat Kumar Gogada
-Realtek 8192CE chipset maintains local irq flags after enabling/disabling
hardware interrupts.
-Hardware interrupts are enabled before enabling the local irq
flags(these flags are being checked in interrupt handler),
leading to race condition on some RP, where the irq line between
bridge and GIC goes high at ASSERT_INTx and goes low only
at DEASSERT_INTx. In this kind of RP by the time ASSERT_INTx is seen
irq_enable flag is still set to false, resulting in continuous
interrupts seen by CPU as DEASSERT_INTx cannot be sent since
flag is still false and making CPU stall.
-Changing the sequence of setting these irq flags.
Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com>
---
drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
index a47be73..143766c4 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
@@ -1306,9 +1306,9 @@ void rtl92ce_enable_interrupt(struct ieee80211_hw *hw)
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
+ rtlpci->irq_enabled = true;
rtl_write_dword(rtlpriv, REG_HIMR, rtlpci->irq_mask[0] & 0xFFFFFFFF);
rtl_write_dword(rtlpriv, REG_HIMRE, rtlpci->irq_mask[1] & 0xFFFFFFFF);
- rtlpci->irq_enabled = true;
}
void rtl92ce_disable_interrupt(struct ieee80211_hw *hw)
@@ -1316,9 +1316,9 @@ void rtl92ce_disable_interrupt(struct ieee80211_hw *hw)
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
+ rtlpci->irq_enabled = false;
rtl_write_dword(rtlpriv, REG_HIMR, IMR8190_DISABLED);
rtl_write_dword(rtlpriv, REG_HIMRE, IMR8190_DISABLED);
- rtlpci->irq_enabled = false;
}
static void _rtl92ce_poweroff_adapter(struct ieee80211_hw *hw)
--
2.1.1
^ permalink raw reply related
* Re: [PATCH v3] net/irda: fix lockdep annotation
From: Dmitry Vyukov @ 2017-01-19 10:33 UTC (permalink / raw)
To: David Miller
Cc: Dave Jones, Samuel Ortiz, Alexander Potapenko, andreyknvl, netdev
In-Reply-To: <CACT4Y+Yx3wVAgxJRYKMOYM419NTSc9naSxUVfwCb21kjvAnF6Q@mail.gmail.com>
On Thu, Jan 19, 2017 at 11:05 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Tue, Jan 17, 2017 at 9:19 PM, David Miller <davem@davemloft.net> wrote:
>> From: Dmitry Vyukov <dvyukov@google.com>
>> Date: Mon, 16 Jan 2017 22:10:52 +0100
>>
>>> The current annotation uses a global variable as recursion counter.
>>> The variable is not atomic nor protected with a mutex, but mutated
>>> by multiple threads. This causes lockdep bug reports episodically:
>>>
>>> BUG: looking up invalid subclass: 4294967295
>>> ...
>>> _raw_spin_lock_irqsave_nested+0x120/0x180
>>> hashbin_delete+0x4fe/0x750
>>> __irias_delete_object+0xab/0x170
>>> irias_delete_object+0x5f/0xc0
>>> ircomm_tty_detach_cable+0x1d5/0x3f0
>>> ...
>>>
>>> Make the hashbin_lock_depth variable atomic to prevent bug reports.
>>>
>>> As is this causes "unused variable 'depth'" warning without LOCKDEP.
>>> So also change raw_spin_lock_irqsave_nested() macro to not cause
>>> the warning without LOCKDEP. Similar to what raw_spin_lock_nested()
>>> already does.
>>>
>>> Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
>>> Cc: Dave Jones <davej@redhat.com>
>>> Cc: Samuel Ortiz <samuel@sortiz.org>
>>> Cc: David S. Miller <davem@davemloft.net>
>>> Cc: netdev@vger.kernel.org
>>> Fixes: c7630a4b932af ("[IrDA]: irda lockdep annotation")
>>
>> I took a closer look at this, and really this whole lockdep thing is
>> more than a hack.
>>
>> The basic problem is that the free_func() passed into hashbin_delete()
>> can trigger hashbin operations on other hashtables.
>>
>> This function is destroying a hash table completely, and is doing so
>> in a context where no new hash table entries can be inserted. The
>> last thing we do is kfree() the hashtable so this must be true.
>>
>> Therefore, it is much better to just kill off all of this lockdep
>> stuff, and recode the teardown loop into something like:
>>
>> if (hashbin->hb_type & HB_LOCK)
>> spin_lock_irqsave(&hashbin->hb_lock, flags);
>> for (i = 0; i < HASHBIN_SIZE; i++) {
>> while (1) {
>> queue = dequeue_first((irda_queue_t **) &hashbin->hb_queue[i]);
>>
>> if (!queue)
>> break;
>> if (free_func) {
>> if (hashbin->hb_type & HB_LOCK)
>> spin_unlock_irqrestore(&hashbin->hb_lock, flags);
>> free_func(queue);
>> if (hashbin->hb_type & HB_LOCK)
>> spin_lock_irqsave(&hashbin->hb_lock, flags);
>> }
>>
>> }
>> }
>> hashbin->hb_current = NULL;
>> hashbin->magic = ~HB_MAGIC;
>> if (hashbin->hb_type & HB_LOCK)
>> spin_unlock_irqrestore(&hashbin->hb_lock, flags);
>>
>> At which point the recursive locking becomes impossible.
>
>
> Thanks for looking into it! This particular issue bothers my fuzzers
> considerably. I agree that removing recursion is better.
> So do how we proceed? Will you mail this as a real patch?
Meanwhile I applied the following locally:
diff --git a/net/irda/irqueue.c b/net/irda/irqueue.c
index acbe61c7e683..c42d66a9ac8c 100644
--- a/net/irda/irqueue.c
+++ b/net/irda/irqueue.c
@@ -383,9 +383,6 @@ EXPORT_SYMBOL(hashbin_new);
* for deallocating this structure if it's complex. If not the user can
* just supply kfree, which should take care of the job.
*/
-#ifdef CONFIG_LOCKDEP
-static int hashbin_lock_depth = 0;
-#endif
int hashbin_delete( hashbin_t* hashbin, FREE_FUNC free_func)
{
irda_queue_t* queue;
@@ -396,22 +393,22 @@ int hashbin_delete( hashbin_t* hashbin,
FREE_FUNC free_func)
IRDA_ASSERT(hashbin->magic == HB_MAGIC, return -1;);
/* Synchronize */
- if ( hashbin->hb_type & HB_LOCK ) {
- spin_lock_irqsave_nested(&hashbin->hb_spinlock, flags,
- hashbin_lock_depth++);
- }
+ if ( hashbin->hb_type & HB_LOCK )
+ spin_lock_irqsave(&hashbin->hb_spinlock, flags);
/*
* Free the entries in the hashbin, TODO: use hashbin_clear when
* it has been shown to work
*/
- for (i = 0; i < HASHBIN_SIZE; i ++ ) {
- queue = dequeue_first((irda_queue_t**) &hashbin->hb_queue[i]);
- while (queue ) {
- if (free_func)
- (*free_func)(queue);
- queue = dequeue_first(
- (irda_queue_t**) &hashbin->hb_queue[i]);
+ for (i = 0; i < HASHBIN_SIZE; i++) {
+ while ((queue = dequeue_first((irda_queue_t **)
&hashbin->hb_queue[i]))) {
+ if (!free_func)
+ continue;
+ if (hashbin->hb_type & HB_LOCK)
+
spin_unlock_irqrestore(&hashbin->hb_spinlock, flags);
+ free_func(queue);
+ if (hashbin->hb_type & HB_LOCK)
+ spin_lock_irqsave(&hashbin->hb_spinlock, flags);
}
}
@@ -420,12 +417,8 @@ int hashbin_delete( hashbin_t* hashbin, FREE_FUNC
free_func)
hashbin->magic = ~HB_MAGIC;
/* Release lock */
- if ( hashbin->hb_type & HB_LOCK) {
+ if ( hashbin->hb_type & HB_LOCK)
spin_unlock_irqrestore(&hashbin->hb_spinlock, flags);
-#ifdef CONFIG_LOCKDEP
- hashbin_lock_depth--;
-#endif
- }
/*
* Free the hashbin structure
The more I look at this file the stranger it all looks to me... Is it
a queue? hashtable? locked or not? and an iterator at the same time?
And the free_func seems to be NULL only when we know the queue is empty anyway.
^ permalink raw reply related
* Re: [net, 3/6] net: korina: increase tx/rx ring sizes
From: Roman Yeryomin @ 2017-01-19 10:55 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1484680816.13165.73.camel@edumazet-glaptop3.roam.corp.google.com>
On 17 January 2017 at 21:20, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2017-01-17 at 20:27 +0200, Roman Yeryomin wrote:
>> On 17 January 2017 at 19:58, David Miller <davem@davemloft.net> wrote:
>> > From: Roman Yeryomin <leroi.lists@gmail.com>
>> > Date: Tue, 17 Jan 2017 19:32:36 +0200
>> >
>> >> Having larger ring sizes almost eliminates rx fifo overflow, thus improving performance.
>> >> This patch reduces rx overflow occurence by approximately 1000 times (from ~25k down to ~25 times per 3M frames)
>> >
>> > Those numbers don't mean much without full context.
>> >
>> > What kind of system, what kind of traffic, and over what kind of link?
>>
>> MIPS rb532 board, TCP iperf3 test over 100M link, NATed speed ~55Mbps.
>> I can do more tests and provide more precise numbers, if needed.
>
> Note that at 100M, 64 rx descriptors have a 8 ms max latency.
>
> Switching to 256 also multiply by 4 the latency -> 32 ms latency.
>
> Presumably switching to NAPI and GRO would avoid the latency increase
> and save a lot of cpu cycles for a MIPS board.
>
Eric, thanks for suggesting GRO, it gives huge performance gain when
receiving locally (55->95Mbps) and more than 25% gain for NAT
(55->70Mbps).
Also reading the datasheet more carefully I see that device rx
descriptor status flags are interpreted incorrectly. So will provide
an updated set.
Thanks for feedback!
Regards,
Roman
^ permalink raw reply
* [PATCH] net: atm: Fix build error when !CONFIG_PROC_FS
From: Augusto Mecking Caringi @ 2017-01-19 11:00 UTC (permalink / raw)
To: David S . Miller
Cc: Augusto Mecking Caringi, Jiri Pirko, Ido Schimmel, netdev,
linux-kernel
When CONFIG_ATM_CLIP is set and CONFIG_PROC_FS is not set, the building
was failing whith the error:
net/atm/clip.c: In function ‘atm_clip_exit’:
net/atm/clip.c:933:27: error: ‘atm_proc_root’ undeclared (first use in
this function)
remove_proc_entry("arp", atm_proc_root);
^
Fix it by putting the proc cleanup code inside a #ifdef CONFIG_PROC_FS
block.
Signed-off-by: Augusto Mecking Caringi <augustocaringi@gmail.com>
---
net/atm/clip.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/atm/clip.c b/net/atm/clip.c
index 53b4ac0..75221e4 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -930,7 +930,9 @@ static void atm_clip_exit_noproc(void)
static void __exit atm_clip_exit(void)
{
+#ifdef CONFIG_PROC_FS
remove_proc_entry("arp", atm_proc_root);
+#endif
atm_clip_exit_noproc();
}
--
2.7.4
^ permalink raw reply related
* Re: [Xen-devel] xennet_start_xmit assumptions
From: Sowmini Varadhan @ 2017-01-19 11:14 UTC (permalink / raw)
To: Paul Durrant
Cc: Konrad Rzeszutek Wilk, Wei Liu, netdev@vger.kernel.org,
xen-devel@lists.xenproject.org
In-Reply-To: <de0d7a7db4c24b46838aee7623d5d2a3@AMSPEX02CL03.citrite.net>
On (01/19/17 09:36), Paul Durrant wrote:
>
> Hi Sowmini,
>
> Sounds like a straightforward bug to me... netfront should be able
> to handle an empty skb and clearly, if it's relying on skb_headlen()
> being non-zero, that's not the case.
>
> Paul
I see. Seems like there are 2 things broken here: recovering
from skb->len = 0, and recovering from the more complex
case of (skb->len > 0 && skb_headlen(skb) == 0)
Do you folks want to take a shot at fixing this,
since you know the code better? If you are interested,
I can share my test program to help you reproduce the
simpler skb->len == 0 case, but it's the fully non-linear
skbs that may be more interesting to reproduce/fix.
I'll probably work on fixing packet_snd to return -EINVAL
or similar when the len is zero this week.
--Sowmini
^ permalink raw reply
* Re: xennet_start_xmit assumptions
From: Paul Durrant @ 2017-01-19 11:31 UTC (permalink / raw)
To: Sowmini Varadhan
Cc: netdev@vger.kernel.org, Wei Liu, xen-devel@lists.xenproject.org,
Konrad Rzeszutek Wilk
In-Reply-To: <20170119111426.GA22018@oracle.com>
> -----Original Message-----
> From: Sowmini Varadhan [mailto:sowmini.varadhan@oracle.com]
> Sent: 19 January 2017 11:14
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Wei Liu
> <wei.liu2@citrix.com>; netdev@vger.kernel.org; xen-
> devel@lists.xenproject.org
> Subject: Re: [Xen-devel] xennet_start_xmit assumptions
>
> On (01/19/17 09:36), Paul Durrant wrote:
> >
> > Hi Sowmini,
> >
> > Sounds like a straightforward bug to me... netfront should be able
> > to handle an empty skb and clearly, if it's relying on skb_headlen()
> > being non-zero, that's not the case.
> >
> > Paul
>
> I see. Seems like there are 2 things broken here: recovering
> from skb->len = 0, and recovering from the more complex
> case of (skb->len > 0 && skb_headlen(skb) == 0)
>
> Do you folks want to take a shot at fixing this,
> since you know the code better? If you are interested,
> I can share my test program to help you reproduce the
> simpler skb->len == 0 case, but it's the fully non-linear
> skbs that may be more interesting to reproduce/fix.
Sowmini,
Yeah, it would be useful to verify any change fixes the particular issue you're seeing so please share the program. For the non-empty non-linear case I'd hope that catching this and doing a pull of some sensible amount of header (which might coincide with the least amount that netback expects to see in the first frag) would be enough.
I can take a shot at a patch for this in the next few days; I'll add your 'Reported-by' so you should get cc-ed.
Cheers,
Paul
>
> I'll probably work on fixing packet_snd to return -EINVAL
> or similar when the len is zero this week.
>
> --Sowmini
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply
* Re: [Xen-devel] xennet_start_xmit assumptions
From: Sowmini Varadhan @ 2017-01-19 11:37 UTC (permalink / raw)
To: Paul Durrant
Cc: Konrad Rzeszutek Wilk, Wei Liu, netdev@vger.kernel.org,
xen-devel@lists.xenproject.org
In-Reply-To: <22c048f35d3845158b225d51dde33bf7@AMSPEX02CL03.citrite.net>
On (01/19/17 11:31), Paul Durrant wrote:
> Sowmini,
>
> Yeah, it would be useful to verify any change fixes the particular
> issue you're seeing so please share the program. For the non-empty
> non-linear case I'd hope that catching this and doing a pull of some
> sensible amount of header (which might coincide with the least amount
> that netback expects to see in the first frag) would be enough.
> I can take a shot at a patch for this in the next few days; I'll add
> your 'Reported-by' so you should get cc-ed.
Sounds good, here is the test program (very basic, no error checks!).
I think you can probably massage the pf_packet code to send down a
purely non-linear skb, I can take a look at what this involves and
get back to you.
#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <netinet/in.h>
#include <net/if.h>
#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <ctype.h>
#include <string.h>
#include <linux/if_packet.h>
#include <linux/if_ether.h>
int main(int argc, char *argv[])
{
struct ifreq ifr;
int sockfd = socket(AF_INET, SOCK_DGRAM, 0);
struct sockaddr_ll sll;
int fd;
struct msghdr msg;
bzero(&ifr, sizeof (ifr));
strncpy(ifr.ifr_name, argv[1], sizeof(ifr.ifr_name));
ioctl(sockfd, SIOCGIFHWADDR, &ifr);
bzero(&sll, sizeof (sll));
sll.sll_family = PF_PACKET;
sll.sll_halen = ETH_ALEN;
sll.sll_ifindex = if_nametoindex(argv[1]);
memcpy(sll.sll_addr, (struct sockaddr *)&(ifr.ifr_hwaddr), ETH_ALEN);
fd = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
bind(fd, (struct sockaddr *)&sll, sizeof(sll));
bzero(&msg, sizeof (msg));
msg.msg_name = (void *)&sll;
msg.msg_namelen = sizeof (struct sockaddr_ll);
sendmsg(fd, &msg, 0);
fprintf(stderr, "sent!\n");
}
^ permalink raw reply
* [PATCH iproute2 net-next] tc: flower: Refactor matching flags to be more user friendly
From: Paul Blakey @ 2017-01-19 11:32 UTC (permalink / raw)
To: Stephen Hemminger, netdev
Cc: Jiri Pirko, Or Gerlitz, Roi Dayan, Jiri Benc, Simon Horman,
Paul Blakey
Instead of "magic numbers" we can now specify each flag
by name. Prefix of "no_" (e.g no_frag) unsets the flag,
otherwise it wil be set.
Example:
# add a flower filter that will drop fragmented packets
tc filter add dev ens4f0 protocol ip parent ffff: \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags frag \
action drop
# add a flower filter that will drop non-fragmented packets
tc filter add dev ens4f0 protocol ip parent ffff: \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags no_frag \
action drop
Fixes: 22a8f019891c ('tc: flower: support matching flags')
Signed-off-by: Paul Blakey <paulb@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
Hi,
Added a framework to add new flags more easily, such
as the upcoming tcp_flags (see kernel cls_flower), and other ip_flags.
Thanks,
Paul.
man/man8/tc-flower.8 | 12 +++++-
tc/f_flower.c | 117 ++++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 102 insertions(+), 27 deletions(-)
diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 2dd2c5e..77da10b 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@ -46,7 +46,9 @@ flower \- flow based traffic control filter
.BR enc_dst_ip " | " enc_src_ip " } { "
.IR ipv4_address " | " ipv6_address " } | "
.B enc_dst_port
-.IR port_number
+.IR port_number " | "
+.BR ip_flags
+.IR IP_FLAGS
.SH DESCRIPTION
The
.B flower
@@ -183,13 +185,19 @@ prefix length. If the prefix is missing, \fBtc\fR assumes a full-length
host match. Dst port
.I NUMBER
is a 16 bit UDP dst port.
+.TP
+.BI ip_flags " IP_FLAGS"
+.I IP_FLAGS
+may be either
+.BR frag " or " no_frag
+to match on fragmented packets or not respectively.
.SH NOTES
As stated above where applicable, matches of a certain layer implicitly depend
on the matches of the next lower layer. Precisely, layer one and two matches
(\fBindev\fR, \fBdst_mac\fR and \fBsrc_mac\fR)
have no dependency, layer three matches
(\fBip_proto\fR, \fBdst_ip\fR, \fBsrc_ip\fR, \fBarp_tip\fR, \fBarp_sip\fR,
-\fBarp_op\fR, \fBarp_tha\fR and \fBarp_sha\fR)
+\fBarp_op\fR, \fBarp_tha\fR, \fBarp_sha\fR and \fBip_flags\fR)
depend on the
.B protocol
option of tc filter, layer four port matches
diff --git a/tc/f_flower.c b/tc/f_flower.c
index d301db3..3bafc75 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -24,6 +24,10 @@
#include "tc_util.h"
#include "rt_names.h"
+enum flower_matching_flags {
+ FLOWER_IP_FLAGS,
+};
+
enum flower_endpoint {
FLOWER_ENDPOINT_SRC,
FLOWER_ENDPOINT_DST
@@ -63,7 +67,7 @@ static void explain(void)
" enc_dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
" enc_src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
" enc_key_id [ KEY-ID ] |\n"
- " matching_flags MATCHING-FLAGS | \n"
+ " ip_flags IP-FLAGS | \n"
" enc_dst_port [ port_number ] }\n"
" FILTERID := X:Y:Z\n"
" MASKED_LLADDR := { LLADDR | LLADDR/MASK | LLADDR/BITS }\n"
@@ -136,28 +140,56 @@ static int flower_parse_vlan_eth_type(char *str, __be16 eth_type, int type,
return 0;
}
-static int flower_parse_matching_flags(char *str, int type, int mask_type,
- struct nlmsghdr *n)
-{
- __u32 mtf, mtf_mask;
- char *c;
+struct flag_to_string {
+ int flag;
+ enum flower_matching_flags type;
+ char *string;
+};
- c = strchr(str, '/');
- if (c)
- *c = '\0';
+static struct flag_to_string flags_str[] = {
+ { TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT, FLOWER_IP_FLAGS, "frag" },
+};
- if (get_u32(&mtf, str, 0))
- return -1;
+static int flower_parse_matching_flags(char *str,
+ enum flower_matching_flags type,
+ __u32 *mtf, __u32 *mtf_mask)
+{
+ char *token;
+ bool no;
+ bool found;
+ int i;
- if (c) {
- if (get_u32(&mtf_mask, ++c, 0))
+ token = strtok(str, "|");
+
+ while (token) {
+ if (!strncmp(token, "no_", 3)) {
+ no = true;
+ token = strchr(token, '_') + 1;
+ } else
+ no = false;
+
+ found = false;
+ for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
+ if (type != flags_str[i].type)
+ continue;
+
+ if (!strcmp(token, flags_str[i].string)) {
+ if (no)
+ *mtf &= ~flags_str[i].flag;
+ else
+ *mtf |= flags_str[i].flag;
+
+ *mtf_mask |= flags_str[i].flag;
+ found = true;
+ break;
+ }
+ }
+ if (!found)
return -1;
- } else {
- mtf_mask = 0xffffffff;
+
+ token = strtok(NULL, "|");
}
- addattr32(n, MAX_MSG, type, htonl(mtf));
- addattr32(n, MAX_MSG, mask_type, htonl(mtf_mask));
return 0;
}
@@ -433,6 +465,8 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
__be16 vlan_ethtype = 0;
__u8 ip_proto = 0xff;
__u32 flags = 0;
+ __u32 mtf = 0;
+ __u32 mtf_mask = 0;
if (handle) {
ret = get_u32(&t->tcm_handle, handle, 0);
@@ -462,14 +496,14 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
return -1;
}
addattr_l(n, MAX_MSG, TCA_FLOWER_CLASSID, &handle, 4);
- } else if (matches(*argv, "matching_flags") == 0) {
+ } else if (matches(*argv, "ip_flags") == 0) {
NEXT_ARG();
ret = flower_parse_matching_flags(*argv,
- TCA_FLOWER_KEY_FLAGS,
- TCA_FLOWER_KEY_FLAGS_MASK,
- n);
+ FLOWER_IP_FLAGS,
+ &mtf,
+ &mtf_mask);
if (ret < 0) {
- fprintf(stderr, "Illegal \"matching_flags\"\n");
+ fprintf(stderr, "Illegal \"ip_flags\"\n");
return -1;
}
} else if (matches(*argv, "skip_hw") == 0) {
@@ -723,6 +757,16 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
parse_done:
addattr32(n, MAX_MSG, TCA_FLOWER_FLAGS, flags);
+ if (mtf_mask) {
+ ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS, htonl(mtf));
+ if (ret)
+ return ret;
+
+ ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS_MASK, htonl(mtf_mask));
+ if (ret)
+ return ret;
+ }
+
ret = addattr16(n, MAX_MSG, TCA_FLOWER_KEY_ETH_TYPE, eth_type);
if (ret) {
fprintf(stderr, "Illegal \"eth_type\"(0x%x)\n",
@@ -828,14 +872,36 @@ static void flower_print_ip_proto(FILE *f, __u8 *p_ip_proto,
}
static void flower_print_matching_flags(FILE *f, char *name,
+ enum flower_matching_flags type,
struct rtattr *attr,
struct rtattr *mask_attr)
{
+ int i;
+ int count = 0;
+ __u32 mtf;
+ __u32 mtf_mask;
+
if (!mask_attr || RTA_PAYLOAD(mask_attr) != 4)
return;
- fprintf(f, "\n %s 0x%08x/0x%08x", name, ntohl(rta_getattr_u32(attr)),
- mask_attr ? ntohl(rta_getattr_u32(mask_attr)) : 0xffffffff);
+ mtf = ntohl(rta_getattr_u32(attr));
+ mtf_mask = ntohl(rta_getattr_u32(mask_attr));
+
+ for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
+ if (type != flags_str[i].type)
+ continue;
+ if (mtf_mask & flags_str[i].flag) {
+ if (++count == 1)
+ fprintf(f, "\n %s ", name);
+ else
+ fprintf(f, "|");
+
+ if (mtf & flags_str[i].flag)
+ fprintf(f, "%s", flags_str[i].string);
+ else
+ fprintf(f, "no_%s", flags_str[i].string);
+ }
+ }
}
static void flower_print_ip_addr(FILE *f, char *name, __be16 eth_type,
@@ -1034,7 +1100,8 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
flower_print_port(f, "enc_dst_port",
tb[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
- flower_print_matching_flags(f, "matching_flags",
+ flower_print_matching_flags(f, "ip_flags",
+ FLOWER_IP_FLAGS,
tb[TCA_FLOWER_KEY_FLAGS],
tb[TCA_FLOWER_KEY_FLAGS_MASK]);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH iproute2 net-next] tc: flower: support matching flags
From: Paul Blakey @ 2017-01-19 11:39 UTC (permalink / raw)
To: Jiri Benc
Cc: paulb, Simon Horman, netdev, Stephen Hemminger, David S. Miller,
Hadar Hen Zion, Or Gerlitz, Roi Dayan
In-Reply-To: <20170118134103.351bd524@griffin>
On 18/01/2017 14:41, Jiri Benc wrote:
> On Wed, 4 Jan 2017 12:55:59 +0100, Jiri Benc wrote:
>> On Wed, 4 Jan 2017 13:51:13 +0200, Paul Blakey wrote:
>>> It mimics the kernel packing of flags, I have no problem either way
>>> (flags, or ip_flags/tcp_flags pairs), what do you think jiri?
>>
>> What Simon says makes sense to me. ip_flags and tcp_flags sounds like
>> the best solution so far (even better than my original suggestion).
>
> Is there any progress with the follow up patch? I don't think we want
> iproute2 with the magic numbers to be released.
>
> Thanks,
>
> Jiri
>
Hi,
I've posted a patch:
"[PATCH iproute2 net-next] tc: flower: Refactor matching flags to be
more user friendly"
Thanks,
Paul.
^ permalink raw reply
* RE: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
From: David Laight @ 2017-01-19 11:43 UTC (permalink / raw)
To: 'Alexander Duyck'
Cc: David Miller, maowenan@huawei.com, netdev@vger.kernel.org,
jeffrey.t.kirsher@intel.com
In-Reply-To: <CAKgT0Ud5K4GZtbyNOMUh=o8oGTukPA7qvvnK6ptO+KwaTSqGkA@mail.gmail.com>
From: Alexander Duyck
> Sent: 18 January 2017 17:25
> On Wed, Jan 18, 2017 at 8:22 AM, David Laight <David.Laight@aculab.com> wrote:
> > From: David Miller
> >> Sent: 17 January 2017 19:16
> >> > Relax ordering(RO) is one feature of 82599 NIC, to enable this feature can
> >> > enhance the performance for some cpu architecure, such as SPARC and so on.
> >> > Currently it only supports one special cpu architecture(SPARC) in 82599
> >> > driver to enable RO feature, this is not very common for other cpu architecture
> >> > which really needs RO feature.
> >> > This patch add one common config CONFIG_ARCH_WANT_RELAX_ORDER to set RO feature,
> >> > and should define CONFIG_ARCH_WANT_RELAX_ORDER in sparc Kconfig firstly.
> >> >
> >> > Signed-off-by: Mao Wenan <maowenan@huawei.com>
> >>
> >> Since no-one has reviewed this patch, and I do not feel comfortable with applying
> >> it without such review, I am tossing this patch.
> >>
> >> If someone eventually reviews it, repost this patch.
> >
> > Having re-read parts of the PCIe spec I think I'd like someone to
> > explain exactly which transfers are affected by the 'relaxed ordering'
> > bit and why any re-ordered transactions aren't a problem.
> >
> > In particular I believe RO allows the write to update the receive
> > descriptor ring to overtake a write of receive packet data.
> > That could lead to the network stack processing a receive frame
> > before it has actually been written.
> >
> > David
> >
>
> The Relaxed Ordering attribute doesn't get applied across the board.
> It ends up being limited to a subset of the transactions if I recall
> correctly. In this case it is the Tx descriptor write back, and the
> Rx data write back. We don't apply the RO bit to any other
> transactions.
>
> In the case of Tx descriptor there is no harm in allowing it to be
> reordered because we only really read the DD bit so we don't care
> about the ordering of the write back. In the case of the Rx data the
> Rx descriptor essentially acts as a flush since it is sent without the
> RO bit set. So all the writes before it must be completed before the
> Rx descriptor write back.
In which case why not set it unconditionally for all architectures?
I'm surprised (I often am) that allowing those re-orderings makes
any significant difference.
Unfortunately you need a PCIe analyser to see what is really happening
and they don't come cheap.
What I do vaguely remember is that some hosts don't always implement
the 'normal' re-ordering of reads and read completions.
Re-ordering of reads allows descriptor reads to overtake transmit
traffic which is likely to make a difference.
David
^ permalink raw reply
* [PATCH v2 net-next] phy: increase size of bus_id and MII_BUS_ID_SIZE
From: Volodymyr Bendiuga @ 2017-01-19 11:46 UTC (permalink / raw)
To: f.fainelli, andrew, netdev, volodymyr.bendiuga
Cc: Volodymyr Bendiuga, Magnus Öberg
Some bus names are pretty long and do not fit into
20 chars, therefore bus_id size is increased together
with MII_BUS_ID_SIZE to host larger names.
Signed-off-by: Volodymyr Bendiuga <volodymyr.bendiuga@gmail.com>
Signed-off-by: Magnus Öberg <magnus.oberg@westermo.se>
---
include/linux/phy.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index f7d95f6..f53f0cd 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -162,7 +162,7 @@ static inline const char *phy_modes(phy_interface_t interface)
* Need to be a little smaller than phydev->dev.bus_id to leave room
* for the ":%02x"
*/
-#define MII_BUS_ID_SIZE (20 - 3)
+#define MII_BUS_ID_SIZE (64 - 3)
/* Or MII_ADDR_C45 into regnum for read/write on mii_bus to enable the 21 bit
IEEE 802.3ae clause 45 addressing mode used by 10GIGE phy chips. */
@@ -632,7 +632,7 @@ struct phy_driver {
/* A Structure for boards to register fixups with the PHY Lib */
struct phy_fixup {
struct list_head list;
- char bus_id[20];
+ char bus_id[64];
u32 phy_uid;
u32 phy_uid_mask;
int (*run)(struct phy_device *phydev);
--
2.7.4
^ permalink raw reply related
* [PATCH iproute2 net-next V2] tc: flower: Refactor matching flags to be more user friendly
From: Paul Blakey @ 2017-01-19 12:17 UTC (permalink / raw)
To: Stephen Hemminger, netdev
Cc: Jiri Pirko, Or Gerlitz, Roi Dayan, Jiri Benc, Simon Horman,
Paul Blakey
Instead of "magic numbers" we can now specify each flag
by name. Prefix of "no_" (e.g no_frag) unsets the flag,
otherwise it wil be set.
Example:
# add a flower filter that will drop fragmented packets
tc filter add dev ens4f0 protocol ip parent ffff: \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags frag \
action drop
# add a flower filter that will drop non-fragmented packets
tc filter add dev ens4f0 protocol ip parent ffff: \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags no_frag \
action drop
Fixes: 22a8f019891c ('tc: flower: support matching flags')
Signed-off-by: Paul Blakey <paulb@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
Hi,
Added a framework to add new flags more easily, such
as the upcoming tcp_flags (see kernel cls_flower), and other ip_flags.
Thanks,
Paul.
man/man8/tc-flower.8 | 12 +++++-
tc/f_flower.c | 117 ++++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 102 insertions(+), 27 deletions(-)
diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 2dd2c5e..77da10b 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -46,7 +46,9 @@ flower \- flow based traffic control filter
.BR enc_dst_ip " | " enc_src_ip " } { "
.IR ipv4_address " | " ipv6_address " } | "
.B enc_dst_port
-.IR port_number
+.IR port_number " | "
+.BR ip_flags
+.IR IP_FLAGS
.SH DESCRIPTION
The
.B flower
@@ -183,13 +185,19 @@ prefix length. If the prefix is missing, \fBtc\fR assumes a full-length
host match. Dst port
.I NUMBER
is a 16 bit UDP dst port.
+.TP
+.BI ip_flags " IP_FLAGS"
+.I IP_FLAGS
+may be either
+.BR frag " or " no_frag
+to match on fragmented packets or not respectively.
.SH NOTES
As stated above where applicable, matches of a certain layer implicitly depend
on the matches of the next lower layer. Precisely, layer one and two matches
(\fBindev\fR, \fBdst_mac\fR and \fBsrc_mac\fR)
have no dependency, layer three matches
(\fBip_proto\fR, \fBdst_ip\fR, \fBsrc_ip\fR, \fBarp_tip\fR, \fBarp_sip\fR,
-\fBarp_op\fR, \fBarp_tha\fR and \fBarp_sha\fR)
+\fBarp_op\fR, \fBarp_tha\fR, \fBarp_sha\fR and \fBip_flags\fR)
depend on the
.B protocol
option of tc filter, layer four port matches
diff --git a/tc/f_flower.c b/tc/f_flower.c
index d301db3..af2ab55 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -24,6 +24,10 @@
#include "tc_util.h"
#include "rt_names.h"
+enum flower_matching_flags {
+ FLOWER_IP_FLAGS,
+};
+
enum flower_endpoint {
FLOWER_ENDPOINT_SRC,
FLOWER_ENDPOINT_DST
@@ -63,7 +67,7 @@ static void explain(void)
" enc_dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
" enc_src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
" enc_key_id [ KEY-ID ] |\n"
- " matching_flags MATCHING-FLAGS | \n"
+ " ip_flags IP-FLAGS | \n"
" enc_dst_port [ port_number ] }\n"
" FILTERID := X:Y:Z\n"
" MASKED_LLADDR := { LLADDR | LLADDR/MASK | LLADDR/BITS }\n"
@@ -136,28 +140,56 @@ static int flower_parse_vlan_eth_type(char *str, __be16 eth_type, int type,
return 0;
}
-static int flower_parse_matching_flags(char *str, int type, int mask_type,
- struct nlmsghdr *n)
-{
- __u32 mtf, mtf_mask;
- char *c;
+struct flag_to_string {
+ int flag;
+ enum flower_matching_flags type;
+ char *string;
+};
- c = strchr(str, '/');
- if (c)
- *c = '\0';
+static struct flag_to_string flags_str[] = {
+ { TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT, FLOWER_IP_FLAGS, "frag" },
+};
- if (get_u32(&mtf, str, 0))
- return -1;
+static int flower_parse_matching_flags(char *str,
+ enum flower_matching_flags type,
+ __u32 *mtf, __u32 *mtf_mask)
+{
+ char *token;
+ bool no;
+ bool found;
+ int i;
- if (c) {
- if (get_u32(&mtf_mask, ++c, 0))
+ token = strtok(str, "/");
+
+ while (token) {
+ if (!strncmp(token, "no_", 3)) {
+ no = true;
+ token = strchr(token, '_') + 1;
+ } else
+ no = false;
+
+ found = false;
+ for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
+ if (type != flags_str[i].type)
+ continue;
+
+ if (!strcmp(token, flags_str[i].string)) {
+ if (no)
+ *mtf &= ~flags_str[i].flag;
+ else
+ *mtf |= flags_str[i].flag;
+
+ *mtf_mask |= flags_str[i].flag;
+ found = true;
+ break;
+ }
+ }
+ if (!found)
return -1;
- } else {
- mtf_mask = 0xffffffff;
+
+ token = strtok(NULL, "/");
}
- addattr32(n, MAX_MSG, type, htonl(mtf));
- addattr32(n, MAX_MSG, mask_type, htonl(mtf_mask));
return 0;
}
@@ -433,6 +465,8 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
__be16 vlan_ethtype = 0;
__u8 ip_proto = 0xff;
__u32 flags = 0;
+ __u32 mtf = 0;
+ __u32 mtf_mask = 0;
if (handle) {
ret = get_u32(&t->tcm_handle, handle, 0);
@@ -462,14 +496,14 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
return -1;
}
addattr_l(n, MAX_MSG, TCA_FLOWER_CLASSID, &handle, 4);
- } else if (matches(*argv, "matching_flags") == 0) {
+ } else if (matches(*argv, "ip_flags") == 0) {
NEXT_ARG();
ret = flower_parse_matching_flags(*argv,
- TCA_FLOWER_KEY_FLAGS,
- TCA_FLOWER_KEY_FLAGS_MASK,
- n);
+ FLOWER_IP_FLAGS,
+ &mtf,
+ &mtf_mask);
if (ret < 0) {
- fprintf(stderr, "Illegal \"matching_flags\"\n");
+ fprintf(stderr, "Illegal \"ip_flags\"\n");
return -1;
}
} else if (matches(*argv, "skip_hw") == 0) {
@@ -723,6 +757,16 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
parse_done:
addattr32(n, MAX_MSG, TCA_FLOWER_FLAGS, flags);
+ if (mtf_mask) {
+ ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS, htonl(mtf));
+ if (ret)
+ return ret;
+
+ ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS_MASK, htonl(mtf_mask));
+ if (ret)
+ return ret;
+ }
+
ret = addattr16(n, MAX_MSG, TCA_FLOWER_KEY_ETH_TYPE, eth_type);
if (ret) {
fprintf(stderr, "Illegal \"eth_type\"(0x%x)\n",
@@ -828,14 +872,36 @@ static void flower_print_ip_proto(FILE *f, __u8 *p_ip_proto,
}
static void flower_print_matching_flags(FILE *f, char *name,
+ enum flower_matching_flags type,
struct rtattr *attr,
struct rtattr *mask_attr)
{
+ int i;
+ int count = 0;
+ __u32 mtf;
+ __u32 mtf_mask;
+
if (!mask_attr || RTA_PAYLOAD(mask_attr) != 4)
return;
- fprintf(f, "\n %s 0x%08x/0x%08x", name, ntohl(rta_getattr_u32(attr)),
- mask_attr ? ntohl(rta_getattr_u32(mask_attr)) : 0xffffffff);
+ mtf = ntohl(rta_getattr_u32(attr));
+ mtf_mask = ntohl(rta_getattr_u32(mask_attr));
+
+ for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
+ if (type != flags_str[i].type)
+ continue;
+ if (mtf_mask & flags_str[i].flag) {
+ if (++count == 1)
+ fprintf(f, "\n %s ", name);
+ else
+ fprintf(f, "/");
+
+ if (mtf & flags_str[i].flag)
+ fprintf(f, "%s", flags_str[i].string);
+ else
+ fprintf(f, "no_%s", flags_str[i].string);
+ }
+ }
}
static void flower_print_ip_addr(FILE *f, char *name, __be16 eth_type,
@@ -1034,7 +1100,8 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
flower_print_port(f, "enc_dst_port",
tb[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
- flower_print_matching_flags(f, "matching_flags",
+ flower_print_matching_flags(f, "ip_flags",
+ FLOWER_IP_FLAGS,
tb[TCA_FLOWER_KEY_FLAGS],
tb[TCA_FLOWER_KEY_FLAGS_MASK]);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH iproute2 net-next] tc: flower: Refactor matching flags to be more user friendly
From: Paul Blakey @ 2017-01-19 12:19 UTC (permalink / raw)
To: Stephen Hemminger, netdev
Cc: paulb, Jiri Pirko, Or Gerlitz, Roi Dayan, Jiri Benc, Simon Horman
In-Reply-To: <1484825537-26141-1-git-send-email-paulb@mellanox.com>
On 19/01/2017 13:32, Paul Blakey wrote:
> Instead of "magic numbers" we can now specify each flag
> by name. Prefix of "no_" (e.g no_frag) unsets the flag,
> otherwise it wil be set.
>
> Example:
> # add a flower filter that will drop fragmented packets
> tc filter add dev ens4f0 protocol ip parent ffff: \
> flower \
> src_mac e4:1d:2d:fd:8b:01 \
> dst_mac e4:1d:2d:fd:8b:02 \
> indev ens4f0 \
> ip_flags frag \
> action drop
>
> # add a flower filter that will drop non-fragmented packets
> tc filter add dev ens4f0 protocol ip parent ffff: \
> flower \
> src_mac e4:1d:2d:fd:8b:01 \
> dst_mac e4:1d:2d:fd:8b:02 \
> indev ens4f0 \
> ip_flags no_frag \
> action drop
>
> Fixes: 22a8f019891c ('tc: flower: support matching flags')
> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> ---
>
> Hi,
> Added a framework to add new flags more easily, such
> as the upcoming tcp_flags (see kernel cls_flower), and other ip_flags.
>
> Thanks,
> Paul.
>
>
> man/man8/tc-flower.8 | 12 +++++-
> tc/f_flower.c | 117 ++++++++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 102 insertions(+), 27 deletions(-)
>
> diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
> index 2dd2c5e..77da10b 100644
> --- a/man/man8/tc-flower.8
> +++ b/man/man8/tc-flower.8
> @ -46,7 +46,9 @@ flower \- flow based traffic control filter
> .BR enc_dst_ip " | " enc_src_ip " } { "
> .IR ipv4_address " | " ipv6_address " } | "
> .B enc_dst_port
> -.IR port_number
> +.IR port_number " | "
> +.BR ip_flags
> +.IR IP_FLAGS
> .SH DESCRIPTION
> The
> .B flower
> @@ -183,13 +185,19 @@ prefix length. If the prefix is missing, \fBtc\fR assumes a full-length
> host match. Dst port
> .I NUMBER
> is a 16 bit UDP dst port.
> +.TP
> +.BI ip_flags " IP_FLAGS"
> +.I IP_FLAGS
> +may be either
> +.BR frag " or " no_frag
> +to match on fragmented packets or not respectively.
> .SH NOTES
> As stated above where applicable, matches of a certain layer implicitly depend
> on the matches of the next lower layer. Precisely, layer one and two matches
> (\fBindev\fR, \fBdst_mac\fR and \fBsrc_mac\fR)
> have no dependency, layer three matches
> (\fBip_proto\fR, \fBdst_ip\fR, \fBsrc_ip\fR, \fBarp_tip\fR, \fBarp_sip\fR,
> -\fBarp_op\fR, \fBarp_tha\fR and \fBarp_sha\fR)
> +\fBarp_op\fR, \fBarp_tha\fR, \fBarp_sha\fR and \fBip_flags\fR)
> depend on the
> .B protocol
> option of tc filter, layer four port matches
> diff --git a/tc/f_flower.c b/tc/f_flower.c
> index d301db3..3bafc75 100644
> --- a/tc/f_flower.c
> +++ b/tc/f_flower.c
> @@ -24,6 +24,10 @@
> #include "tc_util.h"
> #include "rt_names.h"
>
> +enum flower_matching_flags {
> + FLOWER_IP_FLAGS,
> +};
> +
> enum flower_endpoint {
> FLOWER_ENDPOINT_SRC,
> FLOWER_ENDPOINT_DST
> @@ -63,7 +67,7 @@ static void explain(void)
> " enc_dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
> " enc_src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
> " enc_key_id [ KEY-ID ] |\n"
> - " matching_flags MATCHING-FLAGS | \n"
> + " ip_flags IP-FLAGS | \n"
> " enc_dst_port [ port_number ] }\n"
> " FILTERID := X:Y:Z\n"
> " MASKED_LLADDR := { LLADDR | LLADDR/MASK | LLADDR/BITS }\n"
> @@ -136,28 +140,56 @@ static int flower_parse_vlan_eth_type(char *str, __be16 eth_type, int type,
> return 0;
> }
>
> -static int flower_parse_matching_flags(char *str, int type, int mask_type,
> - struct nlmsghdr *n)
> -{
> - __u32 mtf, mtf_mask;
> - char *c;
> +struct flag_to_string {
> + int flag;
> + enum flower_matching_flags type;
> + char *string;
> +};
>
> - c = strchr(str, '/');
> - if (c)
> - *c = '\0';
> +static struct flag_to_string flags_str[] = {
> + { TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT, FLOWER_IP_FLAGS, "frag" },
> +};
>
> - if (get_u32(&mtf, str, 0))
> - return -1;
> +static int flower_parse_matching_flags(char *str,
> + enum flower_matching_flags type,
> + __u32 *mtf, __u32 *mtf_mask)
> +{
> + char *token;
> + bool no;
> + bool found;
> + int i;
>
> - if (c) {
> - if (get_u32(&mtf_mask, ++c, 0))
> + token = strtok(str, "|");
> +
> + while (token) {
> + if (!strncmp(token, "no_", 3)) {
> + no = true;
> + token = strchr(token, '_') + 1;
> + } else
> + no = false;
> +
> + found = false;
> + for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
> + if (type != flags_str[i].type)
> + continue;
> +
> + if (!strcmp(token, flags_str[i].string)) {
> + if (no)
> + *mtf &= ~flags_str[i].flag;
> + else
> + *mtf |= flags_str[i].flag;
> +
> + *mtf_mask |= flags_str[i].flag;
> + found = true;
> + break;
> + }
> + }
> + if (!found)
> return -1;
> - } else {
> - mtf_mask = 0xffffffff;
> +
> + token = strtok(NULL, "|");
> }
>
> - addattr32(n, MAX_MSG, type, htonl(mtf));
> - addattr32(n, MAX_MSG, mask_type, htonl(mtf_mask));
> return 0;
> }
>
> @@ -433,6 +465,8 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
> __be16 vlan_ethtype = 0;
> __u8 ip_proto = 0xff;
> __u32 flags = 0;
> + __u32 mtf = 0;
> + __u32 mtf_mask = 0;
>
> if (handle) {
> ret = get_u32(&t->tcm_handle, handle, 0);
> @@ -462,14 +496,14 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
> return -1;
> }
> addattr_l(n, MAX_MSG, TCA_FLOWER_CLASSID, &handle, 4);
> - } else if (matches(*argv, "matching_flags") == 0) {
> + } else if (matches(*argv, "ip_flags") == 0) {
> NEXT_ARG();
> ret = flower_parse_matching_flags(*argv,
> - TCA_FLOWER_KEY_FLAGS,
> - TCA_FLOWER_KEY_FLAGS_MASK,
> - n);
> + FLOWER_IP_FLAGS,
> + &mtf,
> + &mtf_mask);
> if (ret < 0) {
> - fprintf(stderr, "Illegal \"matching_flags\"\n");
> + fprintf(stderr, "Illegal \"ip_flags\"\n");
> return -1;
> }
> } else if (matches(*argv, "skip_hw") == 0) {
> @@ -723,6 +757,16 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
> parse_done:
> addattr32(n, MAX_MSG, TCA_FLOWER_FLAGS, flags);
>
> + if (mtf_mask) {
> + ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS, htonl(mtf));
> + if (ret)
> + return ret;
> +
> + ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS_MASK, htonl(mtf_mask));
> + if (ret)
> + return ret;
> + }
> +
> ret = addattr16(n, MAX_MSG, TCA_FLOWER_KEY_ETH_TYPE, eth_type);
> if (ret) {
> fprintf(stderr, "Illegal \"eth_type\"(0x%x)\n",
> @@ -828,14 +872,36 @@ static void flower_print_ip_proto(FILE *f, __u8 *p_ip_proto,
> }
>
> static void flower_print_matching_flags(FILE *f, char *name,
> + enum flower_matching_flags type,
> struct rtattr *attr,
> struct rtattr *mask_attr)
> {
> + int i;
> + int count = 0;
> + __u32 mtf;
> + __u32 mtf_mask;
> +
> if (!mask_attr || RTA_PAYLOAD(mask_attr) != 4)
> return;
>
> - fprintf(f, "\n %s 0x%08x/0x%08x", name, ntohl(rta_getattr_u32(attr)),
> - mask_attr ? ntohl(rta_getattr_u32(mask_attr)) : 0xffffffff);
> + mtf = ntohl(rta_getattr_u32(attr));
> + mtf_mask = ntohl(rta_getattr_u32(mask_attr));
> +
> + for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
> + if (type != flags_str[i].type)
> + continue;
> + if (mtf_mask & flags_str[i].flag) {
> + if (++count == 1)
> + fprintf(f, "\n %s ", name);
> + else
> + fprintf(f, "|");
> +
> + if (mtf & flags_str[i].flag)
> + fprintf(f, "%s", flags_str[i].string);
> + else
> + fprintf(f, "no_%s", flags_str[i].string);
> + }
> + }
> }
>
> static void flower_print_ip_addr(FILE *f, char *name, __be16 eth_type,
> @@ -1034,7 +1100,8 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
> flower_print_port(f, "enc_dst_port",
> tb[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
>
> - flower_print_matching_flags(f, "matching_flags",
> + flower_print_matching_flags(f, "ip_flags",
> + FLOWER_IP_FLAGS,
> tb[TCA_FLOWER_KEY_FLAGS],
> tb[TCA_FLOWER_KEY_FLAGS_MASK]);
>
>
Hi,
I'm going to change the delimiter to "/" since "|" might causes some errors.
^ permalink raw reply
* [PATCH iproute2 2/2] tc: flower: Fix incorrect error msg about eth type
From: Roi Dayan @ 2017-01-19 12:31 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, Amir Vadai, Jiri Pirko, Jamal Hadi Salim, Paul Blakey,
Roi Dayan
In-Reply-To: <1484829080-18593-1-git-send-email-roid@mellanox.com>
addattr16 may return an error about the nl msg size
but not about incorrect eth type.
Fixes: 488b41d020fb ("tc: flower no need to specify the ethertype")
Signed-off-by: Roi Dayan <roid@mellanox.com>
Reviewed-by: Paul Blakey <paulb@mellanox.com>
---
tc/f_flower.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 1272a47..314c2dd 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -530,11 +530,8 @@ parse_done:
return ret;
ret = addattr16(n, MAX_MSG, TCA_FLOWER_KEY_ETH_TYPE, eth_type);
- if (ret) {
- fprintf(stderr, "Illegal \"eth_type\"(0x%x)\n",
- ntohs(eth_type));
- return -1;
- }
+ if (ret)
+ return ret;
tail->rta_len = (((void *)n)+n->nlmsg_len) - (void *)tail;
--
1.7.1
^ permalink raw reply related
* [PATCH iproute2 1/2] tc: flower: Add missing err check when parsing flower options
From: Roi Dayan @ 2017-01-19 12:31 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, Amir Vadai, Jiri Pirko, Jamal Hadi Salim, Paul Blakey,
Roi Dayan
In-Reply-To: <1484829080-18593-1-git-send-email-roid@mellanox.com>
addattr32 may return an error.
Fixes: cfcabf18d84a ("tc: flower: Add skip_{hw|sw} support")
Signed-off-by: Roi Dayan <roid@mellanox.com>
Reviewed-by: Paul Blakey <paulb@mellanox.com>
---
tc/f_flower.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 1dbc532..1272a47 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -525,7 +525,9 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
}
parse_done:
- addattr32(n, MAX_MSG, TCA_FLOWER_FLAGS, flags);
+ ret = addattr32(n, MAX_MSG, TCA_FLOWER_FLAGS, flags);
+ if (ret)
+ return ret;
ret = addattr16(n, MAX_MSG, TCA_FLOWER_KEY_ETH_TYPE, eth_type);
if (ret) {
--
1.7.1
^ permalink raw reply related
* [PATCH iproute2 0/2] two small fixes in flower option parser error flow
From: Roi Dayan @ 2017-01-19 12:31 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, Amir Vadai, Jiri Pirko, Jamal Hadi Salim, Paul Blakey,
Roi Dayan
Hi,
the first adds a missing return check and the second removes
an incorrect error msg.
Thanks,
Roi
Roi Dayan (2):
tc: flower: Add missing err check when parsing flower options
tc: flower: Fix incorrect error msg about eth type
tc/f_flower.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)
^ permalink raw reply
* Re: [PATCH] net: ethtool: avoid allocation failure for dump_regs
From: David Arcari @ 2017-01-19 12:35 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20170118.114505.1855687422610394730.davem@davemloft.net>
On 01/18/2017 11:45 AM, David Miller wrote:
> From: David Arcari <darcari@redhat.com>
> Date: Wed, 18 Jan 2017 08:34:05 -0500
>
>> If the user executes 'ethtool -d' for an interface and the associated
>> get_regs_len() function returns 0, the user will see a call trace from
>> the vmalloc() call in ethtool_get_regs(). This patch modifies
>> ethtool_get_regs() to avoid the call to vmalloc when the size is zero.
>>
>> Signed-off-by: David Arcari <darcari@redhat.com>
> I think when the driver indicates this, it is equivalent to saying that
> the operation isn't supported.
>
> Also, this guards us against ->get_regs() methods that don't handle
> zero length requests properly. I see many which are going to do
> really terrible things in that situation.
>
> Therefore, if get_regs_len() returns zero, treat it the safe as if the
> ethtool operations were NULL.
>
> Thanks.
That was actually the fix that I was originally considering, but it
turns out
there is a problem with it.
I found that the vmalloc error was occurring because
ieee80211_get_regs_len() in
net/mac80211/ethtool.c was returning zero. The ieee80211_get_regs in
the same
file returns the hw version. It turns out that this information is used
by the
at76c50x-usb driver in the user space ethtool to report which HW variant
is in
use. Returning an error when regs_len() returns zero would break this
functionality.
-Dave
^ permalink raw reply
* Re: libertas: fix improper return value
From: Kalle Valo @ 2017-01-19 12:39 UTC (permalink / raw)
To: Pan Bian
Cc: Andreas Kemnade, Johannes Berg, libertas-dev, linux-wireless,
netdev, linux-kernel, Pan Bian
In-Reply-To: <1480760857-4549-1-git-send-email-bianpan2016@163.com>
Pan Bian <bianpan2016@163.com> wrote:
> Function lbs_cmd_802_11_sleep_params() always return 0, even if the call
> to lbs_cmd_with_response() fails. In this case, the parameter @sp will
> keep uninitialized. Because the return value is 0, its caller (say
> lbs_sleepparams_read()) will not detect the error, and will copy the
> uninitialized stack memory to user sapce, resulting in stack information
> leak. To avoid the bug, this patch returns variable ret (which takes
> the return value of lbs_cmd_with_response()) instead of 0.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188451
>
> Signed-off-by: Pan Bian <bianpan2016@163.com>
Patch applied to wireless-drivers-next.git, thanks.
259010c509b6 libertas: fix improper return value
--
https://patchwork.kernel.org/patch/9459597/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* RE: [PATCH 2/3] qed: avoid possible stack overflow in qed_ll2_acquire_connection
From: Mintz, Yuval @ 2017-01-19 13:13 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S. Miller, Amrani, Ram, Wei Yongjun, Hannes Reinecke,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Dept-Eng Everest Linux L2
In-Reply-To: <20170118145306.1004008-2-arnd@arndb.de>
> 968,23 +968,19 @@ static int qed_ll2_start_ooo(struct qed_dev *cdev, {
> struct qed_hwfn *hwfn = QED_LEADING_HWFN(cdev);
> u8 *handle = &hwfn->pf_params.iscsi_pf_params.ll2_ooo_queue_id;
> - struct qed_ll2_info *ll2_info;
> + struct qed_ll2_conn ll2_info;
A bit confusing to change the type from qed_ll2_info - > qed_ll2_conn
yet leave the variable named 'll2_info'.
Probably better to have it name `ll2_params' as in:
> @@ -2632,7 +2632,7 @@ static int qed_roce_ll2_start(struct qed_dev *cdev,
> {
> struct qed_hwfn *hwfn = QED_LEADING_HWFN(cdev);
> struct qed_roce_ll2_info *roce_ll2;
> - struct qed_ll2_info ll2_params;
> + struct qed_ll2_conn ll2_params;
But that's nitpicking; You can do this if for some reason you'll need a v2,
or we'll later change it in some semantic patch.
Thanks for taking care of this.
Acked-by: Yuval Mintz <Yuval.Mintz@cavium.com>
^ permalink raw reply
* [PATCH] tcp: initialize max window for a new fastopen socket
From: Alexey Kodanev @ 2017-01-19 13:36 UTC (permalink / raw)
To: David Miller
Cc: netdev, Eric Dumazet, Yuchung Cheng, Neal Cardwell,
Alexey Kodanev
Found that if we run LTP netstress test with large MSS (65K),
the first attempt from server to send data comparable to this
MSS on fastopen connection will be delayed by the probe timer.
Here is an example:
< S seq 0:0 win 43690 options [mss 65495 wscale 7 tfo cookie] length 32
> S. seq 0:0 ack 1 win 43690 options [mss 65495 wscale 7] length 0
< . ack 1 win 342 length 0
Inside tcp_sendmsg(), tcp_send_mss() returns max MSS in 'mss_now',
as well as in 'size_goal'. This results the segment not queued for
transmition until all the data copied from user buffer. Then, inside
__tcp_push_pending_frames(), it breaks on send window test and
continues with the check probe timer.
Fragmentation occurs in tcp_write_wakeup()...
+0.2 > P. seq 1:43777 ack 1 win 342 length 43776
< . ack 43777, win 1365 length 0
> P. seq 43777:65001 ack 1 win 342 options [...] length 21224
...
This also contradicts with the fact that we should bound to the half
of the window if it is large.
Fix this flaw by correctly initializing max_window. Before that, it
could have large values that affect further calculations of 'size_goal'.
Fixes: 168a8f58059a ("tcp: TCP Fast Open Server - main code path")
Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
---
net/ipv4/tcp_fastopen.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index f519195..dd2560c 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -205,6 +205,7 @@ void tcp_fastopen_add_skb(struct sock *sk, struct sk_buff *skb)
* scaled. So correct it appropriately.
*/
tp->snd_wnd = ntohs(tcp_hdr(skb)->window);
+ tp->max_window = tp->snd_wnd;
/* Activate the retrans timer so that SYNACK can be retransmitted.
* The request socket is not added to the ehash
--
1.7.1
^ permalink raw reply related
* Re: [net, 3/6] net: korina: increase tx/rx ring sizes
From: Eric Dumazet @ 2017-01-19 13:37 UTC (permalink / raw)
To: Roman Yeryomin; +Cc: David Miller, netdev
In-Reply-To: <CACiydbJV5djpKdsgnKTDfxHpC0aPhF-bdH=zVzGeo9x127RFJg@mail.gmail.com>
On Thu, 2017-01-19 at 12:55 +0200, Roman Yeryomin wrote:
> On 17 January 2017 at 21:20, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Note that at 100M, 64 rx descriptors have a 8 ms max latency.
> >
> > Switching to 256 also multiply by 4 the latency -> 32 ms latency.
> >
> > Presumably switching to NAPI and GRO would avoid the latency increase
> > and save a lot of cpu cycles for a MIPS board.
> >
>
> Eric, thanks for suggesting GRO, it gives huge performance gain when
> receiving locally (55->95Mbps) and more than 25% gain for NAT
> (55->70Mbps).
> Also reading the datasheet more carefully I see that device rx
> descriptor status flags are interpreted incorrectly. So will provide
> an updated set.
Oh right NAPI for rx is already implemented, so using GRO is a trivial
patch.
^ permalink raw reply
* Re: [PATCH] tcp: initialize max window for a new fastopen socket
From: Eric Dumazet @ 2017-01-19 13:41 UTC (permalink / raw)
To: Alexey Kodanev; +Cc: David Miller, netdev, Yuchung Cheng, Neal Cardwell
In-Reply-To: <1484832999-1849-1-git-send-email-alexey.kodanev@oracle.com>
On Thu, 2017-01-19 at 16:36 +0300, Alexey Kodanev wrote:
> Found that if we run LTP netstress test with large MSS (65K),
> the first attempt from server to send data comparable to this
> MSS on fastopen connection will be delayed by the probe timer.
>
> This also contradicts with the fact that we should bound to the half
> of the window if it is large.
>
> Fix this flaw by correctly initializing max_window. Before that, it
> could have large values that affect further calculations of 'size_goal'.
>
> Fixes: 168a8f58059a ("tcp: TCP Fast Open Server - main code path")
> Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Thanks Alexey !
^ permalink raw reply
* Re: [PATCH] net: fec: Fixed panic problem with non-tso
From: Eric Dumazet @ 2017-01-19 13:24 UTC (permalink / raw)
To: Andy Duan; +Cc: Ashizuka, Yuusuke, netdev@vger.kernel.org, Pravin B Shelar
In-Reply-To: <AM4PR0401MB2260F19C276A1329FD2BB8A7FF7E0@AM4PR0401MB2260.eurprd04.prod.outlook.com>
On Thu, 2017-01-19 at 08:18 +0000, Andy Duan wrote:
> I will double check your fix. Thanks.
>
> And, if driver is to support highmem, then we should add tso highmem
> support in net/core/tso.c, do you think it is necessary ?
Adding TSO highmem support would mean changing net/core/tso.c ABI and
thus changing all net/core/tso.c users.
Looks a lot of work to me.
^ 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