* [PATCH v2] net: cavium: Drop dependency of NET_VENDOR_CAVIUM on PCI
From: Alexander Sverdlin @ 2018-07-17 8:16 UTC (permalink / raw)
To: netdev
Cc: Alexander Sverdlin, David S. Miller, Aleksey Makarov,
Sunil Goutham, Raghu Vatsavayi, Vijaya Mohan Guvva
In-Reply-To: <20180717010159.GD10593@intel.com>
Octeon Ethernet drivers work perfectly without PCI.
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
---
drivers/net/ethernet/cavium/Kconfig | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/cavium/Kconfig b/drivers/net/ethernet/cavium/Kconfig
index 043e3c11c42b..ba65ed49f480 100644
--- a/drivers/net/ethernet/cavium/Kconfig
+++ b/drivers/net/ethernet/cavium/Kconfig
@@ -4,7 +4,6 @@
config NET_VENDOR_CAVIUM
bool "Cavium ethernet drivers"
- depends on PCI
default y
---help---
Select this option if you want enable Cavium network support.
@@ -15,7 +14,7 @@ if NET_VENDOR_CAVIUM
config THUNDER_NIC_PF
tristate "Thunder Physical function driver"
- depends on 64BIT
+ depends on 64BIT && PCI
select THUNDER_NIC_BGX
---help---
This driver supports Thunder's NIC physical function.
@@ -28,13 +27,13 @@ config THUNDER_NIC_PF
config THUNDER_NIC_VF
tristate "Thunder Virtual function driver"
imply CAVIUM_PTP
- depends on 64BIT
+ depends on 64BIT && PCI
---help---
This driver supports Thunder's NIC virtual function
config THUNDER_NIC_BGX
tristate "Thunder MAC interface driver (BGX)"
- depends on 64BIT
+ depends on 64BIT && PCI
select PHYLIB
select MDIO_THUNDER
select THUNDER_NIC_RGX
@@ -44,7 +43,7 @@ config THUNDER_NIC_BGX
config THUNDER_NIC_RGX
tristate "Thunder MAC interface driver (RGX)"
- depends on 64BIT
+ depends on 64BIT && PCI
select PHYLIB
select MDIO_THUNDER
---help---
@@ -53,7 +52,7 @@ config THUNDER_NIC_RGX
config CAVIUM_PTP
tristate "Cavium PTP coprocessor as PTP clock"
- depends on 64BIT
+ depends on 64BIT && PCI
imply PTP_1588_CLOCK
default y
---help---
@@ -65,7 +64,7 @@ config CAVIUM_PTP
config LIQUIDIO
tristate "Cavium LiquidIO support"
- depends on 64BIT
+ depends on 64BIT && PCI
depends on MAY_USE_DEVLINK
imply PTP_1588_CLOCK
select FW_LOADER
--
2.18.0
^ permalink raw reply related
* [PATCH] bpf: sockmap: remove redundant pointer sg
From: Colin King @ 2018-07-17 8:38 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Pointer sg is being assigned but is never used hence it is
redundant and can be removed.
Cleans up clang warning:
warning: variable 'sg' set but not used [-Wunused-but-set-variable]
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
kernel/bpf/sockmap.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 98fb7938beea..0b38be5a955c 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -725,11 +725,8 @@ static int bpf_tcp_sendmsg_do_redirect(struct sock *sk, int send,
{
bool ingress = !!(md->flags & BPF_F_INGRESS);
struct smap_psock *psock;
- struct scatterlist *sg;
int err = 0;
- sg = md->sg_data;
-
rcu_read_lock();
psock = smap_psock_sk(sk);
if (unlikely(!psock))
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 1/3] docs: networking: Fix indices heading indentation
From: Markus Heiser @ 2018-07-17 8:28 UTC (permalink / raw)
To: Tobin C. Harding, David S. Miller; +Cc: linux-doc, netdev, linux-kernel
In-Reply-To: <20180717042906.17308-2-me@tobin.cc>
Am Dienstag, den 17.07.2018, 14:29 +1000 schrieb Tobin C. Harding:
> Currently the 'Indices' heading is not aligned with column 0, it should
> be.
Hi Tobin, thats not correct. The 'Indices' heading is a part of the 'only'
block:
http://www.sphinx-doc.org/en/stable/markup/misc.html#including-content-based-on-tags
-- Markus --
>
> Fix 'Indices' heading indentation.
>
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
> ---
> Documentation/networking/index.rst | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
> index 6123a7e9e1da..a4bbde70bcb9 100644
> --- a/Documentation/networking/index.rst
> +++ b/Documentation/networking/index.rst
> @@ -20,7 +20,7 @@ Contents:
>
> .. only:: subproject
>
> - Indices
> - =======
> +Indices
> +=======
>
> * :ref:`genindex`
^ permalink raw reply
* [PATCH net-next] net: Move skb decrypted field, avoid explicity copy
From: Stefano Brivio @ 2018-07-17 7:18 UTC (permalink / raw)
To: David S. Miller; +Cc: Boris Pismenny, Stephen Rothwell, netdev
Commit 784abe24c903 ("net: Add decrypted field to skb")
introduced a 'decrypted' field that is explicitly copied on skb
copy and clone.
Move it between headers_start[0] and headers_end[0], so that we
don't need to copy it explicitly as it's copied by the memcpy()
in __copy_skb_header().
While at it, drop the assignment in __skb_clone(), it was
already redundant.
This doesn't change the size of sk_buff or cacheline boundaries.
The 15-bits hole before tc_index becomes a 14-bits hole, and
will be again a 15-bits hole when this change is merged with
commit 8b7008620b84 ("net: Don't copy pfmemalloc flag in
__copy_skb_header()").
Fixes: 784abe24c903 ("net: Add decrypted field to skb")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
include/linux/skbuff.h | 9 ++++-----
net/core/skbuff.c | 9 +++------
2 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 3ceb8dcc54da..14bc9ebe30f2 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -630,7 +630,6 @@ typedef unsigned char *sk_buff_data_t;
* @hash: the packet hash
* @queue_mapping: Queue mapping for multiqueue devices
* @xmit_more: More SKBs are pending for this queue
- * @decrypted: Decrypted SKB
* @ndisc_nodetype: router type (from link layer)
* @ooo_okay: allow the mapping of a socket to a queue to be changed
* @l4_hash: indicate hash is a canonical 4-tuple hash over transport
@@ -641,6 +640,7 @@ typedef unsigned char *sk_buff_data_t;
* @no_fcs: Request NIC to treat last 4 bytes as Ethernet FCS
* @csum_not_inet: use CRC32c to resolve CHECKSUM_PARTIAL
* @dst_pending_confirm: need to confirm neighbour
+ * @decrypted: Decrypted SKB
* @napi_id: id of the NAPI struct this skb came from
* @secmark: security marking
* @mark: Generic packet mark
@@ -737,11 +737,7 @@ struct sk_buff {
peeked:1,
head_frag:1,
xmit_more:1,
-#ifdef CONFIG_TLS_DEVICE
- decrypted:1;
-#else
__unused:1;
-#endif
/* fields enclosed in headers_start/headers_end are copied
* using a single memcpy() in __copy_skb_header()
@@ -797,6 +793,9 @@ struct sk_buff {
__u8 tc_redirected:1;
__u8 tc_from_ingress:1;
#endif
+#ifdef CONFIG_TLS_DEVICE
+ __u8 decrypted:1;
+#endif
#ifdef CONFIG_NET_SCHED
__u16 tc_index; /* traffic control index */
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index cfd6c6f35f9c..b8a563b2d2df 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -805,9 +805,6 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
* It is not yet because we do not want to have a 16 bit hole
*/
new->queue_mapping = old->queue_mapping;
-#ifdef CONFIG_TLS_DEVICE
- new->decrypted = old->decrypted;
-#endif
memcpy(&new->headers_start, &old->headers_start,
offsetof(struct sk_buff, headers_end) -
@@ -836,6 +833,9 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
#ifdef CONFIG_XPS
CHECK_SKB_FIELD(sender_cpu);
#endif
+#ifdef CONFIG_TLS_DEVICE
+ CHECK_SKB_FIELD(decrypted);
+#endif
#ifdef CONFIG_NET_SCHED
CHECK_SKB_FIELD(tc_index);
#endif
@@ -868,9 +868,6 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb)
C(head_frag);
C(data);
C(truesize);
-#ifdef CONFIG_TLS_DEVICE
- C(decrypted);
-#endif
refcount_set(&n->users, 1);
atomic_inc(&(skb_shinfo(skb)->dataref));
--
2.15.1
^ permalink raw reply related
* Re: [RFC PATCH net-next] net: mvpp2: mvpp2_flow_get_hek_fields() can be static
From: Maxime Chevallier @ 2018-07-17 7:38 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, netdev, Antoine Tenart, kbuild test robot,
linux-kernel
In-Reply-To: <20180717011812.GZ10593@intel.com>
Hi,
On Tue, 17 Jul 2018 09:18:12 +0800
kbuild test robot <lkp@intel.com> wrote:
>Fixes: d33ec4525007 ("net: mvpp2: add an RSS classification step for each flow")
>Signed-off-by: kbuild test robot <fengguang.wu@intel.com>
>---
> mvpp2_cls.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c
>index 723d0ba..fc9ec51 100644
>--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c
>+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c
>@@ -618,7 +618,7 @@ static int mvpp2_port_rss_hash_opts_set(struct mvpp2_port *port, int flow_type,
> return 0;
> }
>
>-u16 mvpp2_flow_get_hek_fields(struct mvpp2_cls_flow_entry *fe)
>+static u16 mvpp2_flow_get_hek_fields(struct mvpp2_cls_flow_entry *fe)
> {
> u16 hash_opts = 0;
> int n_fields, i, field;
That was a valid patch since it seems I forgot to make
another function static in the RSS series.
However this function is used in debugfs, so this patch isn't valid
anymore.
Sorry about that, but please don't apply this.
Thanks,
Maxime
--
Maxime Chevallier, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH net-next 4/4] act_mirred: use ACT_REDIRECT when possible
From: Eyal Birger @ 2018-07-17 7:01 UTC (permalink / raw)
To: Cong Wang
Cc: Paolo Abeni, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Marcelo Ricardo Leitner, shmulik.ladkani
In-Reply-To: <CAM_iQpVodV_BxKR8T-9Zn26g6gM=PLsip7bjv6SMCB13yQ6YMA@mail.gmail.com>
Hi,
On Mon, 16 Jul 2018 16:39:55 -0700
Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Jul 13, 2018 at 2:55 AM Paolo Abeni <pabeni@redhat.com> wrote:
> >
> > When mirred is invoked from the ingress path, and it wants to
> > redirect the processed packet, it can now use the ACT_REDIRECT
> > action, filling the tcf_result accordingly.
> >
> > This avoids a skb_clone() in the TC S/W data path giving a ~10%
> > improvement in forwarding performances. Overall TC S/W performances
> > are now comparable to the kernel openswitch datapath.
>
> Avoiding skb_clone() for redirection is cool, but why need to use
> skb_do_redirect() here?
>
> There is a subtle difference here:
>
> skb_do_redirect() calls __bpf_rx_skb() which calls
> dev_forward_skb().
>
> while the current mirred action doesn't scrub packets when
> redirecting to ingress (from egress). Although I forget if it is
> intentionally.
>
> Also, skb->skb_iif is unset in skb_do_redirect() when
> redirecting to ingress, I recall we have to set it correctly
> for input routing. Probably yet another reason why we
> can't scrub it, unless my memory goes wrong. :)
Also dev_forward_skb() enforces MTU checks on the packet which are not
done at the moment. I am aware of deployments where this would break
things.
Eyal.
^ permalink raw reply
* Re: [PATCH 2/4 v1] net: dsa: Add bindings for Realtek SMI DSAs
From: Linus Walleij @ 2018-07-17 6:55 UTC (permalink / raw)
To: Rob Herring
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, netdev,
OpenWrt Development List, LEDE Development List,
Antti Seppälä, Roman Yeryomin, Colin Leitner,
Gabor Juhos,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <20180716204535.GA22125@rob-hp-laptop>
On Mon, Jul 16, 2018 at 10:45 PM Rob Herring <robh@kernel.org> wrote:
> On Sat, Jul 14, 2018 at 11:45:54AM +0200, Linus Walleij wrote:
> > +The SMI "Simple Management Interface" is a two-wire protocol using
>
> At least for some other Realtek chips, the documentation I find says the
> S stands for Serial. And Wikipedia says SMI is the same thing as MDIO.
The only datasheet we have mentions both:
http://realtek.info/pdf/rtl8366_8369_datasheet_1-1.pdf
calling it serial management interface when talking to an EEPROM
calling it systems management interface when talking to a
PHY.
(BTW that datsheet has no relation to this driver, that is for
ASICs 8366 and 8369 which of course have nothing to do with
RTL8366RB "revision B" that we are running here, which adds
even more to the confusion.)
Sadly it would not surprise me if Realtek doesn't even know
which one it is themselves, given the state of the code and
documentation that came out of the company.
I just have to pick something. I can rename it the
"S Management Interface" if you prefer, OpenWRT called it
Systems Management Interface.
> Just want to make sure we don't define GPIOs directly when there should
> be a layer of abstraction like mdio-gpio.
OK let's look at it we read a single register with each protocol:
1. MDIO: drivers/net/phy/mdio-bitbang.c
send_bit is setting data and cycling clock 1-0 (falling edge)
begins transactions by sending 32 1:s (well 33 for safe measure)
then sends a start bit 01
then sends the opcode 10 (read)
then sends the PHY address (5 bits)
then sends the register address (5 bits)
turn around (switch MDIO to input)
read 16 bit register value
read stop bit
2. SMI:
sends the sequence "101" to start transactions. (No initial ones)
writes a whole byte which is the "command" read is 0xa9
on RTL8366RB and apparently something else on other chips
writes low byte of 16bit address
writes high byte of 16bit address
turn around (switch MDIO to input)
reads the low byte of the 16bit data
reads the high byte of the 16bit data
sends the stop sequence 01
then an extra clock pulse
As you can see those are pretty different. PHY always being
addressed in MDIO (the switch chips has PHYs but this
protocol is not defined exclusively for that) and both phy and
reg being 5 bits addressing at most 10 address bits is the most
obvious deviation then 8 bits for command instead of 2 etc.
It's just very different.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH net-next] xdp: fix uninitialized 'err' variable
From: Daniel Borkmann @ 2018-07-17 6:44 UTC (permalink / raw)
To: Jakub Kicinski, davem; +Cc: alexei.starovoitov, kbuild-all, oss-drivers, netdev
In-Reply-To: <20180717020850.16510-1-jakub.kicinski@netronome.com>
On 07/17/2018 04:08 AM, Jakub Kicinski wrote:
> Smatch caught an uninitialized variable error which GCC seems
> to miss.
>
> Fixes: a25717d2b604 ("xdp: support simultaneous driver and hw XDP attachment")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
David, feel free to take this directly into net-next, thanks!
^ permalink raw reply
* Re: [PATCH v3 net-next] net/sched: add skbprio scheduler
From: Cong Wang @ 2018-07-17 6:41 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Michel Machado, Nishanth Devarajan, Jamal Hadi Salim, Jiri Pirko,
David Miller, Linux Kernel Network Developers, Cody Doucette
In-Reply-To: <20180714043917.GA20383@localhost.localdomain>
On Fri, Jul 13, 2018 at 9:39 PM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Fri, Jul 13, 2018 at 11:26:28AM -0700, Cong Wang wrote:
> > On Fri, Jul 13, 2018 at 6:04 AM Marcelo Ricardo Leitner
> > <marcelo.leitner@gmail.com> wrote:
> > >
> > > On Thu, Jul 12, 2018 at 11:05:45PM -0700, Cong Wang wrote:
> > > > On Wed, Jul 11, 2018 at 12:33 PM Marcelo Ricardo Leitner
> > > > <marcelo.leitner@gmail.com> wrote:
> > > > >
> > > > > On Tue, Jul 10, 2018 at 07:25:53PM -0700, Cong Wang wrote:
> > > > > > On Mon, Jul 9, 2018 at 2:40 PM Marcelo Ricardo Leitner
> > > > > > <marcelo.leitner@gmail.com> wrote:
> > > > > > >
> > > > > > > On Mon, Jul 09, 2018 at 05:03:31PM -0400, Michel Machado wrote:
> > > > > > > > Changing TC_PRIO_MAX from 15 to 63 risks breaking backward compatibility
> > > > > > > > with applications.
> > > > > > >
> > > > > > > If done, it needs to be done carefully, indeed. I don't know if it's
> > > > > > > doable, neither I know how hard is your requirement for 64 different
> > > > > > > priorities.
> > > > > >
> > > > > > struct tc_prio_qopt {
> > > > > > int bands; /* Number of bands */
> > > > > > __u8 priomap[TC_PRIO_MAX+1]; /* Map: logical priority -> PRIO band */
> > > > > > };
> > > > > >
> > > > > > How would you do it carefully?
> > > > >
> > > > > quick shot, multiplex v1 and v2 formats based on bands and sizeof():
> > > > >
> > > > > #define TCQ_PRIO_BANDS_V1 16
> > > > > #define TCQ_PRIO_BANDS_V2 64
> > > > > #define TC_PRIO_MAX_V2 64
> > > > >
> > > > > struct tc_prio_qopt_v2 {
> > > > > int bands; /* Number of bands */
> > > > > __u8 priomap[TC_PRIO_MAX_V2+1]; /* Map: logical priority -> PRIO band */
> > > > > };
> > > > >
> > > >
> > > > Good try, but:
> > > >
> > > > 1. You don't take padding into account, although the difference
> > > > between 16 and 64 is big here. If it were 16 and 20, almost certainly
> > > > wouldn't work.
> > >
> > > It still would work, no matter how much padding you have, as currently
> > > you can't use more than 3 bands.
> >
> > I am lost.
> >
> > With your proposal above, you have 16 bands for V1 and 64 bands
> > for V2, where does 3 come from???
>
> My bad. s/3/16/
Ah, thanks for clarifying it! Please see below.
>
> >
> >
> > >
> > > >
> > > > 2. What if I compile a new iproute2 on an old kernel? The iproute2
> > > > will use V2, while old kernel has no knowledge of V2, so it only
> > > > copies a part of V2 in the end....
> > >
> > > Yes, and that's not a problem:
> > > - Either bands is > 3 and it will return EINVAL, protecting from
> > > reading beyond the buffer.
> > > - Or 2 <= bands <= 3 and it will handle it as a _v1 struct, and use
> > > only the original size.
> >
> > Again why 3 not 16 or 64 ??
>
> Again, s/3/16/
>
> >
> > Also, why does an old kernel has the logic in its binary to determine
> > this?
>
> It won't, and it doesn't need to. If you use bands > 16 with an old
> kernel, it will reject per current code (that I already pasted):
>
> if (qopt->bands > TCQ_PRIO_BANDS || qopt->bands < 2)
> return -EINVAL;
>
> Simple as that. If you try to use more bands than it supports, it will
> reject it.
Hmm, I see, But in your demo code, you miss the following pieces:
for (i = 0; i <= TC_PRIO_MAX; i++) {
if (qopt->priomap[i] >= qopt->bands)
return -EINVAL;
}
memcpy(q->prio2band, qopt->priomap, TC_PRIO_MAX+1);
I guess you want to change TC_PRIO_MAX to qopt->bands
too.
With this together, your suggestion actually looks reasonable.
Do I understand it correctly?
>
> >
> > >
> > > iproute2 (or other app) may still use _v1 if it wants, btw.
> >
> > Yes, old iproute2 must still have v1, what's point? Are you
>
> ??
>
> > suggesting new iproute2 should still have v1 after you propose
> > v1 and v2 for kernel?
>
> I'm only saying that both versions will be accepted by a new kernel.
I see, I thought you suggest to completely move to V2 for new
kernel.
Nice compatibility trick!
Thanks.
^ permalink raw reply
* Re: [PATCH - revised] rhashtable: detect when object movement might have invalidated a lookup
From: Herbert Xu @ 2018-07-17 6:30 UTC (permalink / raw)
To: NeilBrown; +Cc: David Miller, tgraf, netdev, linux-kernel, eric.dumazet
In-Reply-To: <871sc3uah9.fsf@notabene.neil.brown.name>
On Mon, Jul 16, 2018 at 01:26:42PM +1000, NeilBrown wrote:
>
> Look in Documenation/RCU/rculist_nulls.txt.
> The very first example is a typical lookup for a nulls list.
> The above sample code would read:
OK, but how will this work with rhlist? It would be very bad to
have a feature that works for rhashtable but fails in strange
ways when you use rhlist.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH net-next] liquidio: correct error msg text when removing VLAN ID
From: David Miller @ 2018-07-17 5:56 UTC (permalink / raw)
To: felix.manlunas
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
ricardo.farrington
In-Reply-To: <20180717010607.GA2735@felix-thinkpad.cavium.com>
From: Felix Manlunas <felix.manlunas@cavium.com>
Date: Mon, 16 Jul 2018 18:06:07 -0700
> From: Rick Farrington <ricardo.farrington@cavium.com>
>
> Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
Applied.
^ permalink raw reply
* Re: [PATCH v3 net-next] net/sched: add skbprio scheduler
From: Cong Wang @ 2018-07-17 5:36 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Michel Machado, Nishanth Devarajan, Jamal Hadi Salim, Jiri Pirko,
David Miller, Linux Kernel Network Developers, Cody Doucette
In-Reply-To: <20180714045101.GB20383@localhost.localdomain>
On Fri, Jul 13, 2018 at 9:51 PM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> Well, it would help if you didn't cut out key parts of my words.
Sorry about it, please allow me to copy and paste all of your words
here:
"Yes, but Michel wants to drop from other lower priorities if needed,
and that's not possible if you handle the limit already in a child
qdisc as they don't know about their siblings. The idea in the example
above is to discard it from whatever lower priority is needed, then
queue it. (ok, the example missed to check the priority level)"
So from your own words, you agreed "the idea in the example"
is not what Michel wants, because "is to discard it from whatever
lower priority is needed", as "Michel wants to drop from other lower
priorities if needed".
You also agreed Michel's requirement is not possible (to implement
in sch_prio) because "you handle the limit already in a child qdisc
as they don't know about their siblings" is also true.
Based on the above, I said it "disproves your point of adding a flag
to sch_prio".
What am I missing?
>
> >
> > What am I missing here?
> >
> > Are you go further by suggesting moving the limit out of prio?
> > Or are you going to expand your definition of "adding a flag"?
> > Perhaps two flags? :)
> >
> > I am very open for discussion to see how far we can go.
>
> I am not keen on continuing this discussion if you keep twisting my
> words just for fun.
No, I am trying to understand seriously about what you suggest here.
Please be patient! I know I am stupid!!!! :)
Thanks!
^ permalink raw reply
* Re: [PATCH v3 net-next 0/3] rds: IPv6 support
From: Ka-Cheong Poon @ 2018-07-17 5:32 UTC (permalink / raw)
To: Sowmini Varadhan; +Cc: netdev, santosh.shilimkar, davem, rds-devel
In-Reply-To: <20180716162001.GA22404@oracle.com>
On 07/17/2018 12:20 AM, Sowmini Varadhan wrote:
>
> - Looks like rds_connect() is checking things in the right order (thanks)
> However, rds_cancel_sent_to is still looking at the len to figure
> out the family.. as we move to ipv6, it would be better if we allow
> the caller to specify struct sockaddr_storage, or even a union of
> sockaddr_in/sockaddr_in6, rather than require them to hint at which
> one of ipv4/ipv6 through the optlen.
The app can use either structures to make the call. When the
app fills in the structure, it knows what it is filling in,
either sockaddr_in or sockaddr_in6. So it knows the right size
to use. The app can also use IPv4 mapped address in a sockaddr_in6
without a problem.
> Please see __sys_connect and move_addr_to_kernel if the user-kernel
> copy is the reason you are not doing this. Similar to inet_dgram_connect
> you can then check the sa_family and use that to figure out the
> "Assume IPv4" etc stuff.
>
> This would also make the CANCEL_SEND_TO API consistent with the bind/
> connect etc semantics.
Could you please explain the inconsistency? An app can use IPv4
mapped address in a sockaddr_in6 to operate on an IPv4 connection,
in case you are thinking of this new addition in v3 of the patch.
> - net/rds/rds.h: thanks for moving RDS_CM_PORT to the rdma specific file.
>
> I am guessing (?) that you want to update the comment to talk about
> the non-existent "RDS over UDP" based on the title of the IANA registration?
> I would just like to re-iterate that this is actually inaccurate
> (and confusing to someone looking at this for the first time, since
> there is no RDS-over-UDP today). If it were up to me, I would update
> the comment to say
>
> /* The following ports, 16385, 18634, 18635, are registered with IANA as
> * the ports to be used for "RDS over TCP and UDP".
> * The current linux implementation supports RDS over TCP and IB, and uses
> * the ports as follows: 18634 is the historical value used for the
> * RDMA_CM listener port. RDS/TCP uses port 16385. After
> * IPv6 work, RDMA_CM also uses 16385 as the listener port. 18634 is kept
> * to ensure compatibility with older RDS modules. Those ports are defined
> * in each transport's header file.
Will update it to
/* The following ports, 16385, 18634, 18635, are registered with IANA as
* the ports to be used for RDS over TCP and UDP. Currently, only RDS
over
* TCP and RDS over IB/RDMA are implemented. 18634 is the historical
value
* used for the RDMA_CM listener port. RDS/TCP uses port 16385. After
* IPv6 work, RDMA_CM also uses 16385 as the listener port. 18634 is
kept
* to ensure compatibility with older RDS modules. Those ports are
defined
* in each transport's header file.
*/
--
K. Poon
ka-cheong.poon@oracle.com
^ permalink raw reply
* Re: linux-next: manual merge of the net-next tree with the net tree
From: Stefano Brivio @ 2018-07-17 5:47 UTC (permalink / raw)
To: Boris Pismenny
Cc: Stephen Rothwell, David Miller, Networking,
Linux-Next Mailing List, Linux Kernel Mailing List
In-Reply-To: <20180717123306.3e030fde@canb.auug.org.au>
On Tue, 17 Jul 2018 12:33:06 +1000
Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi all,
>
> Today's linux-next merge of the net-next tree got a conflict in:
>
> include/linux/skbuff.h
>
> between commit:
>
> 8b7008620b84 ("net: Don't copy pfmemalloc flag in __copy_skb_header()")
>
> from the net tree and commit:
>
> 784abe24c903 ("net: Add decrypted field to skb")
>
> from the net-next tree.
>
> [...]
>
> @@@ -736,7 -737,11 +738,11 @@@ struct sk_buff
> peeked:1,
> head_frag:1,
> xmit_more:1,
> + pfmemalloc:1;
> + #ifdef CONFIG_TLS_DEVICE
> - decrypted:1;
> -#else
> - __unused:1;
> ++ __u8 decrypted:1,
> ++ __unused:7;
> + #endif
>
> /* fields enclosed in headers_start/headers_end are copied
> * using a single memcpy() in __copy_skb_header()
I checked the layout of sk_buff after this, we already had a 1-byte
hole there, that now becomes a 7-bits hole (or disappears with
__unused). That's fine.
Boris, I read your commit 784abe24c903 ("net: Add decrypted field to
skb") just now.
I think 'decrypted' shouldn't go there, because you then copy it on
copy and clone: if you move if after headers_start[0] you don't need to
explicitly copy it in __copy_skb_header().
While at it, the copy you added in __skb_clone() is redundant (no
matter the position of 'decrypted').
I can send a clean-up patch against net-next. I guess this might cause
again a linux-next merge conflict, but it's again trivial (the __unused
field above would go away).
--
Stefano
^ permalink raw reply
* Re: [PATCH net-next 0/3] Series to improve setsockopt() TCP_USER_TIMEOUT accuracy
From: Jon Maxwell @ 2018-07-17 5:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jon Maxwell, Miller, David, edumazet, ncardwell, David.Laight,
kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <0cdd742b-ad2e-aeac-c85f-21c59333d54d@gmail.com>
On Tue, Jul 17, 2018 at 2:56 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> On 07/16/2018 09:15 PM, Jon Maxwell wrote:
>> This is a patch series based on:
>>
>> Jon Maxwell (3):
>> [PATCH net-next 1/3] tcp: convert icsk_user_timeout from jiffies to msecs
>> [PATCH net-next v1 2/3] tcp: convert icsk_user_timeout from jiffies to msecs
>> [PATCH net-next v1 3/3] tcp: convert icsk_user_timeout from jiffies to msecs
>
> This would have been nice to use meaningful titles for each patch,
> instead of copy/pasting the first one ?
>
> Thanks !
>
>
>
This is my 1st time doing a patch series. Let me rebase and resubmit
each with a more descriptive title.
But 1st do I still need to put the patch number in the series? Even if
the title differs?
1) e.g:
Jon Maxwell (3):
[PATCH net-next 1/3] tcp: convert icsk_user_timeout from jiffies to msecs
[PATCH net-next 2/3] tcp: Add tcp_retransmit_stamp() helper
[PATCH net-next 3/3] tcp: Add tcp_clamp_rto_to_user_timeout() to
improve accuracy
2) or:
Jon Maxwell (3):
[PATCH net-next] tcp: convert icsk_user_timeout from jiffies to msecs
[PATCH net-next] tcp: Add tcp_retransmit_time() helper
[PATCH net-next] tcp: Add tcp_clamp_rto_to_user_timeout() to
improve TCP_USER_TIMEOUT accuracy
Which is preferred (1) or (2)? Are the above titles descriptive enough?
Regards
Jon
^ permalink raw reply
* Re: [PATCH net-next 0/3] Series to improve setsockopt() TCP_USER_TIMEOUT accuracy
From: Eric Dumazet @ 2018-07-17 4:56 UTC (permalink / raw)
To: Jon Maxwell, davem
Cc: edumazet, ncardwell, David.Laight, kuznet, yoshfuji, netdev,
linux-kernel, jmaxwell
In-Reply-To: <20180717041602.31100-1-jmaxwell37@gmail.com>
On 07/16/2018 09:15 PM, Jon Maxwell wrote:
> This is a patch series based on:
>
> Jon Maxwell (3):
> [PATCH net-next 1/3] tcp: convert icsk_user_timeout from jiffies to msecs
> [PATCH net-next v1 2/3] tcp: convert icsk_user_timeout from jiffies to msecs
> [PATCH net-next v1 3/3] tcp: convert icsk_user_timeout from jiffies to msecs
This would have been nice to use meaningful titles for each patch,
instead of copy/pasting the first one ?
Thanks !
^ permalink raw reply
* [PATCH net-next v1 2/3] tcp: convert icsk_user_timeout from jiffies to msecs
From: Jon Maxwell @ 2018-07-17 4:16 UTC (permalink / raw)
To: davem
Cc: edumazet, ncardwell, David.Laight, kuznet, yoshfuji, netdev,
linux-kernel, jmaxwell
v1 fixes fuzzy Indentation.
Create a seperate helper routine called tcp_retransmit_stamp() as per Neal
Cardwells suggestion. To be used by the final commit in this series and
retransmits_timed_out().
Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
---
net/ipv4/tcp_timer.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index fa34984d0b12..d212f183dd2d 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -22,6 +22,20 @@
#include <linux/gfp.h>
#include <net/tcp.h>
+u32 tcp_retransmit_stamp(const struct sock *sk)
+{
+ u32 start_ts = tcp_sk(sk)->retrans_stamp;
+
+ if (unlikely(!start_ts)) {
+ struct sk_buff *head = tcp_rtx_queue_head(sk);
+
+ if (!head)
+ return 0;
+ start_ts = tcp_skb_timestamp(head);
+ }
+ return start_ts;
+}
+
/**
* tcp_write_err() - close socket and save error info
* @sk: The socket the error has appeared on.
@@ -166,14 +180,9 @@ static bool retransmits_timed_out(struct sock *sk,
if (!inet_csk(sk)->icsk_retransmits)
return false;
- start_ts = tcp_sk(sk)->retrans_stamp;
- if (unlikely(!start_ts)) {
- struct sk_buff *head = tcp_rtx_queue_head(sk);
-
- if (!head)
- return false;
- start_ts = tcp_skb_timestamp(head);
- }
+ start_ts = tcp_retransmit_stamp(sk);
+ if (!start_ts)
+ return false;
if (likely(timeout == 0)) {
linear_backoff_thresh = ilog2(TCP_RTO_MAX/rto_base);
--
2.13.6
^ permalink raw reply related
* Re: [PATCH ipsec-next] xfrm: Allow Set Mark to be Updated Using UPDSA
From: Eyal Birger @ 2018-07-17 4:13 UTC (permalink / raw)
To: Nathan Harold; +Cc: netdev
In-Reply-To: <CADhJOfYbhpr8OzNzGGC1McmbZ+2QP3UC3U9JAWS9N4nG2Q9FNQ@mail.gmail.com>
On Mon, 16 Jul 2018 15:27:26 -0700
Nathan Harold <nharold@google.com> wrote:
> < re-sent with apologies due to incorrect formatting last
> time... :-( >
>
> Hi Eyal,
>
> > If x1 points to a state previously found using
> > __xfrm_state_locate(x), won't __xfrm_state_bump_genids(x1) be
> > equivalent to x1->genid++ in this case?
>
> In the vanilla case this is true. IE, if there are no strange/abusive
> uses of the API such as the test below where multiple SAs can match
> the locate().
>
> > Is it possible that other states will match all of x1 parameters?
>
> Yes. Not sure if it's a bug or a feature, but it's possible for
> multiple SAs to match... for a depressing example, check out
> https://android-review.googlesource.com/c/kernel/tests/+/680958. There
> may be cases where something like this is desired behavior that I'm
> not aware of. Since this is control path, it felt to me like the
> formalism of using the xfrm_state_bump_genids() was worth not possibly
> walking into a different subtle bug later.
Ok. This is indeed depressing and also unexpected.
I wonder if this behavior could be fixed... I'd find it odd if anyone
is relying on being to able to delete a 'no mark' state by supplying
parameters that do include an explicit mark. I have no idea if anyone
is relying on the state insertion order wrt marks - though it would
seem odd to me as well -- obviously such a change is unrelated to this
patch.
I now better understand the need to be cautious.
>
> > Also, any idea why this isn't needed for other changes in the
> > state?
>
> The set_mark (output_mark) is somewhat special because changing this
> mark impacts the routing lookup, which up to now, none of the other
> parameters in the update_sa function do. A new output_mark can and
> will reroute packets to different interfaces. Thus, when we change
> this thing, we want to ensure that we always build a new bundle with a
> new bundle with a new route lookup based on the new set_mark. Since we
> removed the flow cache, things might *incidentally* seem to work right
> now; but, I think that's incidental rather than correct. By bumping
> the genid, we get the dst_entry->check() function to correctly return
> that the dst is obsolete when we call check(). I'm honestly not sure
> what corner cases we could land in if we didn't bump the genid in such
> a case.
>
> There's definitely a lot going on behind the scenes in this little
> change that I only tenuously grasp, so it's possible that I'm being
> overly cautious in this case. Please let me know your further thoughts
> on whether we need to bump the genid. FYI once this patch is settled,
> I plan to upload a patch to update the xfrm_if_id, which I planned to
> nestle in to this same logic (and with similar, albeit possibly
> more-straightforward rationale).
Thanks so much for the clarification. Indeed there are nuances here and
I appreciate you taking the time to describe them.
FWIW you can add my:
Reviewed-by: Eyal Birger <eyal.birger@gmail.com>
Thanks!
Eyal.
^ permalink raw reply
* [PATCH net-next 3/3] docs: networking: Convert bridge.txt to rst
From: Tobin C. Harding @ 2018-07-17 4:29 UTC (permalink / raw)
To: David S. Miller; +Cc: Tobin C. Harding, linux-doc, netdev, linux-kernel
In-Reply-To: <20180717042906.17308-1-me@tobin.cc>
The kernel documentation is now restructured text. Convert the Ethernet
Bridge documentation and include it in the toplevel kernel
documentation.
- Fix heading adornments.
- Add license identifier.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
Documentation/networking/{bridge.txt => bridge.rst} | 6 ++++++
Documentation/networking/index.rst | 1 +
2 files changed, 7 insertions(+)
rename Documentation/networking/{bridge.txt => bridge.rst} (85%)
diff --git a/Documentation/networking/bridge.txt b/Documentation/networking/bridge.rst
similarity index 85%
rename from Documentation/networking/bridge.txt
rename to Documentation/networking/bridge.rst
index a27cb6214ed7..4aef9cddde2f 100644
--- a/Documentation/networking/bridge.txt
+++ b/Documentation/networking/bridge.rst
@@ -1,3 +1,9 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=================
+Ethernet Bridging
+=================
+
In order to use the Ethernet bridging functionality, you'll need the
userspace tools.
diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index 65502f2031a8..d75da2ff25c7 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -18,6 +18,7 @@ Contents:
failover
net_failover
alias
+ bridge
.. only:: subproject
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 2/3] docs: networking: Convert alias.txt to rst
From: Tobin C. Harding @ 2018-07-17 4:29 UTC (permalink / raw)
To: David S. Miller; +Cc: Tobin C. Harding, linux-doc, netdev, linux-kernel
In-Reply-To: <20180717042906.17308-1-me@tobin.cc>
The kernel documentation is now restructured text. Convert the IP
aliasing documentation and include it in the toplevel kernel
documentation.
- Fix heading adornments.
- Correctly indent code snippets.
- Limit line length to 72 characters inline with kernel documentation
standards.
- Add license identifier.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
Documentation/networking/00-INDEX | 2 --
Documentation/networking/alias.rst | 49 ++++++++++++++++++++++++++++++
Documentation/networking/alias.txt | 40 ------------------------
Documentation/networking/index.rst | 1 +
4 files changed, 50 insertions(+), 42 deletions(-)
create mode 100644 Documentation/networking/alias.rst
delete mode 100644 Documentation/networking/alias.txt
diff --git a/Documentation/networking/00-INDEX b/Documentation/networking/00-INDEX
index 2b89d91b376f..1e5153ed8990 100644
--- a/Documentation/networking/00-INDEX
+++ b/Documentation/networking/00-INDEX
@@ -18,8 +18,6 @@ README.ipw2200
- README for the Intel PRO/Wireless 2915ABG and 2200BG driver.
README.sb1000
- info on General Instrument/NextLevel SURFboard1000 cable modem.
-alias.txt
- - info on using alias network devices.
altera_tse.txt
- Altera Triple-Speed Ethernet controller.
arcnet-hardware.txt
diff --git a/Documentation/networking/alias.rst b/Documentation/networking/alias.rst
new file mode 100644
index 000000000000..af7c5ee92014
--- /dev/null
+++ b/Documentation/networking/alias.rst
@@ -0,0 +1,49 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===========
+IP-Aliasing
+===========
+
+IP-aliases are an obsolete way to manage multiple IP-addresses/masks
+per interface. Newer tools such as iproute2 support multiple
+address/prefixes per interface, but aliases are still supported
+for backwards compatibility.
+
+An alias is formed by adding a colon and a string when running ifconfig.
+This string is usually numeric, but this is not a must.
+
+
+Alias creation
+==============
+
+Alias creation is done by 'magic' interface naming: eg. to create a
+200.1.1.1 alias for eth0 ...
+::
+
+ # ifconfig eth0:0 200.1.1.1 etc,etc....
+ ~~ -> request alias #0 creation (if not yet exists) for eth0
+
+The corresponding route is also set up by this command. Please note:
+The route always points to the base interface.
+
+
+Alias deletion
+==============
+
+The alias is removed by shutting the alias down::
+
+ # ifconfig eth0:0 down
+ ~~~~~~~~~~ -> will delete alias
+
+
+Alias (re-)configuring
+======================
+
+Aliases are not real devices, but programs should be able to configure
+and refer to them as usual (ifconfig, route, etc).
+
+
+Relationship with main device
+=============================
+
+If the base device is shut down the added aliases will be deleted too.
diff --git a/Documentation/networking/alias.txt b/Documentation/networking/alias.txt
deleted file mode 100644
index 85046f53fcfc..000000000000
--- a/Documentation/networking/alias.txt
+++ /dev/null
@@ -1,40 +0,0 @@
-
-IP-Aliasing:
-============
-
-IP-aliases are an obsolete way to manage multiple IP-addresses/masks
-per interface. Newer tools such as iproute2 support multiple
-address/prefixes per interface, but aliases are still supported
-for backwards compatibility.
-
-An alias is formed by adding a colon and a string when running ifconfig.
-This string is usually numeric, but this is not a must.
-
-o Alias creation.
- Alias creation is done by 'magic' interface naming: eg. to create a
- 200.1.1.1 alias for eth0 ...
-
- # ifconfig eth0:0 200.1.1.1 etc,etc....
- ~~ -> request alias #0 creation (if not yet exists) for eth0
-
- The corresponding route is also set up by this command.
- Please note: The route always points to the base interface.
-
-
-o Alias deletion.
- The alias is removed by shutting the alias down:
-
- # ifconfig eth0:0 down
- ~~~~~~~~~~ -> will delete alias
-
-
-o Alias (re-)configuring
-
- Aliases are not real devices, but programs should be able to configure and
- refer to them as usual (ifconfig, route, etc).
-
-
-o Relationship with main device
-
- If the base device is shut down the added aliases will be deleted
- too.
diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index a4bbde70bcb9..65502f2031a8 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -17,6 +17,7 @@ Contents:
msg_zerocopy
failover
net_failover
+ alias
.. only:: subproject
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 1/3] docs: networking: Fix indices heading indentation
From: Tobin C. Harding @ 2018-07-17 4:29 UTC (permalink / raw)
To: David S. Miller; +Cc: Tobin C. Harding, linux-doc, netdev, linux-kernel
In-Reply-To: <20180717042906.17308-1-me@tobin.cc>
Currently the 'Indices' heading is not aligned with column 0, it should
be.
Fix 'Indices' heading indentation.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
Documentation/networking/index.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index 6123a7e9e1da..a4bbde70bcb9 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -20,7 +20,7 @@ Contents:
.. only:: subproject
- Indices
- =======
+Indices
+=======
* :ref:`genindex`
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 0/3] docs: Convert alias and bridge to rst
From: Tobin C. Harding @ 2018-07-17 4:29 UTC (permalink / raw)
To: David S. Miller; +Cc: Tobin C. Harding, linux-doc, netdev, linux-kernel
Hi Dave,
Here is my first attempt at working on converting docs in
Documentation/networking to rst format. I've picked a couple of trivial
ones to start with. If there is anything extra I can do to make your
life easier during documentation conversion please say. (Also if there
is some reason that it would be preferable to _not_ embark on this task
please say :)
This set does not make any changes to the converted files apart from
formatting.
thanks,
Tobin.
Tobin C. Harding (3):
docs: networking: Fix indices heading indentation
docs: networking: Convert alias.txt to rst
docs: networking: Convert bridge.txt to rst
Documentation/networking/00-INDEX | 2 -
Documentation/networking/alias.rst | 49 +++++++++++++++++++
Documentation/networking/alias.txt | 40 ---------------
.../networking/{bridge.txt => bridge.rst} | 6 +++
Documentation/networking/index.rst | 6 ++-
5 files changed, 59 insertions(+), 44 deletions(-)
create mode 100644 Documentation/networking/alias.rst
delete mode 100644 Documentation/networking/alias.txt
rename Documentation/networking/{bridge.txt => bridge.rst} (85%)
--
2.17.1
^ permalink raw reply
* [PATCH net-next v1 3/3] tcp: convert icsk_user_timeout from jiffies to msecs
From: Jon Maxwell @ 2018-07-17 4:16 UTC (permalink / raw)
To: davem
Cc: edumazet, ncardwell, David.Laight, kuznet, yoshfuji, netdev,
linux-kernel, jmaxwell
v1 fixes line wrap of min_t().
The final patch in this series. Create the tcp_clamp_rto_to_user_timeout()
helper routine. To calculate the correct rto, so that the TCP_USER_TIMEOUT
socket option is more accurate. Taking suggestions and feedback into account
from Eric Dumazet, Neal Cardwell and David Laight. Due to the 1st commit we
can avoid the msecs_to_jiffies() and jiffies_to_msecs() dance.
Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
---
net/ipv4/tcp_timer.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index d212f183dd2d..a242f8874629 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -36,6 +36,21 @@ u32 tcp_retransmit_stamp(const struct sock *sk)
return start_ts;
}
+static u32 tcp_clamp_rto_to_user_timeout(const struct sock *sk)
+{
+ struct inet_connection_sock *icsk = inet_csk(sk);
+ u32 elapsed, start_ts;
+
+ start_ts = tcp_retransmit_stamp(sk);
+ if (!icsk->icsk_user_timeout || !start_ts)
+ return icsk->icsk_rto;
+ elapsed = tcp_time_stamp(tcp_sk(sk)) - start_ts;
+ if (elapsed >= icsk->icsk_user_timeout)
+ return 1; /* user timeout has passed; fire ASAP */
+ else
+ return min_t(u32, icsk->icsk_rto, msecs_to_jiffies(icsk->icsk_user_timeout - elapsed));
+}
+
/**
* tcp_write_err() - close socket and save error info
* @sk: The socket the error has appeared on.
@@ -544,7 +559,8 @@ void tcp_retransmit_timer(struct sock *sk)
/* Use normal (exponential) backoff */
icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
}
- inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, icsk->icsk_rto, TCP_RTO_MAX);
+ inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
+ tcp_clamp_rto_to_user_timeout(sk), TCP_RTO_MAX);
if (retransmits_timed_out(sk, net->ipv4.sysctl_tcp_retries1 + 1, 0))
__sk_dst_reset(sk);
--
2.13.6
^ permalink raw reply related
* [PATCH net-next 1/3] tcp: convert icsk_user_timeout from jiffies to msecs
From: Jon Maxwell @ 2018-07-17 4:16 UTC (permalink / raw)
To: davem
Cc: edumazet, ncardwell, David.Laight, kuznet, yoshfuji, netdev,
linux-kernel, jmaxwell
This is a preparatory commit for an upcoming patch that improves the socket
TCP_USER_TIMEOUT option accuracy. Implement Eric Dumazets idea to convert
icsk->icsk_user_timeout from jiffies to msecs. To eliminate the
msecs_to_jiffies() and jiffies_to_msecs() dance in future.
There will 3 patches in this series. [net-next v1 2/3] will create a seperate
helper routine called tcp_retransmit_stamp() as per Neal Cardwells suggestion.
Finally [net-next v1 3/3] will implement a new routine called
tcp_clamp_rto_to_user_timeout() to calculate the rto so that TCP_USER_TIMEOUT
is more accurate. As a result of the final patch it won't have the
msecs_to_jiffies() and jiffies_to_msecs() dance that David Laight was concerned
about.
Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
---
net/ipv4/tcp.c | 4 ++--
net/ipv4/tcp_timer.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index e3704a49164b..9d900162f16a 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2984,7 +2984,7 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
if (val < 0)
err = -EINVAL;
else
- icsk->icsk_user_timeout = msecs_to_jiffies(val);
+ icsk->icsk_user_timeout = val;
break;
case TCP_FASTOPEN:
@@ -3440,7 +3440,7 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
break;
case TCP_USER_TIMEOUT:
- val = jiffies_to_msecs(icsk->icsk_user_timeout);
+ val = icsk->icsk_user_timeout;
break;
case TCP_FASTOPEN:
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 3b3611729928..fa34984d0b12 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -183,8 +183,9 @@ static bool retransmits_timed_out(struct sock *sk,
else
timeout = ((2 << linear_backoff_thresh) - 1) * rto_base +
(boundary - linear_backoff_thresh) * TCP_RTO_MAX;
+ timeout = jiffies_to_msecs(timeout);
}
- return (tcp_time_stamp(tcp_sk(sk)) - start_ts) >= jiffies_to_msecs(timeout);
+ return (tcp_time_stamp(tcp_sk(sk)) - start_ts) >= timeout;
}
/* A write timeout has occurred. Process the after effects. */
@@ -337,8 +338,7 @@ static void tcp_probe_timer(struct sock *sk)
if (!start_ts)
skb->skb_mstamp = tp->tcp_mstamp;
else if (icsk->icsk_user_timeout &&
- (s32)(tcp_time_stamp(tp) - start_ts) >
- jiffies_to_msecs(icsk->icsk_user_timeout))
+ (s32)(tcp_time_stamp(tp) - start_ts) > icsk->icsk_user_timeout)
goto abort;
max_probes = sock_net(sk)->ipv4.sysctl_tcp_retries2;
@@ -672,7 +672,7 @@ static void tcp_keepalive_timer (struct timer_list *t)
* to determine when to timeout instead.
*/
if ((icsk->icsk_user_timeout != 0 &&
- elapsed >= icsk->icsk_user_timeout &&
+ elapsed >= msecs_to_jiffies(icsk->icsk_user_timeout) &&
icsk->icsk_probes_out > 0) ||
(icsk->icsk_user_timeout == 0 &&
icsk->icsk_probes_out >= keepalive_probes(tp))) {
--
2.13.6
^ permalink raw reply related
* [PATCH net-next 0/3] Series to improve setsockopt() TCP_USER_TIMEOUT accuracy
From: Jon Maxwell @ 2018-07-17 4:15 UTC (permalink / raw)
To: davem
Cc: edumazet, ncardwell, David.Laight, kuznet, yoshfuji, netdev,
linux-kernel, jmaxwell
This is a patch series based on:
https://patchwork.kernel.org/patch/10516195/
Every time the TCP retransmission timer fires. It checks to see if there is a
timeout before scheduling the next retransmit timer. The retransmit interval
between each retransmission increases exponentially. The issue is that in order
for the timeout to occur the retransmit timer needs to fire again. If the user
timeout check happens after the 9th retransmit for example. It needs to wait for
the 10th retransmit timer to fire in order to evaluate whether a timeout has
occurred or not. If the interval is large enough then the timeout will be
inaccurate.
For example with a TCP_USER_TIMEOUT of 10 seconds without patch:
1st retransmit:
22:25:18.973488 IP host1.49310 > host2.search-agent: Flags [.]
Last retransmit:
22:25:26.205499 IP host1.49310 > host2.search-agent: Flags [.]
Timeout:
send: Connection timed out
Sun Jul 1 22:25:34 EDT 2018
We can see that last retransmit took ~7 seconds. Which pushed the total
timeout to ~15 seconds instead of the expected 10 seconds. This gets more
inaccurate the larger the TCP_USER_TIMEOUT value. As the interval increases.
Add tcp_clamp_rto_to_user_timeout() to determine if the user rto has expired.
Or whether the rto interval needs to be recalculated. Use the original interval
if user rto is not set.
Test results with the patch is the expected 10 second timeout:
1st retransmit:
01:37:59.022555 IP host1.49310 > host2.search-agent: Flags [.]
Last retransmit:
01:38:06.486558 IP host1.49310 > host2.search-agent: Flags [.]
Timeout:
send: Connection timed out
Mon Jul 2 01:38:09 EDT 2018
Jon Maxwell (3):
[PATCH net-next 1/3] tcp: convert icsk_user_timeout from jiffies to msecs
[PATCH net-next v1 2/3] tcp: convert icsk_user_timeout from jiffies to msecs
[PATCH net-next v1 3/3] tcp: convert icsk_user_timeout from jiffies to msecs
net/ipv4/tcp.c | 4 ++--
net/ipv4/tcp_timer.c | 51 ++++++++++++++++++++++++++++++++++++++-------------
2 files changed, 40 insertions(+), 15 deletions(-)
--
2.13.6
^ 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