* [PATCH 1/2] net/mlx5e: shut up maybe-uninitialized warning
From: Arnd Bergmann @ 2016-09-30 16:17 UTC (permalink / raw)
To: Saeed Mahameed, Matan Barak, Leon Romanovsky
Cc: Arnd Bergmann, David S. Miller, Or Gerlitz, Amir Vadai,
Maor Gottlieb, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Build-testing this driver with -Wmaybe-uninitialized gives a new false-positive
warning that I can't really explain:
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c: In function 'mlx5e_configure_flower':
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:509:3: error: 'old_attr' may be used uninitialized in this function [-Werror=maybe-uninitialized]
It's obvious from the code that 'old_attr' is initialized whenever 'old'
is non-NULL here. The warning appears with all versions I tested from gcc-4.7
through gcc-6.1, and I could not come up with a way to rewrite the function
in a more readable way that avoids the warning, so I'm adding another
initialization to shut it up.
Fixes: 8b32580df1cb ("net/mlx5e: Add TC vlan action for SRIOV offloads")
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index a350b7171e3d..ce8c54d18906 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -451,7 +451,7 @@ int mlx5e_configure_flower(struct mlx5e_priv *priv, __be16 protocol,
struct mlx5e_tc_flow *flow;
struct mlx5_flow_spec *spec;
struct mlx5_flow_rule *old = NULL;
- struct mlx5_esw_flow_attr *old_attr;
+ struct mlx5_esw_flow_attr *old_attr = NULL;
struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
if (esw && esw->mode == SRIOV_OFFLOADS)
--
2.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] cxgb4: unexport cxgb4_dcb_enabled
From: Arnd Bergmann @ 2016-09-30 16:15 UTC (permalink / raw)
To: Hariprasad S
Cc: Arnd Bergmann, David S. Miller, Rahul Lakkireddy,
Nicholas Bellinger, Varun Prakash, netdev, linux-kernel
A recent cleanup marked cxgb4_dcb_enabled as 'static', which is correct, but this ignored
how the symbol is also exported. In addition, the export can be compiled out when modules
are disabled, causing a harmless compiler warning in configurations for which it is not
used at all:
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c:282:12: error: 'cxgb4_dcb_enabled' defined but not used [-Werror=unused-function]
This removes the export and moves the function into the correct #ifdef so we only build
it when there are users.
Fixes: 50935857f878 ("cxgb4: mark symbols static where possible")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index c8a5c434ad2c..59f3772eb49d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -277,11 +277,9 @@ static void dcb_tx_queue_prio_enable(struct net_device *dev, int enable)
txq->dcb_prio = value;
}
}
-#endif /* CONFIG_CHELSIO_T4_DCB */
static int cxgb4_dcb_enabled(const struct net_device *dev)
{
-#ifdef CONFIG_CHELSIO_T4_DCB
struct port_info *pi = netdev_priv(dev);
if (!pi->dcb.enabled)
@@ -289,11 +287,8 @@ static int cxgb4_dcb_enabled(const struct net_device *dev)
return ((pi->dcb.state == CXGB4_DCB_STATE_FW_ALLSYNCED) ||
(pi->dcb.state == CXGB4_DCB_STATE_HOST));
-#else
- return 0;
-#endif
}
-EXPORT_SYMBOL(cxgb4_dcb_enabled);
+#endif /* CONFIG_CHELSIO_T4_DCB */
void t4_os_link_changed(struct adapter *adapter, int port_id, int link_stat)
{
--
2.9.0
^ permalink raw reply related
* pull-request: mac80211-next 2016-09-30
From: Johannes Berg @ 2016-09-30 16:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-wireless
Hi Dave,
Last pull request before the merge window, since it's about to open :)
It seems that everyone finally agreed on the NAN (neighbor awareness
networking) APIs, so we have that, along with some other things.
Let me know if there's any problem.
Thanks,
johannes
The following changes since commit c13ed534b8db543e4d8ead3885f4b06585a5771c:
Merge tag 'mac80211-next-for-davem-2016-09-16' of git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next (2016-09-18 22:29:08 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git tags/mac80211-next-for-davem-2016-09-30
for you to fetch changes up to bb42f2d13ffcd0baed7547b37d05add51fcd50e1:
mac80211: Move reorder-sensitive TX handlers to after TXQ dequeue (2016-09-30 14:46:57 +0200)
----------------------------------------------------------------
This time around, we have
* Neighbor Awareness Networking (NAN) APIs
* a fix for a previous patch that caused memory corruption
in wireless extensions key settings
* beacon rate configuration for AP and mesh
* memory limits for mac80211's internal TXQs
* a (fairly involved) fix for the TXQ vs. crypto problems
* direct cfg80211 driver API for WEP keys
----------------------------------------------------------------
Ayala Beker (9):
cfg80211: add start / stop NAN commands
mac80211: add boilerplate code for start / stop NAN
cfg80211: add add_nan_func / del_nan_func
cfg80211: allow the user space to change current NAN configuration
cfg80211: provide a function to report a match for NAN
cfg80211: Provide an API to report NAN function termination
mac80211: implement nan_change_conf
mac80211: Implement add_nan_func and rm_nan_func
mac80211: Add API to report NAN function match
David Spinadel (1):
cfg80211: Add support for static WEP in the driver
Johannes Berg (2):
cfg80211: add checks for beacon rate, extend to mesh
cfg80211: wext: really don't store non-WEP keys
Pedersen, Thomas (2):
mac80211: add offset_tsf driver op and use it for mesh
mac80211: mesh: decrease max drift
Purushottam Kushwaha (1):
cfg80211: Add support to configure a beacon data rate
Toke Høiland-Jørgensen (5):
mac80211: Move ieee802111_tx_dequeue() to later in tx.c
fq.h: Port memory limit mechanism from fq_codel
mac80211: Export fq memory limit information in debugfs
mac80211: Set lower memory limit for non-VHT devices
mac80211: Move reorder-sensitive TX handlers to after TXQ dequeue
include/net/cfg80211.h | 223 +++++++-
include/net/fq.h | 3 +
include/net/fq_impl.h | 7 +-
include/net/mac80211.h | 75 +++
include/uapi/linux/nl80211.h | 270 ++++++++-
net/mac80211/cfg.c | 208 +++++++
net/mac80211/chan.c | 6 +
net/mac80211/debugfs.c | 8 +
net/mac80211/debugfs_netdev.c | 12 +-
net/mac80211/driver-ops.c | 15 +
net/mac80211/driver-ops.h | 83 +++
net/mac80211/ieee80211_i.h | 26 +
net/mac80211/iface.c | 28 +-
net/mac80211/main.c | 8 +
net/mac80211/mesh_sync.c | 12 +-
net/mac80211/offchannel.c | 4 +-
net/mac80211/rx.c | 7 +-
net/mac80211/sta_info.c | 10 +-
net/mac80211/trace.h | 159 ++++++
net/mac80211/tx.c | 351 ++++++++----
net/mac80211/util.c | 61 ++-
net/wireless/chan.c | 2 +
net/wireless/core.c | 35 ++
net/wireless/core.h | 7 +-
net/wireless/ibss.c | 5 +-
net/wireless/mlme.c | 1 +
net/wireless/nl80211.c | 1220 ++++++++++++++++++++++++++++++++---------
net/wireless/rdev-ops.h | 58 ++
net/wireless/sme.c | 6 +-
net/wireless/trace.h | 90 +++
net/wireless/util.c | 30 +-
net/wireless/wext-compat.c | 14 +-
net/wireless/wext-sme.c | 2 +-
33 files changed, 2648 insertions(+), 398 deletions(-)
^ permalink raw reply
* [PATCH] net: rtnl: avoid uninitialized data in IFLA_VF_VLAN_LIST handling
From: Arnd Bergmann @ 2016-09-30 16:13 UTC (permalink / raw)
To: David S. Miller
Cc: Arnd Bergmann, Roopa Prabhu, Nicolas Dichtel, Nikolay Aleksandrov,
Jiri Pirko, Eric Dumazet, Brenden Blanco, Hannes Frederic Sowa,
Nogah Frankel, netdev, linux-kernel
With the newly added support for IFLA_VF_VLAN_LIST netlink messages,
we get a warning about potential uninitialized variable use in
the parsing of the user input when enabling the -Wmaybe-uninitialized
warning:
net/core/rtnetlink.c: In function 'do_setvfinfo':
net/core/rtnetlink.c:1756:9: error: 'ivvl$' may be used uninitialized in this function [-Werror=maybe-uninitialized]
I have not been able to prove whether it is possible to arrive in
this code with an empty IFLA_VF_VLAN_LIST block, but if we do,
then ndo_set_vf_vlan gets called with uninitialized arguments.
This adds an explicit check for an empty list, making it obvious
to the reader and the compiler that this cannot happen.
Fixes: 79aab093a0b5 ("net: Update API for VF vlan protocol 802.1ad support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/core/rtnetlink.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 3ac8946bf244..b06d2f46b83e 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1753,6 +1753,9 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
len++;
}
+ if (len == 0)
+ return -EINVAL;
+
err = ops->ndo_set_vf_vlan(dev, ivvl[0]->vf, ivvl[0]->vlan,
ivvl[0]->qos, ivvl[0]->vlan_proto);
if (err < 0)
--
2.9.0
^ permalink raw reply related
* [PATCH] rxrpc: split up rxrpc_send_call_packet()
From: Arnd Bergmann @ 2016-09-30 16:11 UTC (permalink / raw)
To: David S. Miller, David Howells; +Cc: Arnd Bergmann, netdev, linux-kernel
A number of reworks went into rxrpc_send_call_packet() recently, which
introduced another warning when built with -Wmaybe-uninitialized:
In file included from ../net/rxrpc/output.c:20:0:
net/rxrpc/output.c: In function 'rxrpc_send_call_packet':
net/rxrpc/ar-internal.h:1187:27: error: 'top' may be used uninitialized in this function [-Werror=maybe-uninitialized]
net/rxrpc/output.c:103:24: note: 'top' was declared here
net/rxrpc/output.c:225:25: error: 'hard_ack' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This is a false positive, but it's also an indication that the function is
getting complex enough that the compiler cannot figure out what it does.
This splits out a rxrpc_send_ack_packet() function for one part of
it, making it more understandable by both humans and the compiler
and avoiding the warning.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Unfortunately, this is a larger rework that I was hoping for, and
I have only build tested it, so please review carefully, or just
discard it and treat it as feedback to the original patch.
---
net/rxrpc/output.c | 164 ++++++++++++++++++++++++++++-------------------------
1 file changed, 88 insertions(+), 76 deletions(-)
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index cf43a715685e..821af21c7159 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -90,6 +90,86 @@ static size_t rxrpc_fill_out_ack(struct rxrpc_call *call,
return top - hard_ack + 3;
}
+static int rxrpc_send_ack_packet(struct rxrpc_call *call,
+ struct rxrpc_connection *conn,
+ struct msghdr *msg,
+ struct rxrpc_pkt_buffer *pkt,
+ struct kvec iov[2])
+{
+ bool ping;
+ size_t len, n;
+ rxrpc_serial_t serial;
+ rxrpc_seq_t hard_ack, top;
+ int ioc, ret;
+
+ spin_lock_bh(&call->lock);
+ if (!call->ackr_reason) {
+ spin_unlock_bh(&call->lock);
+ ret = 0;
+ goto out;
+ }
+ ping = (call->ackr_reason == RXRPC_ACK_PING);
+ n = rxrpc_fill_out_ack(call, pkt, &hard_ack, &top);
+ call->ackr_reason = 0;
+
+ spin_unlock_bh(&call->lock);
+
+ pkt->whdr.flags |= RXRPC_SLOW_START_OK;
+
+ iov[0].iov_len += sizeof(pkt->ack) + n;
+ iov[1].iov_base = &pkt->ackinfo;
+ iov[1].iov_len = sizeof(pkt->ackinfo);
+ len = sizeof(pkt->whdr) + sizeof(pkt->ack) + n + sizeof(pkt->ackinfo);
+ ioc = 2;
+ serial = atomic_inc_return(&conn->serial);
+ pkt->whdr.serial = htonl(serial);
+
+ trace_rxrpc_tx_ack(call, serial,
+ ntohl(pkt->ack.firstPacket),
+ ntohl(pkt->ack.serial),
+ pkt->ack.reason, pkt->ack.nAcks);
+
+ if (ping) {
+ call->ackr_ping = serial;
+ smp_wmb();
+ /* We need to stick a time in before we send the packet in case
+ * the reply gets back before kernel_sendmsg() completes - but
+ * asking UDP to send the packet can take a relatively long
+ * time, so we update the time after, on the assumption that
+ * the packet transmission is more likely to happen towards the
+ * end of the kernel_sendmsg() call.
+ */
+ call->ackr_ping_time = ktime_get_real();
+ set_bit(RXRPC_CALL_PINGING, &call->flags);
+ trace_rxrpc_rtt_tx(call, rxrpc_rtt_tx_ping, serial);
+ }
+ ret = kernel_sendmsg(conn->params.local->socket,
+ msg, iov, ioc, len);
+ if (ping)
+ call->ackr_ping_time = ktime_get_real();
+
+ if (call->state < RXRPC_CALL_COMPLETE) {
+ if (ret < 0) {
+ clear_bit(RXRPC_CALL_PINGING, &call->flags);
+ rxrpc_propose_ACK(call, pkt->ack.reason,
+ ntohs(pkt->ack.maxSkew),
+ ntohl(pkt->ack.serial),
+ true, true,
+ rxrpc_propose_ack_retry_tx);
+ } else {
+ spin_lock_bh(&call->lock);
+ if (after(hard_ack, call->ackr_consumed))
+ call->ackr_consumed = hard_ack;
+ if (after(top, call->ackr_seen))
+ call->ackr_seen = top;
+ spin_unlock_bh(&call->lock);
+ }
+ }
+out:
+ return ret;
+}
+
+
/*
* Send an ACK or ABORT call packet.
*/
@@ -99,11 +179,9 @@ int rxrpc_send_call_packet(struct rxrpc_call *call, u8 type)
struct rxrpc_pkt_buffer *pkt;
struct msghdr msg;
struct kvec iov[2];
- rxrpc_serial_t serial;
- rxrpc_seq_t hard_ack, top;
- size_t len, n;
- bool ping = false;
+ size_t len;
int ioc, ret;
+ rxrpc_serial_t serial;
u32 abort_code;
_enter("%u,%s", call->debug_id, rxrpc_pkts[type]);
@@ -140,96 +218,30 @@ int rxrpc_send_call_packet(struct rxrpc_call *call, u8 type)
iov[0].iov_base = pkt;
iov[0].iov_len = sizeof(pkt->whdr);
- len = sizeof(pkt->whdr);
switch (type) {
case RXRPC_PACKET_TYPE_ACK:
- spin_lock_bh(&call->lock);
- if (!call->ackr_reason) {
- spin_unlock_bh(&call->lock);
- ret = 0;
- goto out;
- }
- ping = (call->ackr_reason == RXRPC_ACK_PING);
- n = rxrpc_fill_out_ack(call, pkt, &hard_ack, &top);
- call->ackr_reason = 0;
-
- spin_unlock_bh(&call->lock);
-
-
- pkt->whdr.flags |= RXRPC_SLOW_START_OK;
-
- iov[0].iov_len += sizeof(pkt->ack) + n;
- iov[1].iov_base = &pkt->ackinfo;
- iov[1].iov_len = sizeof(pkt->ackinfo);
- len += sizeof(pkt->ack) + n + sizeof(pkt->ackinfo);
- ioc = 2;
+ ret = rxrpc_send_ack_packet(call, conn, &msg, pkt, iov);
break;
case RXRPC_PACKET_TYPE_ABORT:
abort_code = call->abort_code;
pkt->abort_code = htonl(abort_code);
iov[0].iov_len += sizeof(pkt->abort_code);
- len += sizeof(pkt->abort_code);
+ len = sizeof(pkt->whdr) + sizeof(pkt->abort_code);
ioc = 1;
+ serial = atomic_inc_return(&conn->serial);
+ pkt->whdr.serial = htonl(serial);
+ ret = kernel_sendmsg(conn->params.local->socket,
+ &msg, iov, ioc, len);
break;
default:
BUG();
ret = -ENOANO;
- goto out;
- }
-
- serial = atomic_inc_return(&conn->serial);
- pkt->whdr.serial = htonl(serial);
- switch (type) {
- case RXRPC_PACKET_TYPE_ACK:
- trace_rxrpc_tx_ack(call, serial,
- ntohl(pkt->ack.firstPacket),
- ntohl(pkt->ack.serial),
- pkt->ack.reason, pkt->ack.nAcks);
break;
}
- if (ping) {
- call->ackr_ping = serial;
- smp_wmb();
- /* We need to stick a time in before we send the packet in case
- * the reply gets back before kernel_sendmsg() completes - but
- * asking UDP to send the packet can take a relatively long
- * time, so we update the time after, on the assumption that
- * the packet transmission is more likely to happen towards the
- * end of the kernel_sendmsg() call.
- */
- call->ackr_ping_time = ktime_get_real();
- set_bit(RXRPC_CALL_PINGING, &call->flags);
- trace_rxrpc_rtt_tx(call, rxrpc_rtt_tx_ping, serial);
- }
- ret = kernel_sendmsg(conn->params.local->socket,
- &msg, iov, ioc, len);
- if (ping)
- call->ackr_ping_time = ktime_get_real();
-
- if (type == RXRPC_PACKET_TYPE_ACK &&
- call->state < RXRPC_CALL_COMPLETE) {
- if (ret < 0) {
- clear_bit(RXRPC_CALL_PINGING, &call->flags);
- rxrpc_propose_ACK(call, pkt->ack.reason,
- ntohs(pkt->ack.maxSkew),
- ntohl(pkt->ack.serial),
- true, true,
- rxrpc_propose_ack_retry_tx);
- } else {
- spin_lock_bh(&call->lock);
- if (after(hard_ack, call->ackr_consumed))
- call->ackr_consumed = hard_ack;
- if (after(top, call->ackr_seen))
- call->ackr_seen = top;
- spin_unlock_bh(&call->lock);
- }
- }
-
-out:
rxrpc_put_connection(conn);
kfree(pkt);
return ret;
--
2.9.0
^ permalink raw reply related
* [PATCH 3/3] netfilter: xt_hashlimit: uses div_u64 for division
From: Arnd Bergmann @ 2016-09-30 16:05 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Arnd Bergmann, Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
Joshua Hunt, Vishwanath Pai, netfilter-devel, coreteam, netdev,
linux-kernel
In-Reply-To: <20160930160559.4102745-1-arnd@arndb.de>
The newly added support for high-resolution pps rates introduced multiple 64-bit
division operations in one function, which fails on all 32-bit architectures:
net/netfilter/xt_hashlimit.o: In function `user2credits':
xt_hashlimit.c:(.text.user2credits+0x3c): undefined reference to `__aeabi_uldivmod'
xt_hashlimit.c:(.text.user2credits+0x68): undefined reference to `__aeabi_uldivmod'
xt_hashlimit.c:(.text.user2credits+0x88): undefined reference to `__aeabi_uldivmod'
This replaces the division with an explicit call to div_u64 for version 2
to documents that this is a slow operation, and reverts back to 32-bit arguments
for the version 1 data to restore the original faster 32-bit division.
With both changes combined, we no longer get a link error.
Fixes: 11d5f15723c9 ("netfilter: xt_hashlimit: Create revision 2 to support higher pps rates")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Vishwanath Pai already sent a patch for this, and I did my version independently.
The difference is that his version also the more expensive division for the
version 1 variant that doesn't need it.
See also http://patchwork.ozlabs.org/patch/676713/
---
net/netfilter/xt_hashlimit.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 44a095ecc7b7..3d5525df6eb3 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -464,20 +464,23 @@ static u32 xt_hashlimit_len_to_chunks(u32 len)
static u64 user2credits(u64 user, int revision)
{
if (revision == 1) {
+ u32 user32 = user; /* use 32-bit division */
+
/* If multiplying would overflow... */
- if (user > 0xFFFFFFFF / (HZ*CREDITS_PER_JIFFY_v1))
+ if (user32 > 0xFFFFFFFF / (HZ*CREDITS_PER_JIFFY_v1))
/* Divide first. */
- return (user / XT_HASHLIMIT_SCALE) *\
+ return (user32 / XT_HASHLIMIT_SCALE) *
HZ * CREDITS_PER_JIFFY_v1;
- return (user * HZ * CREDITS_PER_JIFFY_v1) \
- / XT_HASHLIMIT_SCALE;
+ return (user32 * HZ * CREDITS_PER_JIFFY_v1) /
+ XT_HASHLIMIT_SCALE;
} else {
if (user > 0xFFFFFFFFFFFFFFFF / (HZ*CREDITS_PER_JIFFY))
- return (user / XT_HASHLIMIT_SCALE_v2) *\
- HZ * CREDITS_PER_JIFFY;
+ return div_u64_u64(user, XT_HASHLIMIT_SCALE_v2) *
+ HZ * CREDITS_PER_JIFFY;
- return (user * HZ * CREDITS_PER_JIFFY) / XT_HASHLIMIT_SCALE_v2;
+ return div_u64_u64(user * HZ * CREDITS_PER_JIFFY,
+ XT_HASHLIMIT_SCALE_v2);
}
}
--
2.9.0
^ permalink raw reply related
* [PATCH 2/3] netfilter: hide reference to nf_hooks_ingress
From: Arnd Bergmann @ 2016-09-30 16:05 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Arnd Bergmann, Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
Aaron Conole, Florian Westphal, netfilter-devel, coreteam, netdev,
linux-kernel
In-Reply-To: <20160930160559.4102745-1-arnd@arndb.de>
A recent cleanup added an unconditional reference to the nf_hooks_ingress pointer,
but that fails when CONFIG_NETFILTER_INGRESS is disabled and that member is
not present in net_device:
net/netfilter/core.c: In function 'nf_set_hooks_head':
net/netfilter/core.c:96:30: error: 'struct net_device' has no member named 'nf_hooks_ingress'
This avoids the build error by simply enclosing the assignment in an #ifdef,
which may or may not be the correct fix.
Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked list")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/netfilter/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 576a9c0406a9..5ccff1d9f209 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -90,10 +90,12 @@ static void nf_set_hooks_head(struct net *net, const struct nf_hook_ops *reg,
{
switch (reg->pf) {
case NFPROTO_NETDEV:
+#ifdef CONFIG_NETFILTER_INGRESS
/* We already checked in nf_register_net_hook() that this is
* used from ingress.
*/
rcu_assign_pointer(reg->dev->nf_hooks_ingress, entry);
+#endif
break;
default:
rcu_assign_pointer(net->nf.hooks[reg->pf][reg->hooknum],
--
2.9.0
^ permalink raw reply related
* [PATCH 1/3] netfilter: nf_tables: avoid uninitialized variable warning
From: Arnd Bergmann @ 2016-09-30 16:05 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Arnd Bergmann, Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
netfilter-devel, coreteam, netdev, linux-kernel
The newly added nft_range_eval() function handles the two possible
nft range operations, but as the compiler warning points out,
any unexpected value would lead to the 'mismatch' variable being
used without being initialized:
net/netfilter/nft_range.c: In function 'nft_range_eval':
net/netfilter/nft_range.c:45:5: error: 'mismatch' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This can be trivially avoided by added a 'default:' clause.
Fixes: 0f3cd9b36977 ("netfilter: nf_tables: add range expression")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/netfilter/nft_range.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/netfilter/nft_range.c b/net/netfilter/nft_range.c
index c6d5358482d1..72dff5bffca8 100644
--- a/net/netfilter/nft_range.c
+++ b/net/netfilter/nft_range.c
@@ -40,6 +40,8 @@ static void nft_range_eval(const struct nft_expr *expr,
case NFT_RANGE_NEQ:
mismatch = (d1 >= 0 && d2 <= 0);
break;
+ default:
+ mismatch = 0;
}
if (mismatch)
--
2.9.0
^ permalink raw reply related
* Re: [PATCH net-next 2/2] openvswitch: remove skb_mpls_header
From: pravin shelar @ 2016-09-30 16:05 UTC (permalink / raw)
To: Jiri Benc; +Cc: Linux Kernel Network Developers, David Ahern
In-Reply-To: <20160930100608.1b4308b2@griffin>
On Fri, Sep 30, 2016 at 1:06 AM, Jiri Benc <jbenc@redhat.com> wrote:
> On Thu, 29 Sep 2016 16:03:19 -0700, pravin shelar wrote:
>> > --- a/include/net/mpls.h
>> > +++ b/include/net/mpls.h
>> > @@ -25,15 +25,4 @@ static inline bool eth_p_mpls(__be16 eth_type)
>> > eth_type == htons(ETH_P_MPLS_MC);
>> > }
>> >
>> > -/*
>> > - * For non-MPLS skbs this will correspond to the network header.
>> > - * For MPLS skbs it will be before the network_header as the MPLS
>> > - * label stack lies between the end of the mac header and the network
>> > - * header. That is, for MPLS skbs the end of the mac header
>> > - * is the top of the MPLS label stack.
>> > - */
>> > -static inline unsigned char *skb_mpls_header(struct sk_buff *skb)
>> > -{
>> > - return skb_mac_header(skb) + skb->mac_len;
>> > -}
>>
>> I think we should keep this API, so that it is clear that MPLS header
>> mapped skb network header.
>
> I was pondering this but I don't think it really gains us anything.
> Wrappers like ip_hdr() are useful as they do type conversion; it's much
> nicer to write
> ip_hdr(skb)->daddr
> than
> (struct iphdr *)skb_network_header(skb)->daddr.
>
> But we don't really have a good type to return from skb_mpls_header, it
> boils down to be 100% equivalent to skb_network_header. In fact, you
> could just do:
> #define skb_mpls_header skb_network_header
>
I am fine with this too.
> In the code, I don't think there's much benefit from calling the
> wrapper, meaning of skb_network_header itself is clear enough. It *is*
> the network header, after all. In the whole openvswitch code, MPLS is
> treated as L3 header - no dissection is performed after the MPLS
> headers, the network header and mac_len is set as expected now, etc.
>
It makes it easier to locate which modules are using this API. That
also helps if in future we decide to change MPLS header mapping again.
^ permalink raw reply
* Re: [PATCH v4 net-next 16/16] tcp_bbr: add BBR congestion control
From: Lawrence Brakmo @ 2016-09-30 15:42 UTC (permalink / raw)
To: Neal Cardwell, David Miller
Cc: netdev@vger.kernel.org, Van Jacobson, Yuchung Cheng,
Nandita Dukkipati, Eric Dumazet, Soheil Hassas Yeganeh
In-Reply-To: <1474342763-16715-17-git-send-email-ncardwell@google.com>
I ran some BBR tests under different scenarios and I am concerned that
under some conditions BBR can become very aggressive. It seems BBR flows
can decide that losses are un-correlated to congestion, and when in this
mode, they will maintain large cwnds regardless of the losses. In cases
when BBR is wrong, it will starve non-BBR flows sharing the bottleneck. Im
some of my tests even some BBR flows were starved when some flows got into
this mode and others didn¹t.
I posted the report here:
https://drive.google.com/open?id=0B4YZ_0yTgbJEa21CbUVLWFdrX2c
- Lawrence
On 9/19/16, 8:39 PM, "netdev-owner@vger.kernel.org on behalf of Neal
Cardwell" <netdev-owner@vger.kernel.org on behalf of ncardwell@google.com>
wrote:
>This commit implements a new TCP congestion control algorithm: BBR
>(Bottleneck Bandwidth and RTT). A detailed description of BBR will be
>published in ACM Queue, Vol. 14 No. 5, September-October 2016, as
>"BBR: Congestion-Based Congestion Control".
>
>BBR has significantly increased throughput and reduced latency for
>connections on Google's internal backbone networks and google.com and
>YouTube Web servers.
>
>BBR requires only changes on the sender side, not in the network or
>the receiver side. Thus it can be incrementally deployed on today's
>Internet, or in datacenters.
>
>The Internet has predominantly used loss-based congestion control
>(largely Reno or CUBIC) since the 1980s, relying on packet loss as the
>signal to slow down. While this worked well for many years, loss-based
>congestion control is unfortunately out-dated in today's networks. On
>today's Internet, loss-based congestion control causes the infamous
>bufferbloat problem, often causing seconds of needless queuing delay,
>since it fills the bloated buffers in many last-mile links. On today's
>high-speed long-haul links using commodity switches with shallow
>buffers, loss-based congestion control has abysmal throughput because
>it over-reacts to losses caused by transient traffic bursts.
>
>In 1981 Kleinrock and Gale showed that the optimal operating point for
>a network maximizes delivered bandwidth while minimizing delay and
>loss, not only for single connections but for the network as a
>whole. Finding that optimal operating point has been elusive, since
>any single network measurement is ambiguous: network measurements are
>the result of both bandwidth and propagation delay, and those two
>cannot be measured simultaneously.
>
>While it is impossible to disambiguate any single bandwidth or RTT
>measurement, a connection's behavior over time tells a clearer
>story. BBR uses a measurement strategy designed to resolve this
>ambiguity. It combines these measurements with a robust servo loop
>using recent control systems advances to implement a distributed
>congestion control algorithm that reacts to actual congestion, not
>packet loss or transient queue delay, and is designed to converge with
>high probability to a point near the optimal operating point.
>
>In a nutshell, BBR creates an explicit model of the network pipe by
>sequentially probing the bottleneck bandwidth and RTT. On the arrival
>of each ACK, BBR derives the current delivery rate of the last round
>trip, and feeds it through a windowed max-filter to estimate the
>bottleneck bandwidth. Conversely it uses a windowed min-filter to
>estimate the round trip propagation delay. The max-filtered bandwidth
>and min-filtered RTT estimates form BBR's model of the network pipe.
>
>Using its model, BBR sets control parameters to govern sending
>behavior. The primary control is the pacing rate: BBR applies a gain
>multiplier to transmit faster or slower than the observed bottleneck
>bandwidth. The conventional congestion window (cwnd) is now the
>secondary control; the cwnd is set to a small multiple of the
>estimated BDP (bandwidth-delay product) in order to allow full
>utilization and bandwidth probing while bounding the potential amount
>of queue at the bottleneck.
>
>When a BBR connection starts, it enters STARTUP mode and applies a
>high gain to perform an exponential search to quickly probe the
>bottleneck bandwidth (doubling its sending rate each round trip, like
>slow start). However, instead of continuing until it fills up the
>buffer (i.e. a loss), or until delay or ACK spacing reaches some
>threshold (like Hystart), it uses its model of the pipe to estimate
>when that pipe is full: it estimates the pipe is full when it notices
>the estimated bandwidth has stopped growing. At that point it exits
>STARTUP and enters DRAIN mode, where it reduces its pacing rate to
>drain the queue it estimates it has created.
>
>Then BBR enters steady state. In steady state, PROBE_BW mode cycles
>between first pacing faster to probe for more bandwidth, then pacing
>slower to drain any queue that created if no more bandwidth was
>available, and then cruising at the estimated bandwidth to utilize the
>pipe without creating excess queue. Occasionally, on an as-needed
>basis, it sends significantly slower to probe for RTT (PROBE_RTT
>mode).
>
>BBR has been fully deployed on Google's wide-area backbone networks
>and we're experimenting with BBR on Google.com and YouTube on a global
>scale. Replacing CUBIC with BBR has resulted in significant
>improvements in network latency and application (RPC, browser, and
>video) metrics. For more details please refer to our upcoming ACM
>Queue publication.
>
>Example performance results, to illustrate the difference between BBR
>and CUBIC:
>
>Resilience to random loss (e.g. from shallow buffers):
> Consider a netperf TCP_STREAM test lasting 30 secs on an emulated
> path with a 10Gbps bottleneck, 100ms RTT, and 1% packet loss
> rate. CUBIC gets 3.27 Mbps, and BBR gets 9150 Mbps (2798x higher).
>
>Low latency with the bloated buffers common in today's last-mile links:
> Consider a netperf TCP_STREAM test lasting 120 secs on an emulated
> path with a 10Mbps bottleneck, 40ms RTT, and 1000-packet bottleneck
> buffer. Both fully utilize the bottleneck bandwidth, but BBR
> achieves this with a median RTT 25x lower (43 ms instead of 1.09
> secs).
>
>Our long-term goal is to improve the congestion control algorithms
>used on the Internet. We are hopeful that BBR can help advance the
>efforts toward this goal, and motivate the community to do further
>research.
>
>Test results, performance evaluations, feedback, and BBR-related
>discussions are very welcome in the public e-mail list for BBR:
>
>
>https://urldefense.proofpoint.com/v2/url?u=https-3A__groups.google.com_for
>um_-23-21forum_bbr-2Ddev&d=DQIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=pq_Mqvzfy-C8l
>tkgyx1u_g&m=xV8BAzDZSWNmi4irvSU7Cnf_stojC7Qv3TSqkxYzMK0&s=mWB9nxnt76UWKkpT
>0cioXxwy06b0evTHGgwlI3STNCI&e=
>
>NOTE: BBR *must* be used with the fq qdisc ("man tc-fq") with pacing
>enabled, since pacing is integral to the BBR design and
>implementation. BBR without pacing would not function properly, and
>may incur unnecessary high packet loss rates.
>
>Signed-off-by: Van Jacobson <vanj@google.com>
>Signed-off-by: Neal Cardwell <ncardwell@google.com>
>Signed-off-by: Yuchung Cheng <ycheng@google.com>
>Signed-off-by: Nandita Dukkipati <nanditad@google.com>
>Signed-off-by: Eric Dumazet <edumazet@google.com>
>Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
>---
> include/uapi/linux/inet_diag.h | 13 +
> net/ipv4/Kconfig | 18 +
> net/ipv4/Makefile | 1 +
> net/ipv4/tcp_bbr.c | 896
>+++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 928 insertions(+)
> create mode 100644 net/ipv4/tcp_bbr.c
>
>diff --git a/include/uapi/linux/inet_diag.h
>b/include/uapi/linux/inet_diag.h
>index b5c366f..509cd96 100644
>--- a/include/uapi/linux/inet_diag.h
>+++ b/include/uapi/linux/inet_diag.h
>@@ -124,6 +124,7 @@ enum {
> INET_DIAG_PEERS,
> INET_DIAG_PAD,
> INET_DIAG_MARK,
>+ INET_DIAG_BBRINFO,
> __INET_DIAG_MAX,
> };
>
>@@ -157,8 +158,20 @@ struct tcp_dctcp_info {
> __u32 dctcp_ab_tot;
> };
>
>+/* INET_DIAG_BBRINFO */
>+
>+struct tcp_bbr_info {
>+ /* u64 bw: max-filtered BW (app throughput) estimate in Byte per sec: */
>+ __u32 bbr_bw_lo; /* lower 32 bits of bw */
>+ __u32 bbr_bw_hi; /* upper 32 bits of bw */
>+ __u32 bbr_min_rtt; /* min-filtered RTT in uSec */
>+ __u32 bbr_pacing_gain; /* pacing gain shifted left 8 bits */
>+ __u32 bbr_cwnd_gain; /* cwnd gain shifted left 8 bits */
>+};
>+
> union tcp_cc_info {
> struct tcpvegas_info vegas;
> struct tcp_dctcp_info dctcp;
>+ struct tcp_bbr_info bbr;
> };
> #endif /* _UAPI_INET_DIAG_H_ */
>diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
>index 50d6a9b..300b068 100644
>--- a/net/ipv4/Kconfig
>+++ b/net/ipv4/Kconfig
>@@ -640,6 +640,21 @@ config TCP_CONG_CDG
> D.A. Hayes and G. Armitage. "Revisiting TCP congestion control using
> delay gradients." In Networking 2011. Preprint:
>https://urldefense.proofpoint.com/v2/url?u=http-3A__goo.gl_No3vdg&d=DQIBAg
>&c=5VD0RTtNlTh3ycd41b3MUw&r=pq_Mqvzfy-C8ltkgyx1u_g&m=xV8BAzDZSWNmi4irvSU7C
>nf_stojC7Qv3TSqkxYzMK0&s=cVqj8M0_43G6FqE2LfEGLAe5pfUieb8_YYe2TquWgSA&e=
>
>+config TCP_CONG_BBR
>+ tristate "BBR TCP"
>+ default n
>+ ---help---
>+
>+ BBR (Bottleneck Bandwidth and RTT) TCP congestion control aims to
>+ maximize network utilization and minimize queues. It builds an explicit
>+ model of the the bottleneck delivery rate and path round-trip
>+ propagation delay. It tolerates packet loss and delay unrelated to
>+ congestion. It can operate over LAN, WAN, cellular, wifi, or cable
>+ modem links. It can coexist with flows that use loss-based congestion
>+ control, and can operate with shallow buffers, deep buffers,
>+ bufferbloat, policers, or AQM schemes that do not provide a delay
>+ signal. It requires the fq ("Fair Queue") pacing packet scheduler.
>+
> choice
> prompt "Default TCP congestion control"
> default DEFAULT_CUBIC
>@@ -674,6 +689,9 @@ choice
> config DEFAULT_CDG
> bool "CDG" if TCP_CONG_CDG=y
>
>+ config DEFAULT_BBR
>+ bool "BBR" if TCP_CONG_BBR=y
>+
> config DEFAULT_RENO
> bool "Reno"
> endchoice
>diff --git a/net/ipv4/Makefile b/net/ipv4/Makefile
>index 9cfff1a..bc6a6c8 100644
>--- a/net/ipv4/Makefile
>+++ b/net/ipv4/Makefile
>@@ -41,6 +41,7 @@ obj-$(CONFIG_INET_DIAG) += inet_diag.o
> obj-$(CONFIG_INET_TCP_DIAG) += tcp_diag.o
> obj-$(CONFIG_INET_UDP_DIAG) += udp_diag.o
> obj-$(CONFIG_NET_TCPPROBE) += tcp_probe.o
>+obj-$(CONFIG_TCP_CONG_BBR) += tcp_bbr.o
> obj-$(CONFIG_TCP_CONG_BIC) += tcp_bic.o
> obj-$(CONFIG_TCP_CONG_CDG) += tcp_cdg.o
> obj-$(CONFIG_TCP_CONG_CUBIC) += tcp_cubic.o
>diff --git a/net/ipv4/tcp_bbr.c b/net/ipv4/tcp_bbr.c
>new file mode 100644
>index 0000000..0ea66c2
>--- /dev/null
>+++ b/net/ipv4/tcp_bbr.c
>@@ -0,0 +1,896 @@
>+/* Bottleneck Bandwidth and RTT (BBR) congestion control
>+ *
>+ * BBR congestion control computes the sending rate based on the delivery
>+ * rate (throughput) estimated from ACKs. In a nutshell:
>+ *
>+ * On each ACK, update our model of the network path:
>+ * bottleneck_bandwidth = windowed_max(delivered / elapsed, 10
>round trips)
>+ * min_rtt = windowed_min(rtt, 10 seconds)
>+ * pacing_rate = pacing_gain * bottleneck_bandwidth
>+ * cwnd = max(cwnd_gain * bottleneck_bandwidth * min_rtt, 4)
>+ *
>+ * The core algorithm does not react directly to packet losses or delays,
>+ * although BBR may adjust the size of next send per ACK when loss is
>+ * observed, or adjust the sending rate if it estimates there is a
>+ * traffic policer, in order to keep the drop rate reasonable.
>+ *
>+ * BBR is described in detail in:
>+ * "BBR: Congestion-Based Congestion Control",
>+ * Neal Cardwell, Yuchung Cheng, C. Stephen Gunn, Soheil Hassas
>Yeganeh,
>+ * Van Jacobson. ACM Queue, Vol. 14 No. 5, September-October 2016.
>+ *
>+ * There is a public e-mail list for discussing BBR development and
>testing:
>+ *
>https://urldefense.proofpoint.com/v2/url?u=https-3A__groups.google.com_for
>um_-23-21forum_bbr-2Ddev&d=DQIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=pq_Mqvzfy-C8l
>tkgyx1u_g&m=xV8BAzDZSWNmi4irvSU7Cnf_stojC7Qv3TSqkxYzMK0&s=mWB9nxnt76UWKkpT
>0cioXxwy06b0evTHGgwlI3STNCI&e=
>+ *
>+ * NOTE: BBR *must* be used with the fq qdisc ("man tc-fq") with pacing
>enabled,
>+ * since pacing is integral to the BBR design and implementation.
>+ * BBR without pacing would not function properly, and may incur
>unnecessary
>+ * high packet loss rates.
>+ */
>+#include <linux/module.h>
>+#include <net/tcp.h>
>+#include <linux/inet_diag.h>
>+#include <linux/inet.h>
>+#include <linux/random.h>
>+#include <linux/win_minmax.h>
>+
>+/* Scale factor for rate in pkt/uSec unit to avoid truncation in
>bandwidth
>+ * estimation. The rate unit ~= (1500 bytes / 1 usec / 2^24) ~= 715 bps.
>+ * This handles bandwidths from 0.06pps (715bps) to 256Mpps (3Tbps) in a
>u32.
>+ * Since the minimum window is >=4 packets, the lower bound isn't
>+ * an issue. The upper bound isn't an issue with existing technologies.
>+ */
>+#define BW_SCALE 24
>+#define BW_UNIT (1 << BW_SCALE)
>+
>+#define BBR_SCALE 8 /* scaling factor for fractions in BBR (e.g. gains)
>*/
>+#define BBR_UNIT (1 << BBR_SCALE)
>+
>+/* BBR has the following modes for deciding how fast to send: */
>+enum bbr_mode {
>+ BBR_STARTUP, /* ramp up sending rate rapidly to fill pipe */
>+ BBR_DRAIN, /* drain any queue created during startup */
>+ BBR_PROBE_BW, /* discover, share bw: pace around estimated bw */
>+ BBR_PROBE_RTT, /* cut cwnd to min to probe min_rtt */
>+};
>+
>+/* BBR congestion control block */
>+struct bbr {
>+ u32 min_rtt_us; /* min RTT in min_rtt_win_sec window */
>+ u32 min_rtt_stamp; /* timestamp of min_rtt_us */
>+ u32 probe_rtt_done_stamp; /* end time for BBR_PROBE_RTT mode */
>+ struct minmax bw; /* Max recent delivery rate in pkts/uS << 24 */
>+ u32 rtt_cnt; /* count of packet-timed rounds elapsed */
>+ u32 next_rtt_delivered; /* scb->tx.delivered at end of round */
>+ struct skb_mstamp cycle_mstamp; /* time of this cycle phase start */
>+ u32 mode:3, /* current bbr_mode in state machine */
>+ prev_ca_state:3, /* CA state on previous ACK */
>+ packet_conservation:1, /* use packet conservation? */
>+ restore_cwnd:1, /* decided to revert cwnd to old value */
>+ round_start:1, /* start of packet-timed tx->ack round? */
>+ tso_segs_goal:7, /* segments we want in each skb we send */
>+ idle_restart:1, /* restarting after idle? */
>+ probe_rtt_round_done:1, /* a BBR_PROBE_RTT round at 4 pkts? */
>+ unused:5,
>+ lt_is_sampling:1, /* taking long-term ("LT") samples now? */
>+ lt_rtt_cnt:7, /* round trips in long-term interval */
>+ lt_use_bw:1; /* use lt_bw as our bw estimate? */
>+ u32 lt_bw; /* LT est delivery rate in pkts/uS << 24 */
>+ u32 lt_last_delivered; /* LT intvl start: tp->delivered */
>+ u32 lt_last_stamp; /* LT intvl start: tp->delivered_mstamp */
>+ u32 lt_last_lost; /* LT intvl start: tp->lost */
>+ u32 pacing_gain:10, /* current gain for setting pacing rate */
>+ cwnd_gain:10, /* current gain for setting cwnd */
>+ full_bw_cnt:3, /* number of rounds without large bw gains */
>+ cycle_idx:3, /* current index in pacing_gain cycle array */
>+ unused_b:6;
>+ u32 prior_cwnd; /* prior cwnd upon entering loss recovery */
>+ u32 full_bw; /* recent bw, to estimate if pipe is full */
>+};
>+
>+#define CYCLE_LEN 8 /* number of phases in a pacing gain cycle */
>+
>+/* Window length of bw filter (in rounds): */
>+static const int bbr_bw_rtts = CYCLE_LEN + 2;
>+/* Window length of min_rtt filter (in sec): */
>+static const u32 bbr_min_rtt_win_sec = 10;
>+/* Minimum time (in ms) spent at bbr_cwnd_min_target in BBR_PROBE_RTT
>mode: */
>+static const u32 bbr_probe_rtt_mode_ms = 200;
>+/* Skip TSO below the following bandwidth (bits/sec): */
>+static const int bbr_min_tso_rate = 1200000;
>+
>+/* We use a high_gain value of 2/ln(2) because it's the smallest pacing
>gain
>+ * that will allow a smoothly increasing pacing rate that will double
>each RTT
>+ * and send the same number of packets per RTT that an un-paced,
>slow-starting
>+ * Reno or CUBIC flow would:
>+ */
>+static const int bbr_high_gain = BBR_UNIT * 2885 / 1000 + 1;
>+/* The pacing gain of 1/high_gain in BBR_DRAIN is calculated to
>typically drain
>+ * the queue created in BBR_STARTUP in a single round:
>+ */
>+static const int bbr_drain_gain = BBR_UNIT * 1000 / 2885;
>+/* The gain for deriving steady-state cwnd tolerates delayed/stretched
>ACKs: */
>+static const int bbr_cwnd_gain = BBR_UNIT * 2;
>+/* The pacing_gain values for the PROBE_BW gain cycle, to discover/share
>bw: */
>+static const int bbr_pacing_gain[] = {
>+ BBR_UNIT * 5 / 4, /* probe for more available bw */
>+ BBR_UNIT * 3 / 4, /* drain queue and/or yield bw to other flows */
>+ BBR_UNIT, BBR_UNIT, BBR_UNIT, /* cruise at 1.0*bw to utilize pipe, */
>+ BBR_UNIT, BBR_UNIT, BBR_UNIT /* without creating excess queue... */
>+};
>+/* Randomize the starting gain cycling phase over N phases: */
>+static const u32 bbr_cycle_rand = 7;
>+
>+/* Try to keep at least this many packets in flight, if things go
>smoothly. For
>+ * smooth functioning, a sliding window protocol ACKing every other
>packet
>+ * needs at least 4 packets in flight:
>+ */
>+static const u32 bbr_cwnd_min_target = 4;
>+
>+/* To estimate if BBR_STARTUP mode (i.e. high_gain) has filled pipe... */
>+/* If bw has increased significantly (1.25x), there may be more bw
>available: */
>+static const u32 bbr_full_bw_thresh = BBR_UNIT * 5 / 4;
>+/* But after 3 rounds w/o significant bw growth, estimate pipe is full:
>*/
>+static const u32 bbr_full_bw_cnt = 3;
>+
>+/* "long-term" ("LT") bandwidth estimator parameters... */
>+/* The minimum number of rounds in an LT bw sampling interval: */
>+static const u32 bbr_lt_intvl_min_rtts = 4;
>+/* If lost/delivered ratio > 20%, interval is "lossy" and we may be
>policed: */
>+static const u32 bbr_lt_loss_thresh = 50;
>+/* If 2 intervals have a bw ratio <= 1/8, their bw is "consistent": */
>+static const u32 bbr_lt_bw_ratio = BBR_UNIT / 8;
>+/* If 2 intervals have a bw diff <= 4 Kbit/sec their bw is "consistent":
>*/
>+static const u32 bbr_lt_bw_diff = 4000 / 8;
>+/* If we estimate we're policed, use lt_bw for this many round trips: */
>+static const u32 bbr_lt_bw_max_rtts = 48;
>+
>+/* Do we estimate that STARTUP filled the pipe? */
>+static bool bbr_full_bw_reached(const struct sock *sk)
>+{
>+ const struct bbr *bbr = inet_csk_ca(sk);
>+
>+ return bbr->full_bw_cnt >= bbr_full_bw_cnt;
>+}
>+
>+/* Return the windowed max recent bandwidth sample, in pkts/uS <<
>BW_SCALE. */
>+static u32 bbr_max_bw(const struct sock *sk)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+
>+ return minmax_get(&bbr->bw);
>+}
>+
>+/* Return the estimated bandwidth of the path, in pkts/uS << BW_SCALE. */
>+static u32 bbr_bw(const struct sock *sk)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+
>+ return bbr->lt_use_bw ? bbr->lt_bw : bbr_max_bw(sk);
>+}
>+
>+/* Return rate in bytes per second, optionally with a gain.
>+ * The order here is chosen carefully to avoid overflow of u64. This
>should
>+ * work for input rates of up to 2.9Tbit/sec and gain of 2.89x.
>+ */
>+static u64 bbr_rate_bytes_per_sec(struct sock *sk, u64 rate, int gain)
>+{
>+ rate *= tcp_mss_to_mtu(sk, tcp_sk(sk)->mss_cache);
>+ rate *= gain;
>+ rate >>= BBR_SCALE;
>+ rate *= USEC_PER_SEC;
>+ return rate >> BW_SCALE;
>+}
>+
>+/* Pace using current bw estimate and a gain factor. In order to help
>drive the
>+ * network toward lower queues while maintaining high utilization and low
>+ * latency, the average pacing rate aims to be slightly (~1%) lower than
>the
>+ * estimated bandwidth. This is an important aspect of the design. In
>this
>+ * implementation this slightly lower pacing rate is achieved implicitly
>by not
>+ * including link-layer headers in the packet size used for the pacing
>rate.
>+ */
>+static void bbr_set_pacing_rate(struct sock *sk, u32 bw, int gain)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ u64 rate = bw;
>+
>+ rate = bbr_rate_bytes_per_sec(sk, rate, gain);
>+ rate = min_t(u64, rate, sk->sk_max_pacing_rate);
>+ if (bbr->mode != BBR_STARTUP || rate > sk->sk_pacing_rate)
>+ sk->sk_pacing_rate = rate;
>+}
>+
>+/* Return count of segments we want in the skbs we send, or 0 for
>default. */
>+static u32 bbr_tso_segs_goal(struct sock *sk)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+
>+ return bbr->tso_segs_goal;
>+}
>+
>+static void bbr_set_tso_segs_goal(struct sock *sk)
>+{
>+ struct tcp_sock *tp = tcp_sk(sk);
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ u32 min_segs;
>+
>+ min_segs = sk->sk_pacing_rate < (bbr_min_tso_rate >> 3) ? 1 : 2;
>+ bbr->tso_segs_goal = min(tcp_tso_autosize(sk, tp->mss_cache, min_segs),
>+ 0x7FU);
>+}
>+
>+/* Save "last known good" cwnd so we can restore it after losses or
>PROBE_RTT */
>+static void bbr_save_cwnd(struct sock *sk)
>+{
>+ struct tcp_sock *tp = tcp_sk(sk);
>+ struct bbr *bbr = inet_csk_ca(sk);
>+
>+ if (bbr->prev_ca_state < TCP_CA_Recovery && bbr->mode != BBR_PROBE_RTT)
>+ bbr->prior_cwnd = tp->snd_cwnd; /* this cwnd is good enough */
>+ else /* loss recovery or BBR_PROBE_RTT have temporarily cut cwnd */
>+ bbr->prior_cwnd = max(bbr->prior_cwnd, tp->snd_cwnd);
>+}
>+
>+static void bbr_cwnd_event(struct sock *sk, enum tcp_ca_event event)
>+{
>+ struct tcp_sock *tp = tcp_sk(sk);
>+ struct bbr *bbr = inet_csk_ca(sk);
>+
>+ if (event == CA_EVENT_TX_START && tp->app_limited) {
>+ bbr->idle_restart = 1;
>+ /* Avoid pointless buffer overflows: pace at est. bw if we don't
>+ * need more speed (we're restarting from idle and app-limited).
>+ */
>+ if (bbr->mode == BBR_PROBE_BW)
>+ bbr_set_pacing_rate(sk, bbr_bw(sk), BBR_UNIT);
>+ }
>+}
>+
>+/* Find target cwnd. Right-size the cwnd based on min RTT and the
>+ * estimated bottleneck bandwidth:
>+ *
>+ * cwnd = bw * min_rtt * gain = BDP * gain
>+ *
>+ * The key factor, gain, controls the amount of queue. While a small gain
>+ * builds a smaller queue, it becomes more vulnerable to noise in RTT
>+ * measurements (e.g., delayed ACKs or other ACK compression effects).
>This
>+ * noise may cause BBR to under-estimate the rate.
>+ *
>+ * To achieve full performance in high-speed paths, we budget enough
>cwnd to
>+ * fit full-sized skbs in-flight on both end hosts to fully utilize the
>path:
>+ * - one skb in sending host Qdisc,
>+ * - one skb in sending host TSO/GSO engine
>+ * - one skb being received by receiver host LRO/GRO/delayed-ACK engine
>+ * Don't worry, at low rates (bbr_min_tso_rate) this won't bloat cwnd
>because
>+ * in such cases tso_segs_goal is 1. The minimum cwnd is 4 packets,
>+ * which allows 2 outstanding 2-packet sequences, to try to keep pipe
>+ * full even with ACK-every-other-packet delayed ACKs.
>+ */
>+static u32 bbr_target_cwnd(struct sock *sk, u32 bw, int gain)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ u32 cwnd;
>+ u64 w;
>+
>+ /* If we've never had a valid RTT sample, cap cwnd at the initial
>+ * default. This should only happen when the connection is not using TCP
>+ * timestamps and has retransmitted all of the SYN/SYNACK/data packets
>+ * ACKed so far. In this case, an RTO can cut cwnd to 1, in which
>+ * case we need to slow-start up toward something safe: TCP_INIT_CWND.
>+ */
>+ if (unlikely(bbr->min_rtt_us == ~0U)) /* no valid RTT samples yet? */
>+ return TCP_INIT_CWND; /* be safe: cap at default initial cwnd*/
>+
>+ w = (u64)bw * bbr->min_rtt_us;
>+
>+ /* Apply a gain to the given value, then remove the BW_SCALE shift. */
>+ cwnd = (((w * gain) >> BBR_SCALE) + BW_UNIT - 1) / BW_UNIT;
>+
>+ /* Allow enough full-sized skbs in flight to utilize end systems. */
>+ cwnd += 3 * bbr->tso_segs_goal;
>+
>+ /* Reduce delayed ACKs by rounding up cwnd to the next even number. */
>+ cwnd = (cwnd + 1) & ~1U;
>+
>+ return cwnd;
>+}
>+
>+/* An optimization in BBR to reduce losses: On the first round of
>recovery, we
>+ * follow the packet conservation principle: send P packets per P
>packets acked.
>+ * After that, we slow-start and send at most 2*P packets per P packets
>acked.
>+ * After recovery finishes, or upon undo, we restore the cwnd we had when
>+ * recovery started (capped by the target cwnd based on estimated BDP).
>+ *
>+ * TODO(ycheng/ncardwell): implement a rate-based approach.
>+ */
>+static bool bbr_set_cwnd_to_recover_or_restore(
>+ struct sock *sk, const struct rate_sample *rs, u32 acked, u32 *new_cwnd)
>+{
>+ struct tcp_sock *tp = tcp_sk(sk);
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ u8 prev_state = bbr->prev_ca_state, state = inet_csk(sk)->icsk_ca_state;
>+ u32 cwnd = tp->snd_cwnd;
>+
>+ /* An ACK for P pkts should release at most 2*P packets. We do this
>+ * in two steps. First, here we deduct the number of lost packets.
>+ * Then, in bbr_set_cwnd() we slow start up toward the target cwnd.
>+ */
>+ if (rs->losses > 0)
>+ cwnd = max_t(s32, cwnd - rs->losses, 1);
>+
>+ if (state == TCP_CA_Recovery && prev_state != TCP_CA_Recovery) {
>+ /* Starting 1st round of Recovery, so do packet conservation. */
>+ bbr->packet_conservation = 1;
>+ bbr->next_rtt_delivered = tp->delivered; /* start round now */
>+ /* Cut unused cwnd from app behavior, TSQ, or TSO deferral: */
>+ cwnd = tcp_packets_in_flight(tp) + acked;
>+ } else if (prev_state >= TCP_CA_Recovery && state < TCP_CA_Recovery) {
>+ /* Exiting loss recovery; restore cwnd saved before recovery. */
>+ bbr->restore_cwnd = 1;
>+ bbr->packet_conservation = 0;
>+ }
>+ bbr->prev_ca_state = state;
>+
>+ if (bbr->restore_cwnd) {
>+ /* Restore cwnd after exiting loss recovery or PROBE_RTT. */
>+ cwnd = max(cwnd, bbr->prior_cwnd);
>+ bbr->restore_cwnd = 0;
>+ }
>+
>+ if (bbr->packet_conservation) {
>+ *new_cwnd = max(cwnd, tcp_packets_in_flight(tp) + acked);
>+ return true; /* yes, using packet conservation */
>+ }
>+ *new_cwnd = cwnd;
>+ return false;
>+}
>+
>+/* Slow-start up toward target cwnd (if bw estimate is growing, or
>packet loss
>+ * has drawn us down below target), or snap down to target if we're
>above it.
>+ */
>+static void bbr_set_cwnd(struct sock *sk, const struct rate_sample *rs,
>+ u32 acked, u32 bw, int gain)
>+{
>+ struct tcp_sock *tp = tcp_sk(sk);
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ u32 cwnd = 0, target_cwnd = 0;
>+
>+ if (!acked)
>+ return;
>+
>+ if (bbr_set_cwnd_to_recover_or_restore(sk, rs, acked, &cwnd))
>+ goto done;
>+
>+ /* If we're below target cwnd, slow start cwnd toward target cwnd. */
>+ target_cwnd = bbr_target_cwnd(sk, bw, gain);
>+ if (bbr_full_bw_reached(sk)) /* only cut cwnd if we filled the pipe */
>+ cwnd = min(cwnd + acked, target_cwnd);
>+ else if (cwnd < target_cwnd || tp->delivered < TCP_INIT_CWND)
>+ cwnd = cwnd + acked;
>+ cwnd = max(cwnd, bbr_cwnd_min_target);
>+
>+done:
>+ tp->snd_cwnd = min(cwnd, tp->snd_cwnd_clamp); /* apply global cap */
>+ if (bbr->mode == BBR_PROBE_RTT) /* drain queue, refresh min_rtt */
>+ tp->snd_cwnd = min(tp->snd_cwnd, bbr_cwnd_min_target);
>+}
>+
>+/* End cycle phase if it's time and/or we hit the phase's in-flight
>target. */
>+static bool bbr_is_next_cycle_phase(struct sock *sk,
>+ const struct rate_sample *rs)
>+{
>+ struct tcp_sock *tp = tcp_sk(sk);
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ bool is_full_length =
>+ skb_mstamp_us_delta(&tp->delivered_mstamp, &bbr->cycle_mstamp) >
>+ bbr->min_rtt_us;
>+ u32 inflight, bw;
>+
>+ /* The pacing_gain of 1.0 paces at the estimated bw to try to fully
>+ * use the pipe without increasing the queue.
>+ */
>+ if (bbr->pacing_gain == BBR_UNIT)
>+ return is_full_length; /* just use wall clock time */
>+
>+ inflight = rs->prior_in_flight; /* what was in-flight before ACK? */
>+ bw = bbr_max_bw(sk);
>+
>+ /* A pacing_gain > 1.0 probes for bw by trying to raise inflight to at
>+ * least pacing_gain*BDP; this may take more than min_rtt if min_rtt is
>+ * small (e.g. on a LAN). We do not persist if packets are lost, since
>+ * a path with small buffers may not hold that much.
>+ */
>+ if (bbr->pacing_gain > BBR_UNIT)
>+ return is_full_length &&
>+ (rs->losses || /* perhaps pacing_gain*BDP won't fit */
>+ inflight >= bbr_target_cwnd(sk, bw, bbr->pacing_gain));
>+
>+ /* A pacing_gain < 1.0 tries to drain extra queue we added if bw
>+ * probing didn't find more bw. If inflight falls to match BDP then we
>+ * estimate queue is drained; persisting would underutilize the pipe.
>+ */
>+ return is_full_length ||
>+ inflight <= bbr_target_cwnd(sk, bw, BBR_UNIT);
>+}
>+
>+static void bbr_advance_cycle_phase(struct sock *sk)
>+{
>+ struct tcp_sock *tp = tcp_sk(sk);
>+ struct bbr *bbr = inet_csk_ca(sk);
>+
>+ bbr->cycle_idx = (bbr->cycle_idx + 1) & (CYCLE_LEN - 1);
>+ bbr->cycle_mstamp = tp->delivered_mstamp;
>+ bbr->pacing_gain = bbr_pacing_gain[bbr->cycle_idx];
>+}
>+
>+/* Gain cycling: cycle pacing gain to converge to fair share of
>available bw. */
>+static void bbr_update_cycle_phase(struct sock *sk,
>+ const struct rate_sample *rs)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+
>+ if ((bbr->mode == BBR_PROBE_BW) && !bbr->lt_use_bw &&
>+ bbr_is_next_cycle_phase(sk, rs))
>+ bbr_advance_cycle_phase(sk);
>+}
>+
>+static void bbr_reset_startup_mode(struct sock *sk)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+
>+ bbr->mode = BBR_STARTUP;
>+ bbr->pacing_gain = bbr_high_gain;
>+ bbr->cwnd_gain = bbr_high_gain;
>+}
>+
>+static void bbr_reset_probe_bw_mode(struct sock *sk)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+
>+ bbr->mode = BBR_PROBE_BW;
>+ bbr->pacing_gain = BBR_UNIT;
>+ bbr->cwnd_gain = bbr_cwnd_gain;
>+ bbr->cycle_idx = CYCLE_LEN - 1 - prandom_u32_max(bbr_cycle_rand);
>+ bbr_advance_cycle_phase(sk); /* flip to next phase of gain cycle */
>+}
>+
>+static void bbr_reset_mode(struct sock *sk)
>+{
>+ if (!bbr_full_bw_reached(sk))
>+ bbr_reset_startup_mode(sk);
>+ else
>+ bbr_reset_probe_bw_mode(sk);
>+}
>+
>+/* Start a new long-term sampling interval. */
>+static void bbr_reset_lt_bw_sampling_interval(struct sock *sk)
>+{
>+ struct tcp_sock *tp = tcp_sk(sk);
>+ struct bbr *bbr = inet_csk_ca(sk);
>+
>+ bbr->lt_last_stamp = tp->delivered_mstamp.stamp_jiffies;
>+ bbr->lt_last_delivered = tp->delivered;
>+ bbr->lt_last_lost = tp->lost;
>+ bbr->lt_rtt_cnt = 0;
>+}
>+
>+/* Completely reset long-term bandwidth sampling. */
>+static void bbr_reset_lt_bw_sampling(struct sock *sk)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+
>+ bbr->lt_bw = 0;
>+ bbr->lt_use_bw = 0;
>+ bbr->lt_is_sampling = false;
>+ bbr_reset_lt_bw_sampling_interval(sk);
>+}
>+
>+/* Long-term bw sampling interval is done. Estimate whether we're
>policed. */
>+static void bbr_lt_bw_interval_done(struct sock *sk, u32 bw)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ u32 diff;
>+
>+ if (bbr->lt_bw) { /* do we have bw from a previous interval? */
>+ /* Is new bw close to the lt_bw from the previous interval? */
>+ diff = abs(bw - bbr->lt_bw);
>+ if ((diff * BBR_UNIT <= bbr_lt_bw_ratio * bbr->lt_bw) ||
>+ (bbr_rate_bytes_per_sec(sk, diff, BBR_UNIT) <=
>+ bbr_lt_bw_diff)) {
>+ /* All criteria are met; estimate we're policed. */
>+ bbr->lt_bw = (bw + bbr->lt_bw) >> 1; /* avg 2 intvls */
>+ bbr->lt_use_bw = 1;
>+ bbr->pacing_gain = BBR_UNIT; /* try to avoid drops */
>+ bbr->lt_rtt_cnt = 0;
>+ return;
>+ }
>+ }
>+ bbr->lt_bw = bw;
>+ bbr_reset_lt_bw_sampling_interval(sk);
>+}
>+
>+/* Token-bucket traffic policers are common (see "An Internet-Wide
>Analysis of
>+ * Traffic Policing", SIGCOMM 2016). BBR detects token-bucket policers
>and
>+ * explicitly models their policed rate, to reduce unnecessary losses. We
>+ * estimate that we're policed if we see 2 consecutive sampling
>intervals with
>+ * consistent throughput and high packet loss. If we think we're being
>policed,
>+ * set lt_bw to the "long-term" average delivery rate from those 2
>intervals.
>+ */
>+static void bbr_lt_bw_sampling(struct sock *sk, const struct rate_sample
>*rs)
>+{
>+ struct tcp_sock *tp = tcp_sk(sk);
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ u32 lost, delivered;
>+ u64 bw;
>+ s32 t;
>+
>+ if (bbr->lt_use_bw) { /* already using long-term rate, lt_bw? */
>+ if (bbr->mode == BBR_PROBE_BW && bbr->round_start &&
>+ ++bbr->lt_rtt_cnt >= bbr_lt_bw_max_rtts) {
>+ bbr_reset_lt_bw_sampling(sk); /* stop using lt_bw */
>+ bbr_reset_probe_bw_mode(sk); /* restart gain cycling */
>+ }
>+ return;
>+ }
>+
>+ /* Wait for the first loss before sampling, to let the policer exhaust
>+ * its tokens and estimate the steady-state rate allowed by the policer.
>+ * Starting samples earlier includes bursts that over-estimate the bw.
>+ */
>+ if (!bbr->lt_is_sampling) {
>+ if (!rs->losses)
>+ return;
>+ bbr_reset_lt_bw_sampling_interval(sk);
>+ bbr->lt_is_sampling = true;
>+ }
>+
>+ /* To avoid underestimates, reset sampling if we run out of data. */
>+ if (rs->is_app_limited) {
>+ bbr_reset_lt_bw_sampling(sk);
>+ return;
>+ }
>+
>+ if (bbr->round_start)
>+ bbr->lt_rtt_cnt++; /* count round trips in this interval */
>+ if (bbr->lt_rtt_cnt < bbr_lt_intvl_min_rtts)
>+ return; /* sampling interval needs to be longer */
>+ if (bbr->lt_rtt_cnt > 4 * bbr_lt_intvl_min_rtts) {
>+ bbr_reset_lt_bw_sampling(sk); /* interval is too long */
>+ return;
>+ }
>+
>+ /* End sampling interval when a packet is lost, so we estimate the
>+ * policer tokens were exhausted. Stopping the sampling before the
>+ * tokens are exhausted under-estimates the policed rate.
>+ */
>+ if (!rs->losses)
>+ return;
>+
>+ /* Calculate packets lost and delivered in sampling interval. */
>+ lost = tp->lost - bbr->lt_last_lost;
>+ delivered = tp->delivered - bbr->lt_last_delivered;
>+ /* Is loss rate (lost/delivered) >= lt_loss_thresh? If not, wait. */
>+ if (!delivered || (lost << BBR_SCALE) < bbr_lt_loss_thresh * delivered)
>+ return;
>+
>+ /* Find average delivery rate in this sampling interval. */
>+ t = (s32)(tp->delivered_mstamp.stamp_jiffies - bbr->lt_last_stamp);
>+ if (t < 1)
>+ return; /* interval is less than one jiffy, so wait */
>+ t = jiffies_to_usecs(t);
>+ /* Interval long enough for jiffies_to_usecs() to return a bogus 0? */
>+ if (t < 1) {
>+ bbr_reset_lt_bw_sampling(sk); /* interval too long; reset */
>+ return;
>+ }
>+ bw = (u64)delivered * BW_UNIT;
>+ do_div(bw, t);
>+ bbr_lt_bw_interval_done(sk, bw);
>+}
>+
>+/* Estimate the bandwidth based on how fast packets are delivered */
>+static void bbr_update_bw(struct sock *sk, const struct rate_sample *rs)
>+{
>+ struct tcp_sock *tp = tcp_sk(sk);
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ u64 bw;
>+
>+ bbr->round_start = 0;
>+ if (rs->delivered < 0 || rs->interval_us <= 0)
>+ return; /* Not a valid observation */
>+
>+ /* See if we've reached the next RTT */
>+ if (!before(rs->prior_delivered, bbr->next_rtt_delivered)) {
>+ bbr->next_rtt_delivered = tp->delivered;
>+ bbr->rtt_cnt++;
>+ bbr->round_start = 1;
>+ bbr->packet_conservation = 0;
>+ }
>+
>+ bbr_lt_bw_sampling(sk, rs);
>+
>+ /* Divide delivered by the interval to find a (lower bound) bottleneck
>+ * bandwidth sample. Delivered is in packets and interval_us in uS and
>+ * ratio will be <<1 for most connections. So delivered is first scaled.
>+ */
>+ bw = (u64)rs->delivered * BW_UNIT;
>+ do_div(bw, rs->interval_us);
>+
>+ /* If this sample is application-limited, it is likely to have a very
>+ * low delivered count that represents application behavior rather than
>+ * the available network rate. Such a sample could drag down estimated
>+ * bw, causing needless slow-down. Thus, to continue to send at the
>+ * last measured network rate, we filter out app-limited samples unless
>+ * they describe the path bw at least as well as our bw model.
>+ *
>+ * So the goal during app-limited phase is to proceed with the best
>+ * network rate no matter how long. We automatically leave this
>+ * phase when app writes faster than the network can deliver :)
>+ */
>+ if (!rs->is_app_limited || bw >= bbr_max_bw(sk)) {
>+ /* Incorporate new sample into our max bw filter. */
>+ minmax_running_max(&bbr->bw, bbr_bw_rtts, bbr->rtt_cnt, bw);
>+ }
>+}
>+
>+/* Estimate when the pipe is full, using the change in delivery rate: BBR
>+ * estimates that STARTUP filled the pipe if the estimated bw hasn't
>changed by
>+ * at least bbr_full_bw_thresh (25%) after bbr_full_bw_cnt (3)
>non-app-limited
>+ * rounds. Why 3 rounds: 1: rwin autotuning grows the rwin, 2: we fill
>the
>+ * higher rwin, 3: we get higher delivery rate samples. Or transient
>+ * cross-traffic or radio noise can go away. CUBIC Hystart shares a
>similar
>+ * design goal, but uses delay and inter-ACK spacing instead of
>bandwidth.
>+ */
>+static void bbr_check_full_bw_reached(struct sock *sk,
>+ const struct rate_sample *rs)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ u32 bw_thresh;
>+
>+ if (bbr_full_bw_reached(sk) || !bbr->round_start || rs->is_app_limited)
>+ return;
>+
>+ bw_thresh = (u64)bbr->full_bw * bbr_full_bw_thresh >> BBR_SCALE;
>+ if (bbr_max_bw(sk) >= bw_thresh) {
>+ bbr->full_bw = bbr_max_bw(sk);
>+ bbr->full_bw_cnt = 0;
>+ return;
>+ }
>+ ++bbr->full_bw_cnt;
>+}
>+
>+/* If pipe is probably full, drain the queue and then enter
>steady-state. */
>+static void bbr_check_drain(struct sock *sk, const struct rate_sample
>*rs)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+
>+ if (bbr->mode == BBR_STARTUP && bbr_full_bw_reached(sk)) {
>+ bbr->mode = BBR_DRAIN; /* drain queue we created */
>+ bbr->pacing_gain = bbr_drain_gain; /* pace slow to drain */
>+ bbr->cwnd_gain = bbr_high_gain; /* maintain cwnd */
>+ } /* fall through to check if in-flight is already small: */
>+ if (bbr->mode == BBR_DRAIN &&
>+ tcp_packets_in_flight(tcp_sk(sk)) <=
>+ bbr_target_cwnd(sk, bbr_max_bw(sk), BBR_UNIT))
>+ bbr_reset_probe_bw_mode(sk); /* we estimate queue is drained */
>+}
>+
>+/* The goal of PROBE_RTT mode is to have BBR flows cooperatively and
>+ * periodically drain the bottleneck queue, to converge to measure the
>true
>+ * min_rtt (unloaded propagation delay). This allows the flows to keep
>queues
>+ * small (reducing queuing delay and packet loss) and achieve fairness
>among
>+ * BBR flows.
>+ *
>+ * The min_rtt filter window is 10 seconds. When the min_rtt estimate
>expires,
>+ * we enter PROBE_RTT mode and cap the cwnd at bbr_cwnd_min_target=4
>packets.
>+ * After at least bbr_probe_rtt_mode_ms=200ms and at least one
>packet-timed
>+ * round trip elapsed with that flight size <= 4, we leave PROBE_RTT
>mode and
>+ * re-enter the previous mode. BBR uses 200ms to approximately bound the
>+ * performance penalty of PROBE_RTT's cwnd capping to roughly 2%
>(200ms/10s).
>+ *
>+ * Note that flows need only pay 2% if they are busy sending over the
>last 10
>+ * seconds. Interactive applications (e.g., Web, RPCs, video chunks)
>often have
>+ * natural silences or low-rate periods within 10 seconds where the rate
>is low
>+ * enough for long enough to drain its queue in the bottleneck. We pick
>up
>+ * these min RTT measurements opportunistically with our min_rtt filter.
>:-)
>+ */
>+static void bbr_update_min_rtt(struct sock *sk, const struct rate_sample
>*rs)
>+{
>+ struct tcp_sock *tp = tcp_sk(sk);
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ bool filter_expired;
>+
>+ /* Track min RTT seen in the min_rtt_win_sec filter window: */
>+ filter_expired = after(tcp_time_stamp,
>+ bbr->min_rtt_stamp + bbr_min_rtt_win_sec * HZ);
>+ if (rs->rtt_us >= 0 &&
>+ (rs->rtt_us <= bbr->min_rtt_us || filter_expired)) {
>+ bbr->min_rtt_us = rs->rtt_us;
>+ bbr->min_rtt_stamp = tcp_time_stamp;
>+ }
>+
>+ if (bbr_probe_rtt_mode_ms > 0 && filter_expired &&
>+ !bbr->idle_restart && bbr->mode != BBR_PROBE_RTT) {
>+ bbr->mode = BBR_PROBE_RTT; /* dip, drain queue */
>+ bbr->pacing_gain = BBR_UNIT;
>+ bbr->cwnd_gain = BBR_UNIT;
>+ bbr_save_cwnd(sk); /* note cwnd so we can restore it */
>+ bbr->probe_rtt_done_stamp = 0;
>+ }
>+
>+ if (bbr->mode == BBR_PROBE_RTT) {
>+ /* Ignore low rate samples during this mode. */
>+ tp->app_limited =
>+ (tp->delivered + tcp_packets_in_flight(tp)) ? : 1;
>+ /* Maintain min packets in flight for max(200 ms, 1 round). */
>+ if (!bbr->probe_rtt_done_stamp &&
>+ tcp_packets_in_flight(tp) <= bbr_cwnd_min_target) {
>+ bbr->probe_rtt_done_stamp = tcp_time_stamp +
>+ msecs_to_jiffies(bbr_probe_rtt_mode_ms);
>+ bbr->probe_rtt_round_done = 0;
>+ bbr->next_rtt_delivered = tp->delivered;
>+ } else if (bbr->probe_rtt_done_stamp) {
>+ if (bbr->round_start)
>+ bbr->probe_rtt_round_done = 1;
>+ if (bbr->probe_rtt_round_done &&
>+ after(tcp_time_stamp, bbr->probe_rtt_done_stamp)) {
>+ bbr->min_rtt_stamp = tcp_time_stamp;
>+ bbr->restore_cwnd = 1; /* snap to prior_cwnd */
>+ bbr_reset_mode(sk);
>+ }
>+ }
>+ }
>+ bbr->idle_restart = 0;
>+}
>+
>+static void bbr_update_model(struct sock *sk, const struct rate_sample
>*rs)
>+{
>+ bbr_update_bw(sk, rs);
>+ bbr_update_cycle_phase(sk, rs);
>+ bbr_check_full_bw_reached(sk, rs);
>+ bbr_check_drain(sk, rs);
>+ bbr_update_min_rtt(sk, rs);
>+}
>+
>+static void bbr_main(struct sock *sk, const struct rate_sample *rs)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ u32 bw;
>+
>+ bbr_update_model(sk, rs);
>+
>+ bw = bbr_bw(sk);
>+ bbr_set_pacing_rate(sk, bw, bbr->pacing_gain);
>+ bbr_set_tso_segs_goal(sk);
>+ bbr_set_cwnd(sk, rs, rs->acked_sacked, bw, bbr->cwnd_gain);
>+}
>+
>+static void bbr_init(struct sock *sk)
>+{
>+ struct tcp_sock *tp = tcp_sk(sk);
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ u64 bw;
>+
>+ bbr->prior_cwnd = 0;
>+ bbr->tso_segs_goal = 0; /* default segs per skb until first ACK */
>+ bbr->rtt_cnt = 0;
>+ bbr->next_rtt_delivered = 0;
>+ bbr->prev_ca_state = TCP_CA_Open;
>+ bbr->packet_conservation = 0;
>+
>+ bbr->probe_rtt_done_stamp = 0;
>+ bbr->probe_rtt_round_done = 0;
>+ bbr->min_rtt_us = tcp_min_rtt(tp);
>+ bbr->min_rtt_stamp = tcp_time_stamp;
>+
>+ minmax_reset(&bbr->bw, bbr->rtt_cnt, 0); /* init max bw to 0 */
>+
>+ /* Initialize pacing rate to: high_gain * init_cwnd / RTT. */
>+ bw = (u64)tp->snd_cwnd * BW_UNIT;
>+ do_div(bw, (tp->srtt_us >> 3) ? : USEC_PER_MSEC);
>+ sk->sk_pacing_rate = 0; /* force an update of sk_pacing_rate */
>+ bbr_set_pacing_rate(sk, bw, bbr_high_gain);
>+
>+ bbr->restore_cwnd = 0;
>+ bbr->round_start = 0;
>+ bbr->idle_restart = 0;
>+ bbr->full_bw = 0;
>+ bbr->full_bw_cnt = 0;
>+ bbr->cycle_mstamp.v64 = 0;
>+ bbr->cycle_idx = 0;
>+ bbr_reset_lt_bw_sampling(sk);
>+ bbr_reset_startup_mode(sk);
>+}
>+
>+static u32 bbr_sndbuf_expand(struct sock *sk)
>+{
>+ /* Provision 3 * cwnd since BBR may slow-start even during recovery. */
>+ return 3;
>+}
>+
>+/* In theory BBR does not need to undo the cwnd since it does not
>+ * always reduce cwnd on losses (see bbr_main()). Keep it for now.
>+ */
>+static u32 bbr_undo_cwnd(struct sock *sk)
>+{
>+ return tcp_sk(sk)->snd_cwnd;
>+}
>+
>+/* Entering loss recovery, so save cwnd for when we exit or undo
>recovery. */
>+static u32 bbr_ssthresh(struct sock *sk)
>+{
>+ bbr_save_cwnd(sk);
>+ return TCP_INFINITE_SSTHRESH; /* BBR does not use ssthresh */
>+}
>+
>+static size_t bbr_get_info(struct sock *sk, u32 ext, int *attr,
>+ union tcp_cc_info *info)
>+{
>+ if (ext & (1 << (INET_DIAG_BBRINFO - 1)) ||
>+ ext & (1 << (INET_DIAG_VEGASINFO - 1))) {
>+ struct tcp_sock *tp = tcp_sk(sk);
>+ struct bbr *bbr = inet_csk_ca(sk);
>+ u64 bw = bbr_bw(sk);
>+
>+ bw = bw * tp->mss_cache * USEC_PER_SEC >> BW_SCALE;
>+ memset(&info->bbr, 0, sizeof(info->bbr));
>+ info->bbr.bbr_bw_lo = (u32)bw;
>+ info->bbr.bbr_bw_hi = (u32)(bw >> 32);
>+ info->bbr.bbr_min_rtt = bbr->min_rtt_us;
>+ info->bbr.bbr_pacing_gain = bbr->pacing_gain;
>+ info->bbr.bbr_cwnd_gain = bbr->cwnd_gain;
>+ *attr = INET_DIAG_BBRINFO;
>+ return sizeof(info->bbr);
>+ }
>+ return 0;
>+}
>+
>+static void bbr_set_state(struct sock *sk, u8 new_state)
>+{
>+ struct bbr *bbr = inet_csk_ca(sk);
>+
>+ if (new_state == TCP_CA_Loss) {
>+ struct rate_sample rs = { .losses = 1 };
>+
>+ bbr->prev_ca_state = TCP_CA_Loss;
>+ bbr->full_bw = 0;
>+ bbr->round_start = 1; /* treat RTO like end of a round */
>+ bbr_lt_bw_sampling(sk, &rs);
>+ }
>+}
>+
>+static struct tcp_congestion_ops tcp_bbr_cong_ops __read_mostly = {
>+ .flags = TCP_CONG_NON_RESTRICTED,
>+ .name = "bbr",
>+ .owner = THIS_MODULE,
>+ .init = bbr_init,
>+ .cong_control = bbr_main,
>+ .sndbuf_expand = bbr_sndbuf_expand,
>+ .undo_cwnd = bbr_undo_cwnd,
>+ .cwnd_event = bbr_cwnd_event,
>+ .ssthresh = bbr_ssthresh,
>+ .tso_segs_goal = bbr_tso_segs_goal,
>+ .get_info = bbr_get_info,
>+ .set_state = bbr_set_state,
>+};
>+
>+static int __init bbr_register(void)
>+{
>+ BUILD_BUG_ON(sizeof(struct bbr) > ICSK_CA_PRIV_SIZE);
>+ return tcp_register_congestion_control(&tcp_bbr_cong_ops);
>+}
>+
>+static void __exit bbr_unregister(void)
>+{
>+ tcp_unregister_congestion_control(&tcp_bbr_cong_ops);
>+}
>+
>+module_init(bbr_register);
>+module_exit(bbr_unregister);
>+
>+MODULE_AUTHOR("Van Jacobson <vanj@google.com>");
>+MODULE_AUTHOR("Neal Cardwell <ncardwell@google.com>");
>+MODULE_AUTHOR("Yuchung Cheng <ycheng@google.com>");
>+MODULE_AUTHOR("Soheil Hassas Yeganeh <soheil@google.com>");
>+MODULE_LICENSE("Dual BSD/GPL");
>+MODULE_DESCRIPTION("TCP BBR (Bottleneck Bandwidth and RTT)");
>--
>2.8.0.rc3.226.g39d4020
>
^ permalink raw reply
* Re: [PATCH net v2] net: pktgen: fix pkt_size
From: Greg @ 2016-09-30 15:39 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, David S. Miller, Bogdan Hamciuc, Ben Greear,
Sergei Shtylyov
In-Reply-To: <5277a40b08c491bd4ff8f6e3276718848f7406e8.1475246890.git.pabeni@redhat.com>
On Fri, 2016-09-30 at 16:56 +0200, Paolo Abeni wrote:
> The commit 879c7220e828 ("net: pktgen: Observe needed_headroom
> of the device") increased the 'pkt_overhead' field value by
> LL_RESERVED_SPACE.
> As a side effect the generated packet size, computed as:
>
> /* Eth + IPh + UDPh + mpls */
> datalen = pkt_dev->cur_pkt_size - 14 - 20 - 8 -
> pkt_dev->pkt_overhead;
>
> is decreased by the same value.
> The above changed slightly the behavior of existing pktgen users,
> and made the procfs interface somewhat inconsistent.
> Fix it by restoring the previous pkt_overhead value and using
> LL_RESERVED_SPACE as extralen in skb allocation.
> Also, change pktgen_alloc_skb() to only partially reserve
> the headroom to allow the caller to prefetch from ll header
> start.
>
> v1 -> v2:
> - fixed some typos in the comments
>
> Fixes: 879c7220e828 ("net: pktgen: Observe needed_headroom of the device")
> Suggested-by: Ben Greear <greearb@candelatech.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/core/pktgen.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index bbd118b..5219a9e 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -2286,7 +2286,7 @@ out:
>
> static inline void set_pkt_overhead(struct pktgen_dev *pkt_dev)
> {
> - pkt_dev->pkt_overhead = LL_RESERVED_SPACE(pkt_dev->odev);
> + pkt_dev->pkt_overhead = 0;
> pkt_dev->pkt_overhead += pkt_dev->nr_labels*sizeof(u32);
> pkt_dev->pkt_overhead += VLAN_TAG_SIZE(pkt_dev);
> pkt_dev->pkt_overhead += SVLAN_TAG_SIZE(pkt_dev);
> @@ -2777,13 +2777,13 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
> }
>
> static struct sk_buff *pktgen_alloc_skb(struct net_device *dev,
> - struct pktgen_dev *pkt_dev,
> - unsigned int extralen)
> + struct pktgen_dev *pkt_dev)
> {
> + unsigned int extralen = LL_RESERVED_SPACE(dev);
> struct sk_buff *skb = NULL;
> - unsigned int size = pkt_dev->cur_pkt_size + 64 + extralen +
> - pkt_dev->pkt_overhead;
> + unsigned int size;
>
> + size = pkt_dev->cur_pkt_size + 64 + extralen + pkt_dev->pkt_overhead;
> if (pkt_dev->flags & F_NODE) {
> int node = pkt_dev->node >= 0 ? pkt_dev->node : numa_node_id();
>
> @@ -2796,8 +2796,9 @@ static struct sk_buff *pktgen_alloc_skb(struct net_device *dev,
> skb = __netdev_alloc_skb(dev, size, GFP_NOWAIT);
> }
>
> + /* the caller pre-fetches from skb->data and reserves for the mac hdr */
> if (likely(skb))
> - skb_reserve(skb, LL_RESERVED_SPACE(dev));
> + skb_reserve(skb, extralen - 16);
Is the 16 here the same as HD_DATA_MOD?
Magic numbers...
Thanks,
- Greg
>
> return skb;
> }
> @@ -2830,16 +2831,14 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
> mod_cur_headers(pkt_dev);
> queue_map = pkt_dev->cur_queue_map;
>
> - datalen = (odev->hard_header_len + 16) & ~0xf;
> -
> - skb = pktgen_alloc_skb(odev, pkt_dev, datalen);
> + skb = pktgen_alloc_skb(odev, pkt_dev);
> if (!skb) {
> sprintf(pkt_dev->result, "No memory");
> return NULL;
> }
>
> prefetchw(skb->data);
> - skb_reserve(skb, datalen);
> + skb_reserve(skb, 16);
>
> /* Reserve for ethernet and IP header */
> eth = (__u8 *) skb_push(skb, 14);
> @@ -2959,7 +2958,7 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev,
> mod_cur_headers(pkt_dev);
> queue_map = pkt_dev->cur_queue_map;
>
> - skb = pktgen_alloc_skb(odev, pkt_dev, 16);
> + skb = pktgen_alloc_skb(odev, pkt_dev);
> if (!skb) {
> sprintf(pkt_dev->result, "No memory");
> return NULL;
^ permalink raw reply
* [PATCH net v2] net: pktgen: fix pkt_size
From: Paolo Abeni @ 2016-09-30 14:56 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Bogdan Hamciuc, Ben Greear, Sergei Shtylyov
The commit 879c7220e828 ("net: pktgen: Observe needed_headroom
of the device") increased the 'pkt_overhead' field value by
LL_RESERVED_SPACE.
As a side effect the generated packet size, computed as:
/* Eth + IPh + UDPh + mpls */
datalen = pkt_dev->cur_pkt_size - 14 - 20 - 8 -
pkt_dev->pkt_overhead;
is decreased by the same value.
The above changed slightly the behavior of existing pktgen users,
and made the procfs interface somewhat inconsistent.
Fix it by restoring the previous pkt_overhead value and using
LL_RESERVED_SPACE as extralen in skb allocation.
Also, change pktgen_alloc_skb() to only partially reserve
the headroom to allow the caller to prefetch from ll header
start.
v1 -> v2:
- fixed some typos in the comments
Fixes: 879c7220e828 ("net: pktgen: Observe needed_headroom of the device")
Suggested-by: Ben Greear <greearb@candelatech.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/core/pktgen.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index bbd118b..5219a9e 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2286,7 +2286,7 @@ out:
static inline void set_pkt_overhead(struct pktgen_dev *pkt_dev)
{
- pkt_dev->pkt_overhead = LL_RESERVED_SPACE(pkt_dev->odev);
+ pkt_dev->pkt_overhead = 0;
pkt_dev->pkt_overhead += pkt_dev->nr_labels*sizeof(u32);
pkt_dev->pkt_overhead += VLAN_TAG_SIZE(pkt_dev);
pkt_dev->pkt_overhead += SVLAN_TAG_SIZE(pkt_dev);
@@ -2777,13 +2777,13 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
}
static struct sk_buff *pktgen_alloc_skb(struct net_device *dev,
- struct pktgen_dev *pkt_dev,
- unsigned int extralen)
+ struct pktgen_dev *pkt_dev)
{
+ unsigned int extralen = LL_RESERVED_SPACE(dev);
struct sk_buff *skb = NULL;
- unsigned int size = pkt_dev->cur_pkt_size + 64 + extralen +
- pkt_dev->pkt_overhead;
+ unsigned int size;
+ size = pkt_dev->cur_pkt_size + 64 + extralen + pkt_dev->pkt_overhead;
if (pkt_dev->flags & F_NODE) {
int node = pkt_dev->node >= 0 ? pkt_dev->node : numa_node_id();
@@ -2796,8 +2796,9 @@ static struct sk_buff *pktgen_alloc_skb(struct net_device *dev,
skb = __netdev_alloc_skb(dev, size, GFP_NOWAIT);
}
+ /* the caller pre-fetches from skb->data and reserves for the mac hdr */
if (likely(skb))
- skb_reserve(skb, LL_RESERVED_SPACE(dev));
+ skb_reserve(skb, extralen - 16);
return skb;
}
@@ -2830,16 +2831,14 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
mod_cur_headers(pkt_dev);
queue_map = pkt_dev->cur_queue_map;
- datalen = (odev->hard_header_len + 16) & ~0xf;
-
- skb = pktgen_alloc_skb(odev, pkt_dev, datalen);
+ skb = pktgen_alloc_skb(odev, pkt_dev);
if (!skb) {
sprintf(pkt_dev->result, "No memory");
return NULL;
}
prefetchw(skb->data);
- skb_reserve(skb, datalen);
+ skb_reserve(skb, 16);
/* Reserve for ethernet and IP header */
eth = (__u8 *) skb_push(skb, 14);
@@ -2959,7 +2958,7 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev,
mod_cur_headers(pkt_dev);
queue_map = pkt_dev->cur_queue_map;
- skb = pktgen_alloc_skb(odev, pkt_dev, 16);
+ skb = pktgen_alloc_skb(odev, pkt_dev);
if (!skb) {
sprintf(pkt_dev->result, "No memory");
return NULL;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 3/3] mac80211: Set lower memory limit for non-VHT devices
From: Toke Høiland-Jørgensen @ 2016-09-30 14:35 UTC (permalink / raw)
To: Johannes Berg; +Cc: make-wifi-fast, linux-wireless, netdev
In-Reply-To: <1475239915.17481.63.camel@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
>> > I kinda see the logic here - we really don't need to queue as much
>> > if we can't possibly transmit it out quickly - but it seems to me
>> > we should also throw in some kind of limit that's relative to the
>> > amount of memory you have on the system?
>>
>> Yes, ideally. That goes for FQ-CoDel as well, BTW. LEDE currently
>> carries a patch for that which just changes the hard-coded default to
>> another hard-coded default. Not sure how to get a good value to use,
>> though; and deciding on how large a fraction of memory to use for
>> packets starts smelling an awful lot like setting policy in the
>> kernel, doesn't it?
>
> Yeah, I agree it does seem awkward.
>
> Perhaps we should instead pick a low limit and let users change it
> more easily (i.e. not debugfs)? I don't know a good answer to this
> either.
Hmm, I'll talk it over with some of the LEDE people who are more used to
dealing with these sorts of memory-constrained devices than I am. Will
send a patch if we come up with a good solution :)
-Toke
^ permalink raw reply
* Re: [PATCH 2/2] net: hns: add missing function declaration
From: Arnd Bergmann @ 2016-09-30 14:32 UTC (permalink / raw)
To: Baoyou Xie
Cc: yisen.zhuang, salil.mehta, davem, huangdaode, lisheng011,
xieqianqian, fabf, oulijun, vinod.koul, lipeng321, andrew,
tremyfr, chenny.xu, netdev, linux-kernel, xie.baoyou, han.fei,
tang.qiang007
In-Reply-To: <1475221315-18602-1-git-send-email-baoyou.xie@linaro.org>
On Friday 30 September 2016, Baoyou Xie wrote:
> We get 1 warning when building kernel with W=1:
> drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:2784:5: warning: no previous prototype for 'hns_dsaf_roce_reset' [-Wmissing-prototypes]
>
> In fact, this function is not declared in any file, but should be
> declared in a header file. thus can be recognized in other file.
>
> so this patch adds the missing function declaration into
> drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
If you get to a case like this, please describe in the changelog how you determined that
the function is there intentionally, rather than something that should be removed?
I also see that you had sent the patch series for hns previously, and had included
a 'v2' version in the subject, but left out the version this time. Please always
use increasing version numbers when you send a new version of the series.
Arnd
^ permalink raw reply
* Re: [PATCH 1/2] qed: mark symbols static where possible
From: Arnd Bergmann @ 2016-09-30 14:29 UTC (permalink / raw)
To: Baoyou Xie
Cc: Yuval.Mintz, Ariel.Elior, everest-linux-l2, netdev, linux-kernel,
xie.baoyou, han.fei, tang.qiang007
In-Reply-To: <1475222189-19092-1-git-send-email-baoyou.xie@linaro.org>
On Friday 30 September 2016, Baoyou Xie wrote:
>
> We get 12 warnings when building kernel with W=1:
> drivers/net/ethernet/qlogic/qed/qed_cxt.c:346:6: warning: no previous prototype for 'qed_cxt_set_srq_count' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_cxt.c:353:5: warning: no previous prototype for 'qed_cxt_get_srq_count' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_cxt.c:389:5: warning: no previous prototype for 'qed_cxt_get_proto_cid_start' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_cxt.c:395:5: warning: no previous prototype for 'qed_cxt_get_proto_tid_count' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_cxt.c:1801:6: warning: no previous prototype for 'qed_rdma_set_pf_params' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_debug.c:4031:17: warning: no previous prototype for 'qed_mcp_trace_dump' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_debug.c:4133:17: warning: no previous prototype for 'qed_reg_fifo_dump' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_debug.c:4195:17: warning: no previous prototype for 'qed_igu_fifo_dump' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_debug.c:4258:17: warning: no previous prototype for 'qed_protection_override_dump' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_debug.c:6342:17: warning: no previous prototype for 'qed_print_idle_chk_results_wrapper' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_debug.c:6416:17: warning: no previous prototype for 'format_feature' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_debug.c:6483:17: warning: no previous prototype for 'qed_dbg_dump' [-Wmissing-prototypes]
>
> In fact, these functions are only used in the file in which they are
> declared and don't need a declaration, but can be made static.
> So this patch marks these functions with 'static'.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply
* Re: [PATCH 2/2] qed: remove unused function in qed_cxt.c
From: Arnd Bergmann @ 2016-09-30 14:29 UTC (permalink / raw)
To: Baoyou Xie
Cc: Yuval.Mintz, Ariel.Elior, everest-linux-l2, netdev, linux-kernel,
xie.baoyou, han.fei, tang.qiang007
In-Reply-To: <1475222574-19280-1-git-send-email-baoyou.xie@linaro.org>
On Friday 30 September 2016, Baoyou Xie wrote:
> We get 3 warnings when building kernel with W=1:
> drivers/net/ethernet/qlogic/qed/qed_cxt.c:1941:1: warning: no previous prototype for 'qed_cxt_dynamic_ilt_alloc' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_cxt.c:2158:5: warning: no previous prototype for 'qed_cxt_free_proto_ilt' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_cxt.c:2186:5: warning: no previous prototype for 'qed_cxt_get_task_ctx' [-Wmissing-prototypes]
>
> In fact, these functions are unused in
> drivers/net/ethernet/qlogic/qed/qed_cxt.c, but should be removed.
>
> So this patch removes these unused functions.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
These were only recently added in dbb799c39717 ("qed: Initialize hardware for new protocols"),
so it's likely that the plan is to use them in the future, and your commit message should
at least mention that.
If there is no longer a plan to use them, it would probably better to back out that whole
patch, other alternatives in this case might be to mark them as "static __maybe_unused"
so the compiler can drop the code silently, or to add declarations in a header if the
user would be in another file.
Arnd
^ permalink raw reply
* Re: [PATCH 3/3] net: fec: align IP header in hardware
From: Eric Nelson @ 2016-09-30 14:16 UTC (permalink / raw)
To: David Laight, netdev@vger.kernel.org
Cc: linux@arm.linux.org.uk, andrew@lunn.ch, fugang.duan@nxp.com,
otavio@ossystems.com.br, edumazet@google.com,
troy.kisky@boundarydevices.com, davem@davemloft.net,
u.kleine-koenig@pengutronix.de
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB010FE38@AcuExch.aculab.com>
Hi David,
On 09/30/2016 06:49 AM, David Laight wrote:
> From: Eric Nelson
>> Sent: 30 September 2016 14:27
>> Thanks for the feedback David,
>>
>> On 09/29/2016 04:07 AM, David Laight wrote:
>>> From: Eric Nelson
>>>> Sent: 28 September 2016 18:15
>>>> On 09/28/2016 09:42 AM, David Laight wrote:
>>>>> From: Eric Nelson
>>>>>> Sent: 26 September 2016 19:40
>>>>>> Hi David,
>>>>>>
>>>>>> On 09/26/2016 02:26 AM, David Laight wrote:
>>>>>>> From: Eric Nelson
>>>>>>>> Sent: 24 September 2016 15:42
>>>>>>>> The FEC receive accelerator (RACC) supports shifting the data payload of
>>>>>>>> received packets by 16-bits, which aligns the payload (IP header) on a
>>>>>>>> 4-byte boundary, which is, if not required, at least strongly suggested
>>>>>>>> by the Linux networking layer.
>>>>>>> ...
>>>>>>>> + /* align IP header */
>>>>>>>> + val |= FEC_RACC_SHIFT16;
>>>>>>>
>>>>>>> I can't help feeling that there needs to be corresponding
>>>>>>> changes to increase the buffer size by 2 (maybe for large mtu)
>>>>>>> and to discard two bytes from the frame length.
>>>>>>>
>>>>>>
>>>>>> In the normal case, the fec driver over-allocates all receive packets to
>>>>>> be of size FEC_ENET_RX_FRSIZE (2048) minus the value of rx_align,
>>>>>> which is either 0x0f (ARM) or 0x03 (PPC).
>>>>>>
>>>>>> If the frame length is less than rx_copybreak (typically 256), then
>>>>>> the frame length from the receive buffer descriptor is used to
>>>>>> control the allocation size for a copied buffer, and this will include
>>>>>> the two bytes of padding if RACC_SHIFT16 is set.
>>>>>>
>>>>>>> If probably ought to be predicated on NET_IP_ALIGN as well.
>>>>>>>
>>>>>> Can you elaborate?
>>>>>
>>>>> From reading this it seems that the effect of FEC_RACC_SHIFT16 is to
>>>>> add two bytes of 'junk' to the start of every receive frame.
>>>>>
>>>>
>>>> That's right. Two bytes of junk between the MAC header and the
>>>> IP header.
>>>>
>>>>> In the 'copybreak' case the new skb would need to be 2 bytes shorter
>>>>> than the length reported by the hardware, and the data copied from
>>>>> 2 bytes into the dma buffer.
>>>>>
>>>>
>>>> As it stands, the skb allocated by the copybreak routine will include
>>>> the two bytes of padding, and the call to skb_pull_inline will ignore
>>>> them.
>>>
>>> Ok, I didn't see that call being added by this patch.
>>>
>>>>> The extra 2 bytes also mean the that maximum mtu that can be received
>>>>> into a buffer is two bytes less.
>>>>>
>>>>
>>>> Right, but I think the max is already high enough that this isn't a
>>>> problem.
>>>>
>>>>> If someone sets the mtu to (say) 9k for jumbo frames this might matter.
>>>>> Even with fixed 2048 byte buffers it reduces the maximum value the mtu
>>>>> can be set to by 2.
>>>>>
>>>>
>>>> As far as I can tell, the fec driver doesn't support jumbo frames, and
>>>> the max frame length is currently hard-coded at PKT_MAXBUF_SIZE (1522).
>>>>
>>>> This is well within the 2048-byte allocation, even with optional headers
>>>> for VLAN etc.
>>>
>>> Hmm...
>>>
>>> That (probably) means all the skb the driver allocates are actually 4k.
>>> It would be much better to reduce the size so that the entire skb
>>> (with packet buffer) is less than 2k.
>>>
>>
>> That seems worthwhile, but un-related to this patch.
>
> Indeed.
>
>> It appears to me that the received packets could be allocated as
>>
>> PKT_MAXBUF_SIZE+fep->rx_align+NET_IP_ALIGN
>>
>> (+2 if FEC_RACC_SHIFT16 is used)
>
> No.
> The packet buffers need to be allocated NET_IP_ALIGN + PKT_MAXBUF_SIZE
> byte long and (I assume) aligned on a fep->rx_align byte boundary.
>
I think we're saying the same thing here, with the exception of the
+2 for FEC_RACC_SHIFT16.
> If NET_IP_ALIGN is set (to 2) then FEC_RACC_SHIFT16 must also me set
> so that the ethernet frame itself is 4n+2 aligned.
>
This patch does this, but not with the beginning of the skb.
It also does this when NET_IP_ALIGN is zero though, and I believe this
is the right thing, so the IP header is aligned in a sensible way.
The driver can't handle a DMA to (4n+2) on any architecture.
>>>>> Now if NET_IP_ALIGN is zero then it is fine for the rx frame to start
>>>>> on a 4n boundary, and the skb are likely to be allocated that way.
>>>>> In this case you don't want to extra two bytes of 'junk'.
>>>>>
>>>> NET_IP_ALIGN is defaulting to 2 by the conditional in skbuff.h
>>>
>>> Even though it is always currently set is isn't really ideal to have
>>> a driver that breaks if it isn't set.
>>> This could easily happen at some point in the future if the ethernet
>>> logic is put with a different cpu.
>>>
>>
>> After multiple reads, I'm confused about the meaning of NET_IP_ALIGN
>> and how it should be used.
>>
>> From Documentation/unaligned-memory-access.txt, I take it that this
>> should be configured on a per-architecture basis, and it seems to be
>> set to zero on both PPC and x86.
>>
>> I wonder if this is proper though. It seems that its' use might depend
>> on the I/O subsystem(s) in use as much as the architecture.
> ...
>
> If the cpu cannot do misaligned memory cycles then NET_IP_ALIGN must be 2
> and all receive frames must be aligned like that.
>
On ARM, the CPU can't handle misaligned memory cycles without
taking an alignment fault and NET_IP_ALIGN is set to 2.
On PPC, NET_IP_ALIGN is set to zero.
I could use some help from NXP about whether the driver is used on
PPC, but I don't think it can DMA to 4n+2 addresses on any architecture
and the purpose of this patch is to align the frame on a (4n+2)
address.
> If the cpu can do misaligned memory cycles then the alignment of receive
> ethernet frames doesn't matter that much.
> NET_IP_ALIGN is likely to be set to zero because the cost of the cpu
> doing misaligned transfers it likely to be a lot less than that of
> un-optimised dma accesses to misaligned memory [1] [2].
>
On ARM, we have CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y
but I find it hard to believe that taking alignment faults is more
efficient than adding two bytes to the start of the frame.
> If NET_IP_ALIGN is zero then I believe that ethernet drivers are
> allowed to build skb that have the frame on a 4n+2 alignment.
> This is probably sensible if the hardware can write the two bytes.
> (DM might correct me there.)
>
Again, I don't think the FEC can do this, even if PPC does allow
DMA to 4n+2 addresses for other functions.
> David
>
> [1] The original sparc sbus 'DMA' part did multiple 16bit transfers instead
> of a burst of 32bit transfers. This meant the buffer had to be misaligned
> and a software copy done to align the frames. Fixed in the DMA+ part.
>
> [2] PCIe writes are likely to be much faster if they contain entire cache
> lines of data.
>
^ permalink raw reply
* RE: [PATCH 3/3] net: fec: align IP header in hardware
From: David Laight @ 2016-09-30 13:49 UTC (permalink / raw)
To: 'Eric Nelson', netdev@vger.kernel.org
Cc: linux@arm.linux.org.uk, andrew@lunn.ch, fugang.duan@nxp.com,
otavio@ossystems.com.br, edumazet@google.com,
troy.kisky@boundarydevices.com, davem@davemloft.net,
u.kleine-koenig@pengutronix.de
In-Reply-To: <af74a33e-a164-bd2f-4c63-24a128d1ffb5@nelint.com>
From: Eric Nelson
> Sent: 30 September 2016 14:27
> Thanks for the feedback David,
>
> On 09/29/2016 04:07 AM, David Laight wrote:
> > From: Eric Nelson
> >> Sent: 28 September 2016 18:15
> >> On 09/28/2016 09:42 AM, David Laight wrote:
> >>> From: Eric Nelson
> >>>> Sent: 26 September 2016 19:40
> >>>> Hi David,
> >>>>
> >>>> On 09/26/2016 02:26 AM, David Laight wrote:
> >>>>> From: Eric Nelson
> >>>>>> Sent: 24 September 2016 15:42
> >>>>>> The FEC receive accelerator (RACC) supports shifting the data payload of
> >>>>>> received packets by 16-bits, which aligns the payload (IP header) on a
> >>>>>> 4-byte boundary, which is, if not required, at least strongly suggested
> >>>>>> by the Linux networking layer.
> >>>>> ...
> >>>>>> + /* align IP header */
> >>>>>> + val |= FEC_RACC_SHIFT16;
> >>>>>
> >>>>> I can't help feeling that there needs to be corresponding
> >>>>> changes to increase the buffer size by 2 (maybe for large mtu)
> >>>>> and to discard two bytes from the frame length.
> >>>>>
> >>>>
> >>>> In the normal case, the fec driver over-allocates all receive packets to
> >>>> be of size FEC_ENET_RX_FRSIZE (2048) minus the value of rx_align,
> >>>> which is either 0x0f (ARM) or 0x03 (PPC).
> >>>>
> >>>> If the frame length is less than rx_copybreak (typically 256), then
> >>>> the frame length from the receive buffer descriptor is used to
> >>>> control the allocation size for a copied buffer, and this will include
> >>>> the two bytes of padding if RACC_SHIFT16 is set.
> >>>>
> >>>>> If probably ought to be predicated on NET_IP_ALIGN as well.
> >>>>>
> >>>> Can you elaborate?
> >>>
> >>> From reading this it seems that the effect of FEC_RACC_SHIFT16 is to
> >>> add two bytes of 'junk' to the start of every receive frame.
> >>>
> >>
> >> That's right. Two bytes of junk between the MAC header and the
> >> IP header.
> >>
> >>> In the 'copybreak' case the new skb would need to be 2 bytes shorter
> >>> than the length reported by the hardware, and the data copied from
> >>> 2 bytes into the dma buffer.
> >>>
> >>
> >> As it stands, the skb allocated by the copybreak routine will include
> >> the two bytes of padding, and the call to skb_pull_inline will ignore
> >> them.
> >
> > Ok, I didn't see that call being added by this patch.
> >
> >>> The extra 2 bytes also mean the that maximum mtu that can be received
> >>> into a buffer is two bytes less.
> >>>
> >>
> >> Right, but I think the max is already high enough that this isn't a
> >> problem.
> >>
> >>> If someone sets the mtu to (say) 9k for jumbo frames this might matter.
> >>> Even with fixed 2048 byte buffers it reduces the maximum value the mtu
> >>> can be set to by 2.
> >>>
> >>
> >> As far as I can tell, the fec driver doesn't support jumbo frames, and
> >> the max frame length is currently hard-coded at PKT_MAXBUF_SIZE (1522).
> >>
> >> This is well within the 2048-byte allocation, even with optional headers
> >> for VLAN etc.
> >
> > Hmm...
> >
> > That (probably) means all the skb the driver allocates are actually 4k.
> > It would be much better to reduce the size so that the entire skb
> > (with packet buffer) is less than 2k.
> >
>
> That seems worthwhile, but un-related to this patch.
Indeed.
> It appears to me that the received packets could be allocated as
>
> PKT_MAXBUF_SIZE+fep->rx_align+NET_IP_ALIGN
>
> (+2 if FEC_RACC_SHIFT16 is used)
No.
The packet buffers need to be allocated NET_IP_ALIGN + PKT_MAXBUF_SIZE
byte long and (I assume) aligned on a fep->rx_align byte boundary.
If NET_IP_ALIGN is set (to 2) then FEC_RACC_SHIFT16 must also me set
so that the ethernet frame itself is 4n+2 aligned.
> >>> Now if NET_IP_ALIGN is zero then it is fine for the rx frame to start
> >>> on a 4n boundary, and the skb are likely to be allocated that way.
> >>> In this case you don't want to extra two bytes of 'junk'.
> >>>
> >> NET_IP_ALIGN is defaulting to 2 by the conditional in skbuff.h
> >
> > Even though it is always currently set is isn't really ideal to have
> > a driver that breaks if it isn't set.
> > This could easily happen at some point in the future if the ethernet
> > logic is put with a different cpu.
> >
>
> After multiple reads, I'm confused about the meaning of NET_IP_ALIGN
> and how it should be used.
>
> From Documentation/unaligned-memory-access.txt, I take it that this
> should be configured on a per-architecture basis, and it seems to be
> set to zero on both PPC and x86.
>
> I wonder if this is proper though. It seems that its' use might depend
> on the I/O subsystem(s) in use as much as the architecture.
...
If the cpu cannot do misaligned memory cycles then NET_IP_ALIGN must be 2
and all receive frames must be aligned like that.
If the cpu can do misaligned memory cycles then the alignment of receive
ethernet frames doesn't matter that much.
NET_IP_ALIGN is likely to be set to zero because the cost of the cpu
doing misaligned transfers it likely to be a lot less than that of
un-optimised dma accesses to misaligned memory [1] [2].
If NET_IP_ALIGN is zero then I believe that ethernet drivers are
allowed to build skb that have the frame on a 4n+2 alignment.
This is probably sensible if the hardware can write the two bytes.
(DM might correct me there.)
David
[1] The original sparc sbus 'DMA' part did multiple 16bit transfers instead
of a burst of 32bit transfers. This meant the buffer had to be misaligned
and a software copy done to align the frames. Fixed in the DMA+ part.
[2] PCIe writes are likely to be much faster if they contain entire cache
lines of data.
^ permalink raw reply
* Re: [PATCH V2 for-next 0/8] Bug Fixes and Code Improvement in HNS driver
From: Doug Ledford @ 2016-09-30 13:38 UTC (permalink / raw)
To: David Miller, salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org
Cc: yisen.zhuang-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linuxarm-hv44wF8Li93QT0dZR+AlfA@public.gmane.org
In-Reply-To: <20160930.013341.118711534087556597.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1636 bytes --]
On 9/30/16 1:33 AM, David Miller wrote:
> From: Salil Mehta <salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Date: Thu, 29 Sep 2016 18:09:08 +0100
>
>> This patch-set introduces fix to some Bugs, potential problems
>> and code improvements identified during internal review and
>> testing of Hisilicon Network Subsystem driver.
>>
>> Submit Change
>> V1->V2: This addresses the feedbacks provided by David Miller
>> and Doug Ledford
>
> So Doug my understanding is if this makes it through review
> this is going to be merged into your tree,
Correct. Mainly because it sits on top of some other patches from Huawei.
> you prepare a
> branch for me, and then I pull from that?
I can. It's either that or these 8 patches go to Linus through my tree.
Be forewarned, the branch will have to include the entire hns-roce
driver and the hns/hns-roce ACPI reset support patches. That may be
more than you intended to get in a pull. But it's a kind of messed up
branch anyway. I couldn't submit the branch until after you submit your
pull request to Linus because the branch itself is based on an older
version of your net-next tree, which was needed to get the hns-roce
driver to go in cleanly. I just added the hns-roce stuff on top of that.
My original intent, because of all this, was that this branch would be
submitted as its own special pull request once most other stuff was
already in.
> Thanks in advance.
Sure.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG Key ID: 0E572FDD
Red Hat, Inc.
100 E. Davie St
Raleigh, NC 27601 USA
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 907 bytes --]
^ permalink raw reply
* Re: pull-request: wireless-drivers-next 2016-09-29
From: Aaron Conole @ 2016-09-30 13:30 UTC (permalink / raw)
To: David Miller, Pablo Neira Ayuso
Cc: kvalo, linux-wireless, netdev, linux-kernel
In-Reply-To: <20160930.013245.1474369794552343595.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: Kalle Valo <kvalo@codeaurora.org>
> Date: Thu, 29 Sep 2016 19:57:28 +0300
>
...
>> Or actually I had one problem. While doing a test merge I noticed that
>> net-next fails to compile for me, but I don't think this is anything
>> wireless related:
>>
>> CC net/netfilter/core.o
>> net/netfilter/core.c: In function 'nf_set_hooks_head':
>> net/netfilter/core.c:96:149: error: 'struct net_device' has no
>> member named 'nf_hooks_ingress'
>
> Yes, I am aware of this build issue and will tackle it myself if someone
> doesn't beat me to it.
Sorry, I introduced this. I posted a series targetted at nf-next to
solve this, but it could be merged to net-next instead, if that makes
sense.
The patches are here:
https://patchwork.ozlabs.org/patch/676287/
https://patchwork.ozlabs.org/patch/676288/
Again, sorry for this.
^ permalink raw reply
* Re: [PATCH 3/3] net: fec: align IP header in hardware
From: Eric Nelson @ 2016-09-30 13:27 UTC (permalink / raw)
To: David Laight, netdev@vger.kernel.org
Cc: linux@arm.linux.org.uk, andrew@lunn.ch, fugang.duan@nxp.com,
otavio@ossystems.com.br, edumazet@google.com,
troy.kisky@boundarydevices.com, davem@davemloft.net,
u.kleine-koenig@pengutronix.de
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB010E25C@AcuExch.aculab.com>
Thanks for the feedback David,
On 09/29/2016 04:07 AM, David Laight wrote:
> From: Eric Nelson
>> Sent: 28 September 2016 18:15
>> On 09/28/2016 09:42 AM, David Laight wrote:
>>> From: Eric Nelson
>>>> Sent: 26 September 2016 19:40
>>>> Hi David,
>>>>
>>>> On 09/26/2016 02:26 AM, David Laight wrote:
>>>>> From: Eric Nelson
>>>>>> Sent: 24 September 2016 15:42
>>>>>> The FEC receive accelerator (RACC) supports shifting the data payload of
>>>>>> received packets by 16-bits, which aligns the payload (IP header) on a
>>>>>> 4-byte boundary, which is, if not required, at least strongly suggested
>>>>>> by the Linux networking layer.
>>>>> ...
>>>>>> + /* align IP header */
>>>>>> + val |= FEC_RACC_SHIFT16;
>>>>>
>>>>> I can't help feeling that there needs to be corresponding
>>>>> changes to increase the buffer size by 2 (maybe for large mtu)
>>>>> and to discard two bytes from the frame length.
>>>>>
>>>>
>>>> In the normal case, the fec driver over-allocates all receive packets to
>>>> be of size FEC_ENET_RX_FRSIZE (2048) minus the value of rx_align,
>>>> which is either 0x0f (ARM) or 0x03 (PPC).
>>>>
>>>> If the frame length is less than rx_copybreak (typically 256), then
>>>> the frame length from the receive buffer descriptor is used to
>>>> control the allocation size for a copied buffer, and this will include
>>>> the two bytes of padding if RACC_SHIFT16 is set.
>>>>
>>>>> If probably ought to be predicated on NET_IP_ALIGN as well.
>>>>>
>>>> Can you elaborate?
>>>
>>> From reading this it seems that the effect of FEC_RACC_SHIFT16 is to
>>> add two bytes of 'junk' to the start of every receive frame.
>>>
>>
>> That's right. Two bytes of junk between the MAC header and the
>> IP header.
>>
>>> In the 'copybreak' case the new skb would need to be 2 bytes shorter
>>> than the length reported by the hardware, and the data copied from
>>> 2 bytes into the dma buffer.
>>>
>>
>> As it stands, the skb allocated by the copybreak routine will include
>> the two bytes of padding, and the call to skb_pull_inline will ignore
>> them.
>
> Ok, I didn't see that call being added by this patch.
>
>>> The extra 2 bytes also mean the that maximum mtu that can be received
>>> into a buffer is two bytes less.
>>>
>>
>> Right, but I think the max is already high enough that this isn't a
>> problem.
>>
>>> If someone sets the mtu to (say) 9k for jumbo frames this might matter.
>>> Even with fixed 2048 byte buffers it reduces the maximum value the mtu
>>> can be set to by 2.
>>>
>>
>> As far as I can tell, the fec driver doesn't support jumbo frames, and
>> the max frame length is currently hard-coded at PKT_MAXBUF_SIZE (1522).
>>
>> This is well within the 2048-byte allocation, even with optional headers
>> for VLAN etc.
>
> Hmm...
>
> That (probably) means all the skb the driver allocates are actually 4k.
> It would be much better to reduce the size so that the entire skb
> (with packet buffer) is less than 2k.
>
That seems worthwhile, but un-related to this patch.
It appears to me that the received packets could be allocated as
PKT_MAXBUF_SIZE+fep->rx_align+NET_IP_ALIGN
(+2 if FEC_RACC_SHIFT16 is used)
>>> Now if NET_IP_ALIGN is zero then it is fine for the rx frame to start
>>> on a 4n boundary, and the skb are likely to be allocated that way.
>>> In this case you don't want to extra two bytes of 'junk'.
>>>
>> NET_IP_ALIGN is defaulting to 2 by the conditional in skbuff.h
>
> Even though it is always currently set is isn't really ideal to have
> a driver that breaks if it isn't set.
> This could easily happen at some point in the future if the ethernet
> logic is put with a different cpu.
>
After multiple reads, I'm confused about the meaning of NET_IP_ALIGN
and how it should be used.
>From Documentation/unaligned-memory-access.txt, I take it that this
should be configured on a per-architecture basis, and it seems to be
set to zero on both PPC and x86.
I wonder if this is proper though. It seems that its' use might depend
on the I/O subsystem(s) in use as much as the architecture.
For example, it might be desirable to have a different value for a PCIe
interface than for an integrated MAC like the FEC.
Looking at the example of the 3c59x driver, I see a pattern of an
allocation that adds NET_IP_ALIGN followed by an skb->reserve()
of NET_IP_ALIGN before determining the target address to end
up with allocation with 4n+2 alignment.
This seems somewhat equivalent to this patch, except that we're
using the allocated address as the target and using skb_pull_inline
afterwards.
Andy, is the FEC used on any PPC SOCs?
If so, then this patch may cause a DMA of 2 extra bytes per frame
unecessarily although the driver doesn't special-case the allocation
to align the IP header, so this is still probably preferred.
>>> OTOH if NET_IP_ALIGN is 2 then you need to 'fiddle' things so that
>>> the data is dma'd to offset -2 in the skb and then ensure that the
>>> end of frame is set correctly.
>>>
>>
>> That's what the RACC SHIFT16 bit does.
>
> No, that causes the ethernet controller to add 2 bytes to the frame.
> You then need to change the dma target address to match.
>
Or use skb_pull_inline to ignore the two bytes.
> Otherwise if a new version of the silicon stops ignoring the low
> address with the frame will be misaligned in the buffer.
>
I'm not sure I understand this.
> The receive frame length will also (probably) be 2 larger than the
> actual frame - so you need to set the end point correctly as well.
> IP will probably ignore the 2 bytes of pad I think you are generating.
>
The received frame length **is** 2 bytes longer, but these are
eaten by skb_pull_inline().
>> The FEC hardware isn't capable of DMA'ing to an un-aligned address.
>> On ARM, it requires 64-bit alignment, but suggests 128-bit alignment.
>>
>> On other (PPC?) architectures, it requires 32-bit alignment. This is
>> handled by the rx_align field.
>
> That isn't entirely relevant.
>
> If the kernel is being built with NET_IP_ALIGN set to 0 you should
> align the destination mac address on a 4n boundary
> (Or rather the skb are likely to be allocated that way).
They're not currently allocated that way. The routine
fec_enet_alloc_rxq_buffers
forces the allocations to 32 or 128-bit alignment through the
routine fec_enet_new_rxbdp().
> If it causes misaligned memory reads later on that is a different problem.
That's the problem this patch is designed to address. Without this
patch, the IP header is always mis-aligned.
> The MAC driver has aligned the frames as it was told to.
>
> David
>
>
Regards,
Eric
^ permalink raw reply
* Re: [PATCH 1/3] cw1200: Don't leak memory if krealloc failes
From: Johannes Thumshirn @ 2016-09-30 13:00 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Solomon Peachy, Kalle Valo, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <25a38254-f0e3-1f4e-de46-688d9fd1b736-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
On Fri, Sep 30, 2016 at 03:56:45PM +0300, Sergei Shtylyov wrote:
> Hello.
>
> On 9/30/2016 3:11 PM, Johannes Thumshirn wrote:
>
> > The call to krealloc() in wsm_buf_reserve() directly assigns the newly
> > returned memory to buf->begin. This is all fine except when krealloc()
> > failes we loose the ability to free the old memory pointed to by
>
> Fails.
>
> > buf->begin. If we just create a temporary variable to assign memory to
> > and assign the memory to it we can mitigate the memory leak.
> >
> > Signed-off-by: Johannes Thumshirn <jthumshirn-l3A5Bk7waGM@public.gmane.org>
> > ---
> > drivers/net/wireless/st/cw1200/wsm.c | 16 +++++++++-------
> > 1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/wireless/st/cw1200/wsm.c b/drivers/net/wireless/st/cw1200/wsm.c
> > index 680d60e..12fad99 100644
> > --- a/drivers/net/wireless/st/cw1200/wsm.c
> > +++ b/drivers/net/wireless/st/cw1200/wsm.c
> > @@ -1807,16 +1807,18 @@ static int wsm_buf_reserve(struct wsm_buf *buf, size_t extra_size)
> > {
> > size_t pos = buf->data - buf->begin;
> > size_t size = pos + extra_size;
> > + u8 *tmp;
> >
> > size = round_up(size, FWLOAD_BLOCK_SIZE);
> >
> > - buf->begin = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
> > - if (buf->begin) {
> > - buf->data = &buf->begin[pos];
> > - buf->end = &buf->begin[size];
> > - return 0;
> > - } else {
> > - buf->end = buf->data = buf->begin;
> > + tmp = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
> > + if (tmp) {
>
> !tmp, you mean?
Yes, I've already sent out a v2.
--
Johannes Thumshirn Storage
jthumshirn-l3A5Bk7waGM@public.gmane.org +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply
* Re: [PATCH 1/3] cw1200: Don't leak memory if krealloc failes
From: Sergei Shtylyov @ 2016-09-30 12:56 UTC (permalink / raw)
To: Johannes Thumshirn, Solomon Peachy, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <1475237495-15030-1-git-send-email-jthumshirn@suse.de>
Hello.
On 9/30/2016 3:11 PM, Johannes Thumshirn wrote:
> The call to krealloc() in wsm_buf_reserve() directly assigns the newly
> returned memory to buf->begin. This is all fine except when krealloc()
> failes we loose the ability to free the old memory pointed to by
Fails.
> buf->begin. If we just create a temporary variable to assign memory to
> and assign the memory to it we can mitigate the memory leak.
>
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
> drivers/net/wireless/st/cw1200/wsm.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/st/cw1200/wsm.c b/drivers/net/wireless/st/cw1200/wsm.c
> index 680d60e..12fad99 100644
> --- a/drivers/net/wireless/st/cw1200/wsm.c
> +++ b/drivers/net/wireless/st/cw1200/wsm.c
> @@ -1807,16 +1807,18 @@ static int wsm_buf_reserve(struct wsm_buf *buf, size_t extra_size)
> {
> size_t pos = buf->data - buf->begin;
> size_t size = pos + extra_size;
> + u8 *tmp;
>
> size = round_up(size, FWLOAD_BLOCK_SIZE);
>
> - buf->begin = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
> - if (buf->begin) {
> - buf->data = &buf->begin[pos];
> - buf->end = &buf->begin[size];
> - return 0;
> - } else {
> - buf->end = buf->data = buf->begin;
> + tmp = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
> + if (tmp) {
!tmp, you mean?
> + wsm_buf_deinit(buf);
> return -ENOMEM;
> }
> +
> + buf->begin = tmp;
> + buf->data = &buf->begin[pos];
> + buf->end = &buf->begin[size];
> + return 0;
> }
MBR, Sergei
^ permalink raw reply
* Re: [PATCH 3/3] mac80211: Set lower memory limit for non-VHT devices
From: Johannes Berg @ 2016-09-30 12:51 UTC (permalink / raw)
To: Toke Høiland-Jørgensen; +Cc: make-wifi-fast, linux-wireless, netdev
In-Reply-To: <87twcx5zll.fsf@toke.dk>
> > I kinda see the logic here - we really don't need to queue as much
> > if we can't possibly transmit it out quickly - but it seems to me
> > we should also throw in some kind of limit that's relative to the
> > amount of memory you have on the system?
>
> Yes, ideally. That goes for FQ-CoDel as well, BTW. LEDE currently
> carries a patch for that which just changes the hard-coded default to
> another hard-coded default. Not sure how to get a good value to use,
> though; and deciding on how large a fraction of memory to use for
> packets starts smelling an awful lot like setting policy in the
> kernel, doesn't it?
Yeah, I agree it does seem awkward.
Perhaps we should instead pick a low limit and let users change it more
easily (i.e. not debugfs)? I don't know a good answer to this either.
johannes
^ permalink raw reply
* Re: [PATCH 3/3] mac80211: Set lower memory limit for non-VHT devices
From: Toke Høiland-Jørgensen @ 2016-09-30 12:41 UTC (permalink / raw)
To: Johannes Berg; +Cc: make-wifi-fast, linux-wireless, netdev
In-Reply-To: <1475235230.17481.43.camel@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
> On Fri, 2016-09-23 at 21:59 +0200, Toke Høiland-Jørgensen wrote:
>> Small devices can run out of memory from queueing too many packets.
>> If VHT is not supported by the PHY, having more than 4 MBytes of
>> total queue in the TXQ intermediate queues is not needed, and so we
>> can safely limit the memory usage in these cases and avoid OOM.
>
> I kinda see the logic here - we really don't need to queue as much if
> we can't possibly transmit it out quickly - but it seems to me we
> should also throw in some kind of limit that's relative to the amount
> of memory you have on the system?
Yes, ideally. That goes for FQ-CoDel as well, BTW. LEDE currently
carries a patch for that which just changes the hard-coded default to
another hard-coded default. Not sure how to get a good value to use,
though; and deciding on how large a fraction of memory to use for
packets starts smelling an awful lot like setting policy in the kernel,
doesn't it?
> I've applied these anyway though. I just don't like your assumption (b)
> much as the rationale for it.
Right, thanks. I'll come up with a better rationale next time ;)
-Toke
^ 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