Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 0/3] openvswitch: Add STT support.
From: Tom Herbert @ 2015-02-02 22:49 UTC (permalink / raw)
  To: Jesse Gross; +Cc: Pravin Shelar, David Miller, Linux Netdev List
In-Reply-To: <CAEP_g=_rBmDKDM7fjud78YY=HLnf1r3OdcXux=tm55TxG1DFig@mail.gmail.com>

On Mon, Feb 2, 2015 at 12:39 PM, Jesse Gross <jesse@nicira.com> wrote:
> On Mon, Feb 2, 2015 at 8:23 AM, Tom Herbert <therbert@google.com> wrote:
>>> I would recommend you take a look at the draft if you haven't already:
>>> http://tools.ietf.org/html/draft-davie-stt-06
>>>
>>> It is currently in the final stages of the RFC publication process.
>>
>> Sorry, but this statement is completely wrong and misleading.
>> According to datatracker this draft has been expired since October,
>> there's been no discussion on it in IETF, and this has not gone to
>> IESG. You cannot say this is an IETF standard nor that it is about to
>> be published as one.  See
>> https://datatracker.ietf.org/doc/draft-davie-stt/ and please read the
>> Internet Standards process in RFC2026.
>>
>> I suggest that you update the draft and post it on both nvo3 and tsvwg
>> so there can be some real discussion on the implications of
>> repurposing an IP protocol number and breaking TCP protocol standards.
>
> Seriously, Tom?
>
> It's currently in the ISE queue to be published as an RFC, which you
> can see in the history in the datatracker link. I didn't say that it
> was being published as a standard, I said RFC. This is the same
> process and status that VXLAN has.. It also was presented in nvo3
> before you started coming.
>
> Please get your facts straight before making accusations.

The draft has not come up before the IESG, I have confirmed that with the ADs.

^ permalink raw reply

* Re: [PATCH] net: rocker: Change netdev names to include slot number
From: David Miller @ 2015-02-02 22:16 UTC (permalink / raw)
  To: dsahern; +Cc: sfeldma, netdev
In-Reply-To: <54CFEF20.8060300@gmail.com>

From: David Ahern <dsahern@gmail.com>
Date: Mon, 02 Feb 2015 14:41:52 -0700

> On 2/2/15 2:33 PM, Scott Feldman wrote:
>> On Sun, Feb 1, 2015 at 10:03 PM, David Ahern <dsahern@gmail.com>
>> wrote:
>>> Currently, rocker devices are given eth%d names. If you have multiple
>>> rocker devices it is difficult to easily correlate eth%d names to a
>>> rocker device and port. Change the device name to sw + PCI slot
>>> number + p + id (sw%dp%d). This makes the device names easier to
>>> correlate. ie., Rather than eth0, ..., eth N (N = number of ports in
>>> device) the ports get netdev names like sw5p0, ..., sw5pN.
>>
>> I think udev is the preferred tool for interface naming, rather than
>> hard-coding interface names in the driver.
>>
> 
> hmmm...  What I am seeing right now is a race as to which devices are
> detected first -- rocker or virtio. On half of the boots the virtio
> are detected first and named eth0 and eth1. The other half of the
> boots virtio devices are detected last and become ethN+1 and ethN+2
> (N=number of rocker ports) -- which makes it a PITA to script
> commands. AFAIK udev won't solve that problem.

udev has already tackled this problem, it uses platform specific code
to determine the physical geographic location of devices on the bus,
and uses that to map device names.

^ permalink raw reply

* Re: [PATCH] net: rocker: Change netdev names to include slot number
From: David Miller @ 2015-02-02 22:15 UTC (permalink / raw)
  To: sfeldma; +Cc: dsahern, netdev
In-Reply-To: <CAE4R7bA0Ea9zugm=OR0EN2qSeoiROEoMiYZ-5YMwd9UdS4L0Sg@mail.gmail.com>

From: Scott Feldman <sfeldma@gmail.com>
Date: Mon, 2 Feb 2015 13:33:08 -0800

> On Sun, Feb 1, 2015 at 10:03 PM, David Ahern <dsahern@gmail.com> wrote:
>> Currently, rocker devices are given eth%d names. If you have multiple
>> rocker devices it is difficult to easily correlate eth%d names to a
>> rocker device and port. Change the device name to sw + PCI slot
>> number + p + id (sw%dp%d). This makes the device names easier to
>> correlate. ie., Rather than eth0, ..., eth N (N = number of ports in
>> device) the ports get netdev names like sw5p0, ..., sw5pN.
> 
> I think udev is the preferred tool for interface naming, rather than
> hard-coding interface names in the driver.

That's correct.

^ permalink raw reply

* Re: [PATCH] net: rocker: Change netdev names to include slot number
From: David Ahern @ 2015-02-02 22:09 UTC (permalink / raw)
  To: Florian Fainelli, Scott Feldman; +Cc: Netdev
In-Reply-To: <54CFF420.3090603@gmail.com>

On 2/2/15 3:03 PM, Florian Fainelli wrote:
> Why not? virtio and rocker interfaces are backed by different devices
> drivers which should allow you to use that to name interfaces
> differently. In the case of rocker, you would probably want to read the
> phys_port_id sysfs attribute to name them after their parent switch id too.

[root@f21 ~]# cat /sys/devices/virtual/net/sw5p0/phys_port_id
cat: /sys/devices/virtual/net/sw5p0/phys_port_id: Operation not supported

But in general I guess need to find time to figure out systemd-udev files.

David

^ permalink raw reply

* Re: [PATCH] net: rocker: Change netdev names to include slot number
From: Florian Fainelli @ 2015-02-02 22:03 UTC (permalink / raw)
  To: David Ahern, Scott Feldman; +Cc: Netdev
In-Reply-To: <54CFEF20.8060300@gmail.com>

On 02/02/15 13:41, David Ahern wrote:
> On 2/2/15 2:33 PM, Scott Feldman wrote:
>> On Sun, Feb 1, 2015 at 10:03 PM, David Ahern <dsahern@gmail.com> wrote:
>>> Currently, rocker devices are given eth%d names. If you have multiple
>>> rocker devices it is difficult to easily correlate eth%d names to a
>>> rocker device and port. Change the device name to sw + PCI slot
>>> number + p + id (sw%dp%d). This makes the device names easier to
>>> correlate. ie., Rather than eth0, ..., eth N (N = number of ports in
>>> device) the ports get netdev names like sw5p0, ..., sw5pN.
>>
>> I think udev is the preferred tool for interface naming, rather than
>> hard-coding interface names in the driver.
>>
> 
> 
> hmmm...  What I am seeing right now is a race as to which devices are
> detected first -- rocker or virtio. On half of the boots the virtio are
> detected first and named eth0 and eth1. The other half of the boots
> virtio devices are detected last and become ethN+1 and ethN+2 (N=number
> of rocker ports) -- which makes it a PITA to script commands. AFAIK udev
> won't solve that problem.

Why not? virtio and rocker interfaces are backed by different devices
drivers which should allow you to use that to name interfaces
differently. In the case of rocker, you would probably want to read the
phys_port_id sysfs attribute to name them after their parent switch id too.

> 
> David
> -- 
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Florian

^ permalink raw reply

* Re: [PATCH] net: rocker: Change netdev names to include slot number
From: David Ahern @ 2015-02-02 21:41 UTC (permalink / raw)
  To: Scott Feldman; +Cc: Netdev
In-Reply-To: <CAE4R7bA0Ea9zugm=OR0EN2qSeoiROEoMiYZ-5YMwd9UdS4L0Sg@mail.gmail.com>

On 2/2/15 2:33 PM, Scott Feldman wrote:
> On Sun, Feb 1, 2015 at 10:03 PM, David Ahern <dsahern@gmail.com> wrote:
>> Currently, rocker devices are given eth%d names. If you have multiple
>> rocker devices it is difficult to easily correlate eth%d names to a
>> rocker device and port. Change the device name to sw + PCI slot
>> number + p + id (sw%dp%d). This makes the device names easier to
>> correlate. ie., Rather than eth0, ..., eth N (N = number of ports in
>> device) the ports get netdev names like sw5p0, ..., sw5pN.
>
> I think udev is the preferred tool for interface naming, rather than
> hard-coding interface names in the driver.
>


hmmm...  What I am seeing right now is a race as to which devices are 
detected first -- rocker or virtio. On half of the boots the virtio are 
detected first and named eth0 and eth1. The other half of the boots 
virtio devices are detected last and become ethN+1 and ethN+2 (N=number 
of rocker ports) -- which makes it a PITA to script commands. AFAIK udev 
won't solve that problem.

David

^ permalink raw reply

* Re: [PATCH] net: rocker: Change netdev names to include slot number
From: Scott Feldman @ 2015-02-02 21:33 UTC (permalink / raw)
  To: David Ahern; +Cc: Netdev
In-Reply-To: <1422856985-16530-1-git-send-email-dsahern@gmail.com>

On Sun, Feb 1, 2015 at 10:03 PM, David Ahern <dsahern@gmail.com> wrote:
> Currently, rocker devices are given eth%d names. If you have multiple
> rocker devices it is difficult to easily correlate eth%d names to a
> rocker device and port. Change the device name to sw + PCI slot
> number + p + id (sw%dp%d). This makes the device names easier to
> correlate. ie., Rather than eth0, ..., eth N (N = number of ports in
> device) the ports get netdev names like sw5p0, ..., sw5pN.

I think udev is the preferred tool for interface naming, rather than
hard-coding interface names in the driver.

^ permalink raw reply

* Re: Question: should local address be expired when updating PMTU?
From: David Miller @ 2015-02-02 21:31 UTC (permalink / raw)
  To: shengyong1; +Cc: netdev, yangyingling, steffen.klassert, hannes
In-Reply-To: <54CF3348.40207@huawei.com>

From: shengyong <shengyong1@huawei.com>
Date: Mon, 2 Feb 2015 16:20:24 +0800

> Hi, David Miller

There are other people on this list more skilled than I am at answering
this question, just FYI...

^ permalink raw reply

* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Ben Greear @ 2015-02-02 21:25 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Michal Kazior, linux-wireless, Network Development, eyalpe
In-Reply-To: <1422903136.21689.114.camel@edumazet-glaptop2.roam.corp.google.com>

On 02/02/2015 10:52 AM, Eric Dumazet wrote:
> On Mon, 2015-02-02 at 11:27 +0100, Michal Kazior wrote:
> 
>> While testing I've had my internal GRO patch for ath10k and no stretch
>> ack patches.
> 
> Thanks for the data, I took a look at it.
> 
> I am afraid this GRO patch might be the problem.
> 
> It seems to break ACK clocking badly (linux stack has a somewhat buggy
> tcp_tso_should_defer(), which relies on ACK being received smoothly, as
> no timer is setup to split the TSO packet.)

It is a big throughput win to have fewer TCP ack packets on
wireless since it is a half-duplex environment.  Is there anything
we could improve so that we can have fewer acks and still get
good tcp stack behaviour?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* Re: [PATCH v2] net: bluetooth: hci_sock: Use 'const void *' instead of 'void *' for 2nd parameter of hci_test_bit()
From: Joe Perches @ 2015-02-02 21:20 UTC (permalink / raw)
  To: Chen Gang S
  Cc: marcel-kz+m5ild9QBg9hUCZPvPmw, gustavo-THi1TnShQwVAfugRpC6u6w,
	johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w, David S. Miller,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <54CFE8BE.5030700-/B7AUNIrSHOPt1CcHtbs0g@public.gmane.org>

On Tue, 2015-02-03 at 05:14 +0800, Chen Gang S wrote:
> hci_test_bit() does not modify 2nd parameter, so it is better to let it
> be constant, or may cause build warning. The related warning (with
> allmodconfig under xtensa):
[]
> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
[]
> @@ -46,7 +46,7 @@ struct hci_pinfo {
>  	unsigned short    channel;
>  };
>  
> -static inline int hci_test_bit(int nr, void *addr)
> +static inline int hci_test_bit(int nr, const void *addr)
>  {
>  	return *((__u32 *) addr + (nr >> 5)) & ((__u32) 1 << (nr & 31));
>  }

It's probably better to use const __u32 * here too, but the
real thing I wonder is whether or not there's an issue with
one of the 2 uses of this function.

One of them passes a unsigned long *, the other a u32 *.

$ git grep -w hci_test_bit
net/bluetooth/hci_sock.c:static inline int hci_test_bit(int nr, void *addr)
net/bluetooth/hci_sock.c:       if (!hci_test_bit(flt_event, &flt->event_mask))
net/bluetooth/hci_sock.c:                    !hci_test_bit(ocf & HCI_FLT_OCF_BITS,
net/bluetooth/hci_sock.c-                                  &hci_sec_filter.ocf_mask[ogf])) &&

hci_sec_filter.ocf_mask is __u32
but flt->event_mask is unsigned long.

Any possible issue here on 64-bit systems?

---

$ git grep -A4 "struct hci_filter {"
include/net/bluetooth/hci_sock.h:struct hci_filter {
include/net/bluetooth/hci_sock.h-       unsigned long type_mask;
include/net/bluetooth/hci_sock.h-       unsigned long event_mask[2];
include/net/bluetooth/hci_sock.h-       __le16 opcode;
include/net/bluetooth/hci_sock.h-};

---

static bool is_filtered_packet(struct sock *sk, struct sk_buff *skb)
{
	struct hci_filter *flt;
[...]
	if (!hci_test_bit(flt_event, &flt->event_mask))
		return true;

^ permalink raw reply

* [PATCH v2] net: bluetooth: hci_sock: Use 'const void *' instead of 'void *' for 2nd parameter of hci_test_bit()
From: Chen Gang S @ 2015-02-02 21:14 UTC (permalink / raw)
  To: marcel-kz+m5ild9QBg9hUCZPvPmw, gustavo-THi1TnShQwVAfugRpC6u6w,
	johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w
  Cc: David S. Miller, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

hci_test_bit() does not modify 2nd parameter, so it is better to let it
be constant, or may cause build warning. The related warning (with
allmodconfig under xtensa):

  net/bluetooth/hci_sock.c: In function 'hci_sock_sendmsg':
  net/bluetooth/hci_sock.c:955:8: warning: passing argument 2 of 'hci_test_bit' discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
          &hci_sec_filter.ocf_mask[ogf])) &&
          ^
  net/bluetooth/hci_sock.c:49:19: note: expected 'void *' but argument is of type 'const __u32 (*)[4] {aka const unsigned int (*)[4]}'
   static inline int hci_test_bit(int nr, void *addr)
                     ^

Signed-off-by: Chen Gang <gang.chen.5i5j-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 net/bluetooth/hci_sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 80c5a79..858b53a 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -46,7 +46,7 @@ struct hci_pinfo {
 	unsigned short    channel;
 };
 
-static inline int hci_test_bit(int nr, void *addr)
+static inline int hci_test_bit(int nr, const void *addr)
 {
 	return *((__u32 *) addr + (nr >> 5)) & ((__u32) 1 << (nr & 31));
 }
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH net-next 2/8] cxgb4: Added support in debugfs to display tp_la stats
From: David Miller @ 2015-02-02 21:14 UTC (permalink / raw)
  To: hariprasad; +Cc: netdev, leedom, anish, nirranjan, praveenm
In-Reply-To: <1422888789-12016-3-git-send-email-hariprasad@chelsio.com>

From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Mon,  2 Feb 2015 20:23:03 +0530

> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>

Ok I've seen enough.

You guys really, truly, abuse debugfs.  So I'm putting my foot down now.

Stats like this can be exported through traditional means such as via
ethtool.

^ permalink raw reply

* Re: [PATCH v3 0/3] Restore UFO support to virtio_net devices
From: David Miller @ 2015-02-02 21:11 UTC (permalink / raw)
  To: vyasevich; +Cc: eric.dumazet, mst, netdev, virtualization, hannes, ben
In-Reply-To: <1422889269-16007-1-git-send-email-vyasevic@redhat.com>


Vlad, this still fails the same way.

[davem@dokdo net]$ make -s -j8
kernel/Makefile:132: *** No X.509 certificates found ***
kernel/Makefile:132: *** No X.509 certificates found ***
net/built-in.o: In function `udp6_ufo_fragment':
udp_offload.c:(.text+0x103514): undefined reference to `ipv6_select_ident'
make: *** [vmlinux] Error 1

You're putting ipv6_select_ident() into the ipv6 module via
ip6_output.c, but that means that code like udp_offload.c which is
built statically into the kernel can't see the symbol.

Please build allmodconfig, that is the exact build I use to test your
and everyone else's changes.

^ permalink raw reply

* Per-connection tcp_retries2 and RFC 1122 compliance
From: John Eckersberg @ 2015-02-02 21:05 UTC (permalink / raw)
  To: netdev

Greetings,

RFC 1122, section 4.2.3.5 "TCP Connection Failures", states:

  (d)  An application MUST be able to set the value for R2 for
       a particular connection.  For example, an interactive
       application might set R2 to "infinity," giving the user
       control over when to disconnect.

The R2 value referenced above is implemented as the tcp_retries2 sysctl.
However it seems that the only way to tune that value is via the global
sysctl knob.  In other words, there is no provided way to set it only
for a particular connection as RFC 1122 requires.

Could someone confirm that this is a legitimate bug/deficiency?  Or am I
just missing something?  If this is a real bug, I would be willing to
put a patch together to fix it although I will probably require some
handholding (this would be my first contribution to the kernel).

Thanks,
John

^ permalink raw reply

* [PATCH] net: usb: sr9700: Use 'SR_' prefix for the common register macros
From: Chen Gang S @ 2015-02-02 21:00 UTC (permalink / raw)
  To: linux-usb, netdev, linux-kernel@vger.kernel.org

The commone register macors (e.g. RSR) is too commont to drivers, it may
be conflict with the architectures (e.g. xtensa, sh).

The related warnings (with allmodconfig under xtensa):

    CC [M]  drivers/net/usb/sr9700.o
  In file included from drivers/net/usb/sr9700.c:24:0:
  drivers/net/usb/sr9700.h:65:0: warning: "RSR" redefined
   #define RSR   0x06
   ^
  In file included from ./arch/xtensa/include/asm/bitops.h:22:0,
                   from include/linux/bitops.h:36,
                   from include/linux/kernel.h:10,
                   from include/linux/list.h:8,
                   from include/linux/module.h:9,
                   from drivers/net/usb/sr9700.c:13:
  ./arch/xtensa/include/asm/processor.h:190:0: note: this is the location of the previous definition
   #define RSR(v,sr) __asm__ __volatile__ ("rsr %0,"__stringify(sr) : "=a"(v));
   ^

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 drivers/net/usb/sr9700.c | 36 +++++++++++++-------------
 drivers/net/usb/sr9700.h | 66 ++++++++++++++++++++++++------------------------
 2 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c
index 99b69af..4a1e9c4 100644
--- a/drivers/net/usb/sr9700.c
+++ b/drivers/net/usb/sr9700.c
@@ -77,7 +77,7 @@ static int wait_phy_eeprom_ready(struct usbnet *dev, int phy)
 		int ret;
 
 		udelay(1);
-		ret = sr_read_reg(dev, EPCR, &tmp);
+		ret = sr_read_reg(dev, SR_EPCR, &tmp);
 		if (ret < 0)
 			return ret;
 
@@ -98,15 +98,15 @@ static int sr_share_read_word(struct usbnet *dev, int phy, u8 reg,
 
 	mutex_lock(&dev->phy_mutex);
 
-	sr_write_reg(dev, EPAR, phy ? (reg | EPAR_PHY_ADR) : reg);
-	sr_write_reg(dev, EPCR, phy ? (EPCR_EPOS | EPCR_ERPRR) : EPCR_ERPRR);
+	sr_write_reg(dev, SR_EPAR, phy ? (reg | EPAR_PHY_ADR) : reg);
+	sr_write_reg(dev, SR_EPCR, phy ? (EPCR_EPOS | EPCR_ERPRR) : EPCR_ERPRR);
 
 	ret = wait_phy_eeprom_ready(dev, phy);
 	if (ret < 0)
 		goto out_unlock;
 
-	sr_write_reg(dev, EPCR, 0x0);
-	ret = sr_read(dev, EPDR, 2, value);
+	sr_write_reg(dev, SR_EPCR, 0x0);
+	ret = sr_read(dev, SR_EPDR, 2, value);
 
 	netdev_dbg(dev->net, "read shared %d 0x%02x returned 0x%04x, %d\n",
 		   phy, reg, *value, ret);
@@ -123,19 +123,19 @@ static int sr_share_write_word(struct usbnet *dev, int phy, u8 reg,
 
 	mutex_lock(&dev->phy_mutex);
 
-	ret = sr_write(dev, EPDR, 2, &value);
+	ret = sr_write(dev, SR_EPDR, 2, &value);
 	if (ret < 0)
 		goto out_unlock;
 
-	sr_write_reg(dev, EPAR, phy ? (reg | EPAR_PHY_ADR) : reg);
-	sr_write_reg(dev, EPCR, phy ? (EPCR_WEP | EPCR_EPOS | EPCR_ERPRW) :
+	sr_write_reg(dev, SR_EPAR, phy ? (reg | EPAR_PHY_ADR) : reg);
+	sr_write_reg(dev, SR_EPCR, phy ? (EPCR_WEP | EPCR_EPOS | EPCR_ERPRW) :
 		    (EPCR_WEP | EPCR_ERPRW));
 
 	ret = wait_phy_eeprom_ready(dev, phy);
 	if (ret < 0)
 		goto out_unlock;
 
-	sr_write_reg(dev, EPCR, 0x0);
+	sr_write_reg(dev, SR_EPCR, 0x0);
 
 out_unlock:
 	mutex_unlock(&dev->phy_mutex);
@@ -188,7 +188,7 @@ static int sr_mdio_read(struct net_device *netdev, int phy_id, int loc)
 	if (loc == MII_BMSR) {
 		u8 value;
 
-		sr_read_reg(dev, NSR, &value);
+		sr_read_reg(dev, SR_NSR, &value);
 		if (value & NSR_LINKST)
 			rc = 1;
 	}
@@ -228,7 +228,7 @@ static u32 sr9700_get_link(struct net_device *netdev)
 	int rc = 0;
 
 	/* Get the Link Status directly */
-	sr_read_reg(dev, NSR, &value);
+	sr_read_reg(dev, SR_NSR, &value);
 	if (value & NSR_LINKST)
 		rc = 1;
 
@@ -281,8 +281,8 @@ static void sr9700_set_multicast(struct net_device *netdev)
 		}
 	}
 
-	sr_write_async(dev, MAR, SR_MCAST_SIZE, hashes);
-	sr_write_reg_async(dev, RCR, rx_ctl);
+	sr_write_async(dev, SR_MAR, SR_MCAST_SIZE, hashes);
+	sr_write_reg_async(dev, SR_RCR, rx_ctl);
 }
 
 static int sr9700_set_mac_address(struct net_device *netdev, void *p)
@@ -297,7 +297,7 @@ static int sr9700_set_mac_address(struct net_device *netdev, void *p)
 	}
 
 	memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);
-	sr_write_async(dev, PAR, 6, netdev->dev_addr);
+	sr_write_async(dev, SR_PAR, 6, netdev->dev_addr);
 
 	return 0;
 }
@@ -340,7 +340,7 @@ static int sr9700_bind(struct usbnet *dev, struct usb_interface *intf)
 	mii->phy_id_mask = 0x1f;
 	mii->reg_num_mask = 0x1f;
 
-	sr_write_reg(dev, NCR, NCR_RST);
+	sr_write_reg(dev, SR_NCR, NCR_RST);
 	udelay(20);
 
 	/* read MAC
@@ -348,17 +348,17 @@ static int sr9700_bind(struct usbnet *dev, struct usb_interface *intf)
 	 * EEPROM automatically to PAR. In case there is no EEPROM externally,
 	 * a default MAC address is stored in PAR for making chip work properly.
 	 */
-	if (sr_read(dev, PAR, ETH_ALEN, netdev->dev_addr) < 0) {
+	if (sr_read(dev, SR_PAR, ETH_ALEN, netdev->dev_addr) < 0) {
 		netdev_err(netdev, "Error reading MAC address\n");
 		ret = -ENODEV;
 		goto out;
 	}
 
 	/* power up and reset phy */
-	sr_write_reg(dev, PRR, PRR_PHY_RST);
+	sr_write_reg(dev, SR_PRR, PRR_PHY_RST);
 	/* at least 10ms, here 20ms for safe */
 	mdelay(20);
-	sr_write_reg(dev, PRR, 0);
+	sr_write_reg(dev, SR_PRR, 0);
 	/* at least 1ms, here 2ms for reading right register */
 	udelay(2 * 1000);
 
diff --git a/drivers/net/usb/sr9700.h b/drivers/net/usb/sr9700.h
index fd687c5..258b030 100644
--- a/drivers/net/usb/sr9700.h
+++ b/drivers/net/usb/sr9700.h
@@ -14,13 +14,13 @@
 /* sr9700 spec. register table on Linux platform */
 
 /* Network Control Reg */
-#define	NCR			0x00
+#define	SR_NCR			0x00
 #define		NCR_RST			(1 << 0)
 #define		NCR_LBK			(3 << 1)
 #define		NCR_FDX			(1 << 3)
 #define		NCR_WAKEEN		(1 << 6)
 /* Network Status Reg */
-#define	NSR			0x01
+#define	SR_NSR			0x01
 #define		NSR_RXRDY		(1 << 0)
 #define		NSR_RXOV		(1 << 1)
 #define		NSR_TX1END		(1 << 2)
@@ -30,7 +30,7 @@
 #define		NSR_LINKST		(1 << 6)
 #define		NSR_SPEED		(1 << 7)
 /* Tx Control Reg */
-#define	TCR			0x02
+#define	SR_TCR			0x02
 #define		TCR_CRC_DIS		(1 << 1)
 #define		TCR_PAD_DIS		(1 << 2)
 #define		TCR_LC_CARE		(1 << 3)
@@ -38,7 +38,7 @@
 #define		TCR_EXCECM		(1 << 5)
 #define		TCR_LF_EN		(1 << 6)
 /* Tx Status Reg for Packet Index 1 */
-#define	TSR1		0x03
+#define	SR_TSR1		0x03
 #define		TSR1_EC			(1 << 2)
 #define		TSR1_COL		(1 << 3)
 #define		TSR1_LC			(1 << 4)
@@ -46,7 +46,7 @@
 #define		TSR1_LOC		(1 << 6)
 #define		TSR1_TLF		(1 << 7)
 /* Tx Status Reg for Packet Index 2 */
-#define	TSR2		0x04
+#define	SR_TSR2		0x04
 #define		TSR2_EC			(1 << 2)
 #define		TSR2_COL		(1 << 3)
 #define		TSR2_LC			(1 << 4)
@@ -54,7 +54,7 @@
 #define		TSR2_LOC		(1 << 6)
 #define		TSR2_TLF		(1 << 7)
 /* Rx Control Reg*/
-#define	RCR			0x05
+#define	SR_RCR			0x05
 #define		RCR_RXEN		(1 << 0)
 #define		RCR_PRMSC		(1 << 1)
 #define		RCR_RUNT		(1 << 2)
@@ -62,87 +62,87 @@
 #define		RCR_DIS_CRC		(1 << 4)
 #define		RCR_DIS_LONG	(1 << 5)
 /* Rx Status Reg */
-#define	RSR			0x06
+#define	SR_RSR			0x06
 #define		RSR_AE			(1 << 2)
 #define		RSR_MF			(1 << 6)
 #define		RSR_RF			(1 << 7)
 /* Rx Overflow Counter Reg */
-#define	ROCR		0x07
+#define	SR_ROCR		0x07
 #define		ROCR_ROC		(0x7F << 0)
 #define		ROCR_RXFU		(1 << 7)
 /* Back Pressure Threshold Reg */
-#define	BPTR		0x08
+#define	SR_BPTR		0x08
 #define		BPTR_JPT		(0x0F << 0)
 #define		BPTR_BPHW		(0x0F << 4)
 /* Flow Control Threshold Reg */
-#define	FCTR		0x09
+#define	SR_FCTR		0x09
 #define		FCTR_LWOT		(0x0F << 0)
 #define		FCTR_HWOT		(0x0F << 4)
 /* rx/tx Flow Control Reg */
-#define	FCR			0x0A
+#define	SR_FCR			0x0A
 #define		FCR_FLCE		(1 << 0)
 #define		FCR_BKPA		(1 << 4)
 #define		FCR_TXPEN		(1 << 5)
 #define		FCR_TXPF		(1 << 6)
 #define		FCR_TXP0		(1 << 7)
 /* Eeprom & Phy Control Reg */
-#define	EPCR		0x0B
+#define	SR_EPCR		0x0B
 #define		EPCR_ERRE		(1 << 0)
 #define		EPCR_ERPRW		(1 << 1)
 #define		EPCR_ERPRR		(1 << 2)
 #define		EPCR_EPOS		(1 << 3)
 #define		EPCR_WEP		(1 << 4)
 /* Eeprom & Phy Address Reg */
-#define	EPAR		0x0C
+#define	SR_EPAR		0x0C
 #define		EPAR_EROA		(0x3F << 0)
 #define		EPAR_PHY_ADR_MASK	(0x03 << 6)
 #define		EPAR_PHY_ADR		(0x01 << 6)
 /* Eeprom &	Phy Data Reg */
-#define	EPDR		0x0D	/* 0x0D ~ 0x0E for Data Reg Low & High */
+#define	SR_EPDR		0x0D	/* 0x0D ~ 0x0E for Data Reg Low & High */
 /* Wakeup Control Reg */
-#define	WCR			0x0F
+#define	SR_WCR			0x0F
 #define		WCR_MAGICST		(1 << 0)
 #define		WCR_LINKST		(1 << 2)
 #define		WCR_MAGICEN		(1 << 3)
 #define		WCR_LINKEN		(1 << 5)
 /* Physical Address Reg */
-#define	PAR			0x10	/* 0x10 ~ 0x15 6 bytes for PAR */
+#define	SR_PAR			0x10	/* 0x10 ~ 0x15 6 bytes for PAR */
 /* Multicast Address Reg */
-#define	MAR			0x16	/* 0x16 ~ 0x1D 8 bytes for MAR */
+#define	SR_MAR			0x16	/* 0x16 ~ 0x1D 8 bytes for MAR */
 /* 0x1e unused */
 /* Phy Reset Reg */
-#define	PRR			0x1F
+#define	SR_PRR			0x1F
 #define		PRR_PHY_RST		(1 << 0)
 /* Tx sdram Write Pointer Address Low */
-#define	TWPAL		0x20
+#define	SR_TWPAL		0x20
 /* Tx sdram Write Pointer Address High */
-#define	TWPAH		0x21
+#define	SR_TWPAH		0x21
 /* Tx sdram Read Pointer Address Low */
-#define	TRPAL		0x22
+#define	SR_TRPAL		0x22
 /* Tx sdram Read Pointer Address High */
-#define	TRPAH		0x23
+#define	SR_TRPAH		0x23
 /* Rx sdram Write Pointer Address Low */
-#define	RWPAL		0x24
+#define	SR_RWPAL		0x24
 /* Rx sdram Write Pointer Address High */
-#define	RWPAH		0x25
+#define	SR_RWPAH		0x25
 /* Rx sdram Read Pointer Address Low */
-#define	RRPAL		0x26
+#define	SR_RRPAL		0x26
 /* Rx sdram Read Pointer Address High */
-#define	RRPAH		0x27
+#define	SR_RRPAH		0x27
 /* Vendor ID register */
-#define	VID			0x28	/* 0x28 ~ 0x29 2 bytes for VID */
+#define	SR_VID			0x28	/* 0x28 ~ 0x29 2 bytes for VID */
 /* Product ID register */
-#define	PID			0x2A	/* 0x2A ~ 0x2B 2 bytes for PID */
+#define	SR_PID			0x2A	/* 0x2A ~ 0x2B 2 bytes for PID */
 /* CHIP Revision register */
-#define	CHIPR		0x2C
+#define	SR_CHIPR		0x2C
 /* 0x2D --> 0xEF unused */
 /* USB Device Address */
-#define	USBDA		0xF0
+#define	SR_USBDA		0xF0
 #define		USBDA_USBFA		(0x7F << 0)
 /* RX packet Counter Reg */
-#define	RXC			0xF1
+#define	SR_RXC			0xF1
 /* Tx packet Counter & USB Status Reg */
-#define	TXC_USBS	0xF2
+#define	SR_TXC_USBS		0xF2
 #define		TXC_USBS_TXC0		(1 << 0)
 #define		TXC_USBS_TXC1		(1 << 1)
 #define		TXC_USBS_TXC2		(1 << 2)
@@ -150,7 +150,7 @@
 #define		TXC_USBS_SUSFLAG	(1 << 6)
 #define		TXC_USBS_RXFAULT	(1 << 7)
 /* USB Control register */
-#define	USBC		0xF4
+#define	SR_USBC			0xF4
 #define		USBC_EP3NAK		(1 << 4)
 #define		USBC_EP3ACK		(1 << 5)
 
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH] bluetooth: hci_sock: Use type cast "(void *)" to avoid building warnings
From: Chen Gang S @ 2015-02-02 21:07 UTC (permalink / raw)
  To: Joe Perches
  Cc: marcel, gustavo, johan.hedberg, David S. Miller, linux-bluetooth,
	netdev@vger.kernel.org
In-Reply-To: <1422909598.30476.20.camel@perches.com>

On 2/3/15 04:39, Joe Perches wrote:
> On Tue, 2015-02-03 at 04:37 +0800, Chen Gang S wrote:
>> The related warning (with allmodconfig under xtensa):
> []
>> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> []
>> @@ -952,7 +952,7 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
>>  
>>  		if (((ogf > HCI_SFLT_MAX_OGF) ||
>>  		     !hci_test_bit(ocf & HCI_FLT_OCF_BITS,
>> -				   &hci_sec_filter.ocf_mask[ogf])) &&
>> +				   (void *)&hci_sec_filter.ocf_mask[ogf])) &&
>>  		    !capable(CAP_NET_RAW)) {
>>  			err = -EPERM;
>>  			goto drop;
> 
> Probably better to change the hci_test_bit to take const void *
> 
> static inline int hci_test_bit(int nr, void *addr)
> {
> 	return *((__u32 *) addr + (nr >> 5)) & ((__u32) 1 << (nr & 31));
> }
> 

Yeah, thanks. It should be fixed like what you said above, I shall send
patch v2 for it.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

^ permalink raw reply

* Re: [PATCH net-next 5/6] udpv6: Add lockless sendmsg() support
From: Vlad Yasevich @ 2015-02-02 20:42 UTC (permalink / raw)
  To: Sergei Shtylyov, netdev; +Cc: herbert, hannes, Vladislav Yasevich
In-Reply-To: <54CD3FF4.1030007@cogentembedded.com>

On 01/31/2015 03:49 PM, Sergei Shtylyov wrote:
> Hello.
> 
> On 1/31/2015 6:40 PM, Vladislav Yasevich wrote:
> 
>> This commit adds the same functionaliy to IPv6 that
>> commit 903ab86d195cca295379699299c5fc10beba31c7
>> Author: Herbert Xu <herbert@gondor.apana.org.au>
>> Date:   Tue Mar 1 02:36:48 2011 +0000
> 
>>      udp: Add lockless transmit path
> 
>> added to IPv4.
> 
>> UDP transmit path can now run without a socket lock,
>> thus allowing multiple threads to send to a single socket
>> more efficiently.
>> This is only used when corking/MSG_MORE is not used.
> 
>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>> ---
>>   net/ipv6/udp.c | 24 ++++++++++++++++++++----
>>   1 file changed, 20 insertions(+), 4 deletions(-)
> 
>> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
>> index 67a3d70..d048d46 100644
>> --- a/net/ipv6/udp.c
>> +++ b/net/ipv6/udp.c
> [...]
>> @@ -1307,6 +1308,20 @@ do_udp_sendmsg:
>>           goto do_confirm;
>>   back_from_confirm:
>>
>> +    /* Lockless fast path for the non-corking case */
>> +    if (!corkreq) {
>> +        struct sk_buff *skb;
>>
>> +        skb = ip6_make_skb(sk, getfrag, msg, ulen,
>> +                   sizeof(struct udphdr), hlimit, tclass, opt,
>> +                   &fl6, (struct rt6_info *)dst,
>> +                   msg->msg_flags, dontfrag);
>> +        err = PTR_ERR(skb);
> 
>    You should use PTR_ERR_OR_ZERO() here, I think.
> 

That particular code was stolen from ipv4/udp.c.  You are
right, we can use PTR_ERR_OR_ZERO() and simplify the following
check as well.

Will fix.

Thanks
-vlad

> [...]
> 
> WBR, Sergei
> 

^ permalink raw reply

* Re: [PATCH net-next 0/3] openvswitch: Add STT support.
From: Jesse Gross @ 2015-02-02 20:39 UTC (permalink / raw)
  To: Tom Herbert; +Cc: Pravin Shelar, David Miller, Linux Netdev List
In-Reply-To: <CA+mtBx8mQ4ai3mbY4=HrKjximaEe-+ui2uEqeunQ3kOG=YNBpA@mail.gmail.com>

On Mon, Feb 2, 2015 at 8:23 AM, Tom Herbert <therbert@google.com> wrote:
>> I would recommend you take a look at the draft if you haven't already:
>> http://tools.ietf.org/html/draft-davie-stt-06
>>
>> It is currently in the final stages of the RFC publication process.
>
> Sorry, but this statement is completely wrong and misleading.
> According to datatracker this draft has been expired since October,
> there's been no discussion on it in IETF, and this has not gone to
> IESG. You cannot say this is an IETF standard nor that it is about to
> be published as one.  See
> https://datatracker.ietf.org/doc/draft-davie-stt/ and please read the
> Internet Standards process in RFC2026.
>
> I suggest that you update the draft and post it on both nvo3 and tsvwg
> so there can be some real discussion on the implications of
> repurposing an IP protocol number and breaking TCP protocol standards.

Seriously, Tom?

It's currently in the ISE queue to be published as an RFC, which you
can see in the history in the datatracker link. I didn't say that it
was being published as a standard, I said RFC. This is the same
process and status that VXLAN has.. It also was presented in nvo3
before you started coming.

Please get your facts straight before making accusations.

^ permalink raw reply

* Re: [PATCH] bluetooth: hci_sock: Use type cast "(void *)" to avoid building warnings
From: Joe Perches @ 2015-02-02 20:39 UTC (permalink / raw)
  To: Chen Gang S
  Cc: marcel, gustavo, johan.hedberg, David S. Miller, linux-bluetooth,
	netdev@vger.kernel.org
In-Reply-To: <54CFE01D.5090309@sunrus.com.cn>

On Tue, 2015-02-03 at 04:37 +0800, Chen Gang S wrote:
> The related warning (with allmodconfig under xtensa):
[]
> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
[]
> @@ -952,7 +952,7 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
>  
>  		if (((ogf > HCI_SFLT_MAX_OGF) ||
>  		     !hci_test_bit(ocf & HCI_FLT_OCF_BITS,
> -				   &hci_sec_filter.ocf_mask[ogf])) &&
> +				   (void *)&hci_sec_filter.ocf_mask[ogf])) &&
>  		    !capable(CAP_NET_RAW)) {
>  			err = -EPERM;
>  			goto drop;

Probably better to change the hci_test_bit to take const void *

static inline int hci_test_bit(int nr, void *addr)
{
	return *((__u32 *) addr + (nr >> 5)) & ((__u32) 1 << (nr & 31));
}

^ permalink raw reply

* [PATCH] bluetooth: hci_sock: Use type cast "(void *)" to avoid building warnings
From: Chen Gang S @ 2015-02-02 20:37 UTC (permalink / raw)
  To: marcel-kz+m5ild9QBg9hUCZPvPmw, gustavo-THi1TnShQwVAfugRpC6u6w,
	johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w, David S. Miller
  Cc: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

The related warning (with allmodconfig under xtensa):

  net/bluetooth/hci_sock.c: In function 'hci_sock_sendmsg':
  net/bluetooth/hci_sock.c:955:8: warning: passing argument 2 of 'hci_test_bit' discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
          &hci_sec_filter.ocf_mask[ogf])) &&
          ^
  net/bluetooth/hci_sock.c:49:19: note: expected 'void *' but argument is of type 'const __u32 (*)[4] {aka const unsigned int (*)[4]}'
   static inline int hci_test_bit(int nr, void *addr)
                     ^

Signed-off-by: Chen Gang <gang.chen.5i5j-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 net/bluetooth/hci_sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 1d65c5b..80c5a79 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -952,7 +952,7 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 
 		if (((ogf > HCI_SFLT_MAX_OGF) ||
 		     !hci_test_bit(ocf & HCI_FLT_OCF_BITS,
-				   &hci_sec_filter.ocf_mask[ogf])) &&
+				   (void *)&hci_sec_filter.ocf_mask[ogf])) &&
 		    !capable(CAP_NET_RAW)) {
 			err = -EPERM;
 			goto drop;
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCHv3, ipsec-next] xfrm: Do not parse 32bits compiled xfrm netlink msg on 64bits host
From: David Miller @ 2015-02-02 19:45 UTC (permalink / raw)
  To: nicolas.dichtel
  Cc: steffen.klassert, fan.du, herbert, netdev, fengyuleidian0615
In-Reply-To: <54CF3D3A.5040607@6wind.com>

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Mon, 02 Feb 2015 10:02:50 +0100

> Le 02/02/2015 09:44, Steffen Klassert a écrit :
>> On Thu, Jan 29, 2015 at 11:29:51AM +0100, Nicolas Dichtel wrote:
> [snip]
>>>
>>> The point I try to make is that patching userland apps allows to use
>>> xfrm on a
>>> 32bits userland / 64bits kernel.
>>
>> Ugh, I did not know that this is used that way. Which applications do
>> this?
>> So the situation is worse than I thought. What happens to such
>> applications
>> if we add a compat layer in the kernel? I'd guess they will break,
>> right?
>
> A compat layer will be perfect. I just wanted to highlight the fact
> that without this patch, it's possible to have a workaround to use
> netlink-xfrm and after it, it will be impossible.

Just a little history, there was a case where we tried to work around this
in userspace by messing with the structure definitions when building
the userland binaries, and that completely exploded.  This was with
the wireless extensions about 15 years ago.

If you work around it in userspace, then you can't fix the kernel to
do the right thing without potentially breaking things again for
the work around binaries that have been created.

^ permalink raw reply

* Re: [PATCH net-next 1/2] pkt_sched: fq: avoid artificial bursts for clocked flows
From: Eric Dumazet @ 2015-02-02 19:24 UTC (permalink / raw)
  To: Kenneth Klette Jonassen; +Cc: netdev
In-Reply-To: <1422903556-30393-1-git-send-email-kennetkl@ifi.uio.no>

On Mon, 2015-02-02 at 19:59 +0100, Kenneth Klette Jonassen wrote:
> Current pacing behavior always throttle flows for a time equal to one full
> quantum, starting at the instance in time when a flow depletes its credit.
> This is optimal for burst sizes that are a multiple of the chosen quantum.
> 
> For flows with many small and evenly clocked packets, the depletion and
> refilling of credits cause packets to queue and transmit in bursts, even
> when their clocked rate is below the pacing rate. With TCP ACKs, this
> artificial queueing induces significant noise to RTTs, e.g. up to 2.07 ms
> for rtt 20 ms, cwnd 10 and quantum 3028.
> 
> Packetdrill script to illustrate bursts:
> 0.000 socket(..., SOCK_DGRAM, IPPROTO_UDP) = 3
> 0.000 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> 0.000 bind(3, ..., ...) = 0
> 0.000 connect(3, ..., ...) = 0
> 
> // SO_MAX_PACING_RATE: 2500 Bps, 100 ms per quantum, 20 ms per 50B packet.
> 0.000 setsockopt(3, SOL_SOCKET, 47, [2500], 4) = 0
> 0.000 `tc qdisc add dev tun0 root fq initial_quantum 250 quantum 250`
> 
> // Use 200 credits: send four perfectly spaced 50 byte packets.
> 0.000 write(3, ..., 22) = 22
> 0.000 > udp (22)
> 0.020 write(3, ..., 22) = 22
> 0.020 > udp (22)
> 0.040 write(3, ..., 22) = 22
> 0.040 > udp (22)
> 0.060 write(3, ..., 22) = 22
> 0.060 > udp (22)


We do not want to perfectly space packets, but have an efficient packet
scheduler, allowing TCP pacing.

I chose to not use ktime_get() in enqueue() when I wrote sch_fq.

A Token Bucket Filter has the notion of quantum, meaning you configure
the granularity given this quantum.

At Google, we have a special handling for TCP ACK packets, so that they
do not interfere with FQ/pacing.

ACK packets are not paced, ever.

This patch also allows skb->ooo_okay being set even if the DATA packet
immediately follows a train of ACK packets (as in typical RPC patterns)

^ permalink raw reply

* [PATCH net-next 2/2] pkt_sched: fq: remove redundant flow credit refill
From: Kenneth Klette Jonassen @ 2015-02-02 18:59 UTC (permalink / raw)
  To: netdev; +Cc: Kenneth Klette Jonassen
In-Reply-To: <1422903556-30393-1-git-send-email-kennetkl@ifi.uio.no>

Current behavior explicitly refills flow credit after idle. But following
the first patch in this series, regular refill no longer throttles a flow
if idle_time >= quantum_time.

Remove redundant refill, and warn possible users of the refill delay knob.

Updates f52ed89971ad ("pkt_sched: fq: fix pacing for small frames").
Inspired by 65c5189a2b57 ("pkt_sched: fq: warn users using defrate").

Signed-off-by: Kenneth Klette Jonassen <kennetkl@ifi.uio.no>
---
 include/uapi/linux/pkt_sched.h |  2 +-
 net/sched/sch_fq.c             | 20 ++++++--------------
 2 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index d62316b..5a9afb4 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -772,7 +772,7 @@ enum {
 
 	TCA_FQ_BUCKETS_LOG,	/* log2(number of buckets) */
 
-	TCA_FQ_FLOW_REFILL_DELAY,	/* flow credit refill delay in usec */
+	TCA_FQ_FLOW_REFILL_DELAY,	/* obsolete, do not use */
 
 	__TCA_FQ_MAX
 };
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 6f0c45e..81695ac 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -90,7 +90,6 @@ struct fq_sched_data {
 	struct fq_flow	internal;	/* for non classified or high prio packets */
 	u32		quantum;
 	u32		initial_quantum;
-	u32		flow_refill_delay;
 	u32		flow_max_rate;	/* optional max rate per flow */
 	u32		flow_plimit;	/* max packets per flow */
 	struct rb_root	*fq_root;
@@ -377,10 +376,6 @@ static int fq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	qdisc_qstats_backlog_inc(sch, skb);
 	if (fq_flow_is_detached(f)) {
 		fq_flow_add_tail(&q->new_flows, f);
-		if (time_after(jiffies, f->age + q->flow_refill_delay)) {
-			f->credit = max_t(u32, f->credit, q->quantum);
-			f->time_credit_filled = ktime_get_ns();
-		}
 		q->inactive_flows--;
 	}
 
@@ -701,11 +696,9 @@ static int fq_change(struct Qdisc *sch, struct nlattr *opt)
 			err = -EINVAL;
 	}
 
-	if (tb[TCA_FQ_FLOW_REFILL_DELAY]) {
-		u32 usecs_delay = nla_get_u32(tb[TCA_FQ_FLOW_REFILL_DELAY]) ;
-
-		q->flow_refill_delay = usecs_to_jiffies(usecs_delay);
-	}
+	if (tb[TCA_FQ_FLOW_REFILL_DELAY])
+		pr_warn_ratelimited("sch_fq: refill delay %u ignored.\n",
+				    nla_get_u32(tb[TCA_FQ_FLOW_REFILL_DELAY]));
 
 	if (!err) {
 		sch_tree_unlock(sch);
@@ -744,7 +737,6 @@ static int fq_init(struct Qdisc *sch, struct nlattr *opt)
 	q->flow_plimit		= 100;
 	q->quantum		= 2 * psched_mtu(qdisc_dev(sch));
 	q->initial_quantum	= 10 * psched_mtu(qdisc_dev(sch));
-	q->flow_refill_delay	= msecs_to_jiffies(40);
 	q->flow_max_rate	= ~0U;
 	q->rate_enable		= 1;
 	q->new_flows.first	= NULL;
@@ -771,7 +763,9 @@ static int fq_dump(struct Qdisc *sch, struct sk_buff *skb)
 	if (opts == NULL)
 		goto nla_put_failure;
 
-	/* TCA_FQ_FLOW_DEFAULT_RATE is not used anymore */
+	/* TCA_FQ_FLOW_DEFAULT_RATE and TCA_FQ_FLOW_REFILL_DELAY
+	 * is not used anymore.
+	 */
 
 	if (nla_put_u32(skb, TCA_FQ_PLIMIT, sch->limit) ||
 	    nla_put_u32(skb, TCA_FQ_FLOW_PLIMIT, q->flow_plimit) ||
@@ -779,8 +773,6 @@ static int fq_dump(struct Qdisc *sch, struct sk_buff *skb)
 	    nla_put_u32(skb, TCA_FQ_INITIAL_QUANTUM, q->initial_quantum) ||
 	    nla_put_u32(skb, TCA_FQ_RATE_ENABLE, q->rate_enable) ||
 	    nla_put_u32(skb, TCA_FQ_FLOW_MAX_RATE, q->flow_max_rate) ||
-	    nla_put_u32(skb, TCA_FQ_FLOW_REFILL_DELAY,
-			jiffies_to_usecs(q->flow_refill_delay)) ||
 	    nla_put_u32(skb, TCA_FQ_BUCKETS_LOG, q->fq_trees_log))
 		goto nla_put_failure;
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 1/2] pkt_sched: fq: avoid artificial bursts for clocked flows
From: Kenneth Klette Jonassen @ 2015-02-02 18:59 UTC (permalink / raw)
  To: netdev; +Cc: Kenneth Klette Jonassen

Current pacing behavior always throttle flows for a time equal to one full
quantum, starting at the instance in time when a flow depletes its credit.
This is optimal for burst sizes that are a multiple of the chosen quantum.

For flows with many small and evenly clocked packets, the depletion and
refilling of credits cause packets to queue and transmit in bursts, even
when their clocked rate is below the pacing rate. With TCP ACKs, this
artificial queueing induces significant noise to RTTs, e.g. up to 2.07 ms
for rtt 20 ms, cwnd 10 and quantum 3028.

Packetdrill script to illustrate bursts:
0.000 socket(..., SOCK_DGRAM, IPPROTO_UDP) = 3
0.000 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
0.000 bind(3, ..., ...) = 0
0.000 connect(3, ..., ...) = 0

// SO_MAX_PACING_RATE: 2500 Bps, 100 ms per quantum, 20 ms per 50B packet.
0.000 setsockopt(3, SOL_SOCKET, 47, [2500], 4) = 0
0.000 `tc qdisc add dev tun0 root fq initial_quantum 250 quantum 250`

// Use 200 credits: send four perfectly spaced 50 byte packets.
0.000 write(3, ..., 22) = 22
0.000 > udp (22)
0.020 write(3, ..., 22) = 22
0.020 > udp (22)
0.040 write(3, ..., 22) = 22
0.040 > udp (22)
0.060 write(3, ..., 22) = 22
0.060 > udp (22)

// Send five perfectly spaced packets. The first credits are depleted at
// 1.000, and the remaining four packets are sent in a burst at 1.100.
// Packets are sent at their intended times when this patch is applied.
1.000 write(3, ..., 22) = 22
1.000 > udp (22)
1.020 write(3, ..., 22) = 22
1.040 write(3, ..., 22) = 22
1.060 write(3, ..., 22) = 22
1.080 write(3, ..., 22) = 22
1.100 > udp (22)
1.100 > udp (22)
1.100 > udp (22)
1.100 > udp (22)

Keep track of when a flows credit was last filled, and use this to
approximate a credit refill for each quantum of time that passes.

Increases memory footprint from 104 to 112 bytes per flow.

Signed-off-by: Kenneth Klette Jonassen <kennetkl@ifi.uio.no>
---
 net/sched/sch_fq.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 2a50f5c..6f0c45e 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -71,6 +71,7 @@ struct fq_flow {
 
 	struct rb_node  rate_node;	/* anchor in q->delayed tree */
 	u64		time_next_packet;
+	u64		time_credit_filled;
 };
 
 struct fq_flow_head {
@@ -250,6 +251,7 @@ static struct fq_flow *fq_classify(struct sk_buff *skb, struct fq_sched_data *q)
 			if (unlikely(skb->sk &&
 				     f->socket_hash != sk->sk_hash)) {
 				f->credit = q->initial_quantum;
+				f->time_credit_filled = ktime_get_ns();
 				f->socket_hash = sk->sk_hash;
 				f->time_next_packet = 0ULL;
 			}
@@ -271,6 +273,7 @@ static struct fq_flow *fq_classify(struct sk_buff *skb, struct fq_sched_data *q)
 	if (skb->sk)
 		f->socket_hash = sk->sk_hash;
 	f->credit = q->initial_quantum;
+	f->time_credit_filled = ktime_get_ns();
 
 	rb_link_node(&f->fq_node, parent, p);
 	rb_insert_color(&f->fq_node, root);
@@ -374,8 +377,10 @@ static int fq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	qdisc_qstats_backlog_inc(sch, skb);
 	if (fq_flow_is_detached(f)) {
 		fq_flow_add_tail(&q->new_flows, f);
-		if (time_after(jiffies, f->age + q->flow_refill_delay))
+		if (time_after(jiffies, f->age + q->flow_refill_delay)) {
 			f->credit = max_t(u32, f->credit, q->quantum);
+			f->time_credit_filled = ktime_get_ns();
+		}
 		q->inactive_flows--;
 	}
 
@@ -440,6 +445,7 @@ begin:
 
 	if (f->credit <= 0) {
 		f->credit += q->quantum;
+		f->time_credit_filled = max(now, f->time_next_packet);
 		head->first = f->next;
 		fq_flow_add_tail(&q->old_flows, f);
 		goto begin;
@@ -489,7 +495,10 @@ begin:
 			q->stat_pkts_too_long++;
 		}
 
-		f->time_next_packet = now + len;
+		/* If now < time_next_packet, throttles flow for a time equal
+		 * to one quantum (len) after current credits were filled.
+		 */
+		f->time_next_packet = f->time_credit_filled + len;
 	}
 out:
 	qdisc_bstats_update(sch, skb);
-- 
1.9.1

^ permalink raw reply related

* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Eric Dumazet @ 2015-02-02 18:52 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, Network Development, eyalpe
In-Reply-To: <CA+BoTQkV+mOZfe_Niz5101sMQeaV6muKCsShptjGQ1AgOHqqoQ@mail.gmail.com>

On Mon, 2015-02-02 at 11:27 +0100, Michal Kazior wrote:

> While testing I've had my internal GRO patch for ath10k and no stretch
> ack patches.

Thanks for the data, I took a look at it.

I am afraid this GRO patch might be the problem.

It seems to break ACK clocking badly (linux stack has a somewhat buggy
tcp_tso_should_defer(), which relies on ACK being received smoothly, as
no timer is setup to split the TSO packet.)

I am seeing huge delays on ACK packets and bursts like that :

05:01:53.413038 IP 192.168.1.2.5001 > 192.168.1.3.49669: Flags [.], ack 76745, win 4435, options [nop,nop,TS val 4294758508 ecr 4294757300], length 0
05:01:53.413407 IP 192.168.1.2.5001 > 192.168.1.3.49669: Flags [.], ack 79641, win 4435, options [nop,nop,TS val 4294758508 ecr 4294757301], length 0
05:01:53.413969 IP 192.168.1.2.5001 > 192.168.1.3.49669: Flags [.], ack 92673, win 4435, options [nop,nop,TS val 4294758510 ecr 4294757302], length 0
05:01:53.413990 IP 192.168.1.2.5001 > 192.168.1.3.49669: Flags [.], ack 97017, win 4435, options [nop,nop,TS val 4294758510 ecr 4294757302], length 0
05:01:53.414011 IP 192.168.1.2.5001 > 192.168.1.3.49669: Flags [.], ack 110049, win 4435, options [nop,nop,TS val 4294758510 ecr 4294757302], length 0
...
05:01:53.422663 IP 192.168.1.2.5001 > 192.168.1.3.49669: Flags [.], ack 189689, win 4435, options [nop,nop,TS val 4294758519 ecr 4294757310], length 0
05:01:53.424354 IP 192.168.1.2.5001 > 192.168.1.3.49669: Flags [.], ack 198377, win 4435, options [nop,nop,TS val 4294758520 ecr 4294757311], length 0
05:01:53.424400 IP 192.168.1.2.5001 > 192.168.1.3.49669: Flags [.], ack 202721, win 4435, options [nop,nop,TS val 4294758520 ecr 4294757313], length 0
05:01:53.424409 IP 192.168.1.2.5001 > 192.168.1.3.49669: Flags [.], ack 205617, win 4435, options [nop,nop,TS val 4294758520 ecr 4294757313], length 0
...
05:01:53.450248 IP 192.168.1.2.5001 > 192.168.1.3.49669: Flags [.], ack 419921, win 4435, options [nop,nop,TS val 4294758547 ecr 4294757337], length 0
05:01:53.450266 IP 192.168.1.2.5001 > 192.168.1.3.49669: Flags [.], ack 427161, win 4435, options [nop,nop,TS val 4294758547 ecr 4294757340], length 0
05:01:53.450289 IP 192.168.1.2.5001 > 192.168.1.3.49669: Flags [.], ack 431505, win 4435, options [nop,nop,TS val 4294758547 ecr 4294757340], length 0

Could you make again your experiments using upstream kernel (David
Miller net tree) ?

You also could post the GRO patch so that we can comment on it.

Thanks

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox