* Re: [PATCH net V2] vhost-vsock: fix use after free
From: Michael S. Tsirkin @ 2018-09-27 23:50 UTC (permalink / raw)
To: Jason Wang
Cc: stefanha, kvm, virtualization, netdev, linux-kernel,
sergei.shtylyov
In-Reply-To: <588ed28b-7e4b-9dc8-92ce-d75692836c9e@redhat.com>
On Fri, Sep 28, 2018 at 07:37:37AM +0800, Jason Wang wrote:
>
>
> On 2018年09月28日 01:04, Michael S. Tsirkin wrote:
> > On Thu, Sep 27, 2018 at 08:22:04PM +0800, Jason Wang wrote:
> > > The access of vsock is not protected by vhost_vsock_lock. This may
> > > lead to use after free since vhost_vsock_dev_release() may free the
> > > pointer at the same time.
> > >
> > > Fix this by holding the lock during the access.
> > >
> > > Reported-by:syzbot+e3e074963495f92a89ed@syzkaller.appspotmail.com
> > > Fixes: 16320f363ae1 ("vhost-vsock: add pkt cancel capability")
> > > Fixes: 433fc58e6bf2 ("VSOCK: Introduce vhost_vsock.ko")
> > > Cc: Stefan Hajnoczi<stefanha@redhat.com>
> > > Signed-off-by: Jason Wang<jasowang@redhat.com>
> > Wow is that really the best we can do?
>
> For net/stable, probably yes.
>
> > A global lock on a data path
> > operation?
>
> It's already there,
&vhost_vsock_lock? were is it takes on data path?
> and the patch only increase the critical section.
>
> > Granted use after free is nasty but Stefan said he sees
> > a way to fix it using a per socket refcount. He's on vacation
> > until Oct 4 though ...
> >
>
> Stefan has acked the pacth, so I think it's ok? We can do optimization for
> -next on top.
>
> Thanks
Well on high SMP serializing can drop performance as much as x100 so I'm
not sure it's appropriate - seems to fix a bug but can introduce a
regression. Let's see how does a proper fix look first?
--
MST
^ permalink raw reply
* RE: [PATCH] net/ncsi: Add NCSI OEM command for FB Tiogapass
From: Justin.Lee1 @ 2018-09-27 17:46 UTC (permalink / raw)
To: joel
Cc: amithash, vijaykhemka, sam, linux-aspeed, openbmc, sdasari,
netdev, christian
In-Reply-To: <CACPK8XfuXa19dGRqL+Ycdgh1LKmoQNuqGvtrrOgeNyvTZR01kQ@mail.gmail.com>
> Thanks for the overview. We look forward to your patches; please
> include the same cc list as this series.
> I think it makes sense to have some OEM NCSI handing purely in the
> kenrel. This would allow eg. the MAC address of an interface to be
> correct at boot, without requiring userspace to come up first.
> I have also heard stories of temperature sensors over NCSI. Those
> would make sense to be hwmon drivers, which again would mean handling
> them in the kernel.
> Justin, Vijay, can you please list the known NCSI OEM
> commands/extensions that we plan on implementing?
In our implementation, All OEM commands are transparent to Kernel.
We don't plan to add any specific handler in the Kernel.
Thanks,
Justin
^ permalink raw reply
* [PATCH net-next 0/7] rtnetlink: add RTM_GETADDR2
From: Christian Brauner @ 2018-09-27 17:58 UTC (permalink / raw)
To: jbenc, davem, dsahern, stephen, netdev, linux-kernel; +Cc: Christian Brauner
Various userspace programs (e.g. iproute2) have sent RTM_GETADDR
requests with struct ifinfomsg. This is wrong and should have been
struct ifaddrmsg all along as mandated by the manpages. However, dump
requests so far didn't parse the netlink message that was sent and
succeeded even when a wrong struct was passed along.
Currently, the message is parsed under the assumption that the correct
struct ifaddrmsg is sent down. If the parsing fails the kernel will
still fulfill the request to preserve backwards compatability but a
rate-limited message that there were leftover bytes after parsing the
message is recorded in dmesg. It has been argued that this is
unacceptable [1].
But various new features that got and will get added to RTM_GETADDR make
it necessary to definitely know what header was passed along.
This is currently not possible without resorting to (likely unreliable)
hacks such as introducing a nested attribute that ensures that
RTM_GETADDR which pass along properties such as IFA_TARGET_NETNSID
always exceed RTM_GETADDR requests that send down the wrong struct
ifinfomsg [2]. Basically, multiple approaches to solve this have been
shut down. Furthermore, the API expressed via RTM_GETADDR is apparently
frozen [3] so the wiggle room at this point seems very much zero.
The correct solution at this point seems to me to introduce a new
RTM_GETADDR2 request. This way we can parse the message and fail hard if
the struct is not struct ifaddrmsg and can safely extend it in the
future. Userspace tools that rely on the buggy RTM_GETADDR API will
still keep working without even having to see any log messages and new
userspace tools that want to make user of new features can make use of
the new RTM_GETADDR2 requests.
[1]: https://lists.openwall.net/netdev/2018/09/25/59
[2]: https://lists.openwall.net/netdev/2018/09/25/75
[3]: https://lists.openwall.net/netdev/2018/09/26/166
Signed-off-by: Christian Brauner <christian@brauner.io>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Benc <jbenc@redhat.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Christian Brauner (7):
rtnetlink: add RTM_GETADDR2
ipv4: add RTM_GETADDR2
ipv6: add RTM_GETADDR2
decnet: add RTM_GETADDR2
phonet: add RTM_GETADDR2
selinux: add RTM_GETADDR2
rtnetlink: enable RTM_GETADDR2
include/uapi/linux/rtnetlink.h | 3 +++
net/core/rtnetlink.c | 1 +
net/decnet/dn_dev.c | 25 +++++++++++++++++++++++--
net/ipv4/devinet.c | 24 +++++++++++++++++++++---
net/ipv6/addrconf.c | 30 ++++++++++++++++++++++++------
net/phonet/pn_netlink.c | 25 +++++++++++++++++++++++--
security/selinux/nlmsgtab.c | 3 ++-
7 files changed, 97 insertions(+), 14 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net-next 1/7] rtnetlink: add RTM_GETADDR2
From: Christian Brauner @ 2018-09-27 17:58 UTC (permalink / raw)
To: jbenc, davem, dsahern, stephen, netdev, linux-kernel; +Cc: Christian Brauner
In-Reply-To: <20180927175857.3511-1-christian@brauner.io>
Various userspace programs (e.g. iproute2) have sent RTM_GETADDR
requests with struct ifinfomsg. This is wrong and should have been
struct ifaddrmsg all along as mandated by the manpages. However, dump
requests so far didn't parse the netlink message that was sent and
succeeded even when a wrong struct was passed along.
Currently, the message is parsed under the assumption that the correct
struct ifaddrmsg is sent down. If the parsing fails the kernel will
still fulfill the request to preserve backwards compatability but a
rate-limited message that there were leftover bytes after parsing the
message is recorded in dmesg. It has been argued that this is
unacceptable [1].
But various new features that got and will get added to RTM_GETADDR make
it necessary to definitely know what header was passed along.
This is currently not possible without resorting to (likely unreliable)
hacks such as introducing a nested attribute that ensures that
RTM_GETADDR which pass along properties such as IFA_TARGET_NETNSID
always exceed RTM_GETADDR requests that send down the wrong struct
ifinfomsg [2]. Basically, multiple approaches to solve this have been
shut down. Furthermore, the API expressed via RTM_GETADDR is apparently
frozen [3] so the wiggle room at this point seems very much zero.
The correct solution at this point seems to me to introduce a new
RTM_GETADDR2 request. This way we can parse the message and fail hard if
the struct is not struct ifaddrmsg and can safely extend it in the
future. Userspace tools that rely on the buggy RTM_GETADDR API will
still keep working without even having to see any log messages and new
userspace tools that want to make user of new features can make use of
the new RTM_GETADDR2 requests.
[1]: https://lists.openwall.net/netdev/2018/09/25/59
[2]: https://lists.openwall.net/netdev/2018/09/25/75
[3]: https://lists.openwall.net/netdev/2018/09/26/166
Signed-off-by: Christian Brauner <christian@brauner.io>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Benc <jbenc@redhat.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
---
include/uapi/linux/rtnetlink.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 46399367627f..e167f90c3d7a 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -157,6 +157,9 @@ enum {
RTM_GETCHAIN,
#define RTM_GETCHAIN RTM_GETCHAIN
+ RTM_GETADDR2 = 106,
+#define RTM_GETADDR2 RTM_GETADDR2
+
__RTM_MAX,
#define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
};
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 3/7] ipv6: add RTM_GETADDR2
From: Christian Brauner @ 2018-09-27 17:58 UTC (permalink / raw)
To: jbenc, davem, dsahern, stephen, netdev, linux-kernel; +Cc: Christian Brauner
In-Reply-To: <20180927175857.3511-1-christian@brauner.io>
Various userspace programs (e.g. iproute2) have sent RTM_GETADDR
requests with struct ifinfomsg. This is wrong and should have been
struct ifaddrmsg all along as mandated by the manpages. However, dump
requests so far didn't parse the netlink message that was sent and
succeeded even when a wrong struct was passed along.
Currently, the message is parsed under the assumption that the correct
struct ifaddrmsg is sent down. If the parsing fails the kernel will
still fulfill the request to preserve backwards compatability but a
rate-limited message that there were leftover bytes after parsing the
message is recorded in dmesg. It has been argued that this is
unacceptable [1].
But various new features that got and will get added to RTM_GETADDR make
it necessary to definitely know what header was passed along.
This is currently not possible without resorting to (likely unreliable)
hacks such as introducing a nested attribute that ensures that
RTM_GETADDR which pass along properties such as IFA_TARGET_NETNSID
always exceed RTM_GETADDR requests that send down the wrong struct
ifinfomsg [2]. Basically, multiple approaches to solve this have been
shut down. Furthermore, the API expressed via RTM_GETADDR is apparently
frozen [3] so the wiggle room at this point seems very much zero.
The correct solution at this point seems to me to introduce a new
RTM_GETADDR2 request. This way we can parse the message and fail hard if
the struct is not struct ifaddrmsg and can safely extend it in the
future. Userspace tools that rely on the buggy RTM_GETADDR API will
still keep working without even having to see any log messages and new
userspace tools that want to make user of new features can make use of
the new RTM_GETADDR2 requests.
[1]: https://lists.openwall.net/netdev/2018/09/25/59
[2]: https://lists.openwall.net/netdev/2018/09/25/75
[3]: https://lists.openwall.net/netdev/2018/09/26/166
Signed-off-by: Christian Brauner <christian@brauner.io>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Benc <jbenc@redhat.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
---
net/ipv6/addrconf.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index a9a317322388..6e76e5cc76c4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5003,7 +5003,7 @@ static int in6_dump_addrs(struct inet6_dev *idev, struct sk_buff *skb,
}
static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
- enum addr_type_t type)
+ enum addr_type_t type, int rtm_version)
{
struct net *net = sock_net(skb->sk);
struct nlattr *tb[IFA_MAX+1];
@@ -5020,8 +5020,14 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
s_idx = idx = cb->args[1];
s_ip_idx = ip_idx = cb->args[2];
- if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
- ifa_ipv6_policy, NULL) >= 0) {
+ if (rtm_version == RTM_GETADDR2) {
+ int err;
+
+ err = nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb,
+ IFA_MAX, ifa_ipv6_policy, NULL);
+ if (err < 0)
+ return err;
+
if (tb[IFA_TARGET_NETNSID]) {
netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
@@ -5068,14 +5074,21 @@ static int inet6_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
{
enum addr_type_t type = UNICAST_ADDR;
- return inet6_dump_addr(skb, cb, type);
+ return inet6_dump_addr(skb, cb, type, RTM_GETADDR);
+}
+
+static int inet6_dump_ifaddr2(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ enum addr_type_t type = UNICAST_ADDR;
+
+ return inet6_dump_addr(skb, cb, type, RTM_GETADDR2);
}
static int inet6_dump_ifmcaddr(struct sk_buff *skb, struct netlink_callback *cb)
{
enum addr_type_t type = MULTICAST_ADDR;
- return inet6_dump_addr(skb, cb, type);
+ return inet6_dump_addr(skb, cb, type, RTM_GETMULTICAST);
}
@@ -5083,7 +5096,7 @@ static int inet6_dump_ifacaddr(struct sk_buff *skb, struct netlink_callback *cb)
{
enum addr_type_t type = ANYCAST_ADDR;
- return inet6_dump_addr(skb, cb, type);
+ return inet6_dump_addr(skb, cb, type, RTM_GETANYCAST);
}
static int inet6_rtm_getaddr(struct sk_buff *in_skb, struct nlmsghdr *nlh,
@@ -6833,6 +6846,11 @@ int __init addrconf_init(void)
RTNL_FLAG_DOIT_UNLOCKED);
if (err < 0)
goto errout;
+ err = rtnl_register_module(THIS_MODULE, PF_INET6, RTM_GETADDR2,
+ inet6_rtm_getaddr, inet6_dump_ifaddr2,
+ RTNL_FLAG_DOIT_UNLOCKED);
+ if (err < 0)
+ goto errout;
err = rtnl_register_module(THIS_MODULE, PF_INET6, RTM_GETMULTICAST,
NULL, inet6_dump_ifmcaddr, 0);
if (err < 0)
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 4/7] decnet: add RTM_GETADDR2
From: Christian Brauner @ 2018-09-27 17:58 UTC (permalink / raw)
To: jbenc, davem, dsahern, stephen, netdev, linux-kernel; +Cc: Christian Brauner
In-Reply-To: <20180927175857.3511-1-christian@brauner.io>
Various userspace programs (e.g. iproute2) have sent RTM_GETADDR
requests with struct ifinfomsg. This is wrong and should have been
struct ifaddrmsg all along as mandated by the manpages. However, dump
requests so far didn't parse the netlink message that was sent and
succeeded even when a wrong struct was passed along.
Currently, the message is parsed under the assumption that the correct
struct ifaddrmsg is sent down. If the parsing fails the kernel will
still fulfill the request to preserve backwards compatability but a
rate-limited message that there were leftover bytes after parsing the
message is recorded in dmesg. It has been argued that this is
unacceptable [1].
But various new features that got and will get added to RTM_GETADDR make
it necessary to definitely know what header was passed along.
This is currently not possible without resorting to (likely unreliable)
hacks such as introducing a nested attribute that ensures that
RTM_GETADDR which pass along properties such as IFA_TARGET_NETNSID
always exceed RTM_GETADDR requests that send down the wrong struct
ifinfomsg [2]. Basically, multiple approaches to solve this have been
shut down. Furthermore, the API expressed via RTM_GETADDR is apparently
frozen [3] so the wiggle room at this point seems very much zero.
The correct solution at this point seems to me to introduce a new
RTM_GETADDR2 request. This way we can parse the message and fail hard if
the struct is not struct ifaddrmsg and can safely extend it in the
future. Userspace tools that rely on the buggy RTM_GETADDR API will
still keep working without even having to see any log messages and new
userspace tools that want to make user of new features can make use of
the new RTM_GETADDR2 requests.
[1]: https://lists.openwall.net/netdev/2018/09/25/59
[2]: https://lists.openwall.net/netdev/2018/09/25/75
[3]: https://lists.openwall.net/netdev/2018/09/26/166
Signed-off-by: Christian Brauner <christian@brauner.io>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Benc <jbenc@redhat.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
---
net/decnet/dn_dev.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c
index d0b3e69c6b39..8da6ca154c08 100644
--- a/net/decnet/dn_dev.c
+++ b/net/decnet/dn_dev.c
@@ -738,10 +738,12 @@ static void dn_ifaddr_notify(int event, struct dn_ifaddr *ifa)
rtnl_set_sk_err(&init_net, RTNLGRP_DECnet_IFADDR, err);
}
-static int dn_nl_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
+static int dn_nl_dump_ifaddr_common(struct sk_buff *skb,
+ struct netlink_callback *cb, int rtm_type)
{
struct net *net = sock_net(skb->sk);
- int idx, dn_idx = 0, skip_ndevs, skip_naddr;
+ int err, idx, dn_idx = 0, skip_ndevs, skip_naddr;
+ struct nlattr *tb[IFA_MAX+1];
struct net_device *dev;
struct dn_dev *dn_db;
struct dn_ifaddr *ifa;
@@ -749,6 +751,13 @@ static int dn_nl_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
if (!net_eq(net, &init_net))
return 0;
+ if (rtm_type == RTM_GETADDR2) {
+ err = nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
+ dn_ifa_policy, NULL);
+ if (err < 0)
+ return err;
+ }
+
skip_ndevs = cb->args[0];
skip_naddr = cb->args[1];
@@ -787,6 +796,16 @@ static int dn_nl_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
return skb->len;
}
+static int dn_nl_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ return dn_nl_dump_ifaddr_common(skb, cb, RTM_GETADDR);
+}
+
+static int dn_nl_dump_ifaddr2(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ return dn_nl_dump_ifaddr_common(skb, cb, RTM_GETADDR2);
+}
+
static int dn_dev_get_first(struct net_device *dev, __le16 *addr)
{
struct dn_dev *dn_db;
@@ -1410,6 +1429,8 @@ void __init dn_dev_init(void)
dn_nl_deladdr, NULL, 0);
rtnl_register_module(THIS_MODULE, PF_DECnet, RTM_GETADDR,
NULL, dn_nl_dump_ifaddr, 0);
+ rtnl_register_module(THIS_MODULE, PF_DECnet, RTM_GETADDR2,
+ NULL, dn_nl_dump_ifaddr2, 0);
proc_create_seq("decnet_dev", 0444, init_net.proc_net, &dn_dev_seq_ops);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 6/7] selinux: add RTM_GETADDR2
From: Christian Brauner @ 2018-09-27 17:58 UTC (permalink / raw)
To: jbenc, davem, dsahern, stephen, netdev, linux-kernel; +Cc: Christian Brauner
In-Reply-To: <20180927175857.3511-1-christian@brauner.io>
Various userspace programs (e.g. iproute2) have sent RTM_GETADDR
requests with struct ifinfomsg. This is wrong and should have been
struct ifaddrmsg all along as mandated by the manpages. However, dump
requests so far didn't parse the netlink message that was sent and
succeeded even when a wrong struct was passed along.
Currently, the message is parsed under the assumption that the correct
struct ifaddrmsg is sent down. If the parsing fails the kernel will
still fulfill the request to preserve backwards compatability but a
rate-limited message that there were leftover bytes after parsing the
message is recorded in dmesg. It has been argued that this is
unacceptable [1].
But various new features that got and will get added to RTM_GETADDR make
it necessary to definitely know what header was passed along.
This is currently not possible without resorting to (likely unreliable)
hacks such as introducing a nested attribute that ensures that
RTM_GETADDR which pass along properties such as IFA_TARGET_NETNSID
always exceed RTM_GETADDR requests that send down the wrong struct
ifinfomsg [2]. Basically, multiple approaches to solve this have been
shut down. Furthermore, the API expressed via RTM_GETADDR is apparently
frozen [3] so the wiggle room at this point seems very much zero.
The correct solution at this point seems to me to introduce a new
RTM_GETADDR2 request. This way we can parse the message and fail hard if
the struct is not struct ifaddrmsg and can safely extend it in the
future. Userspace tools that rely on the buggy RTM_GETADDR API will
still keep working without even having to see any log messages and new
userspace tools that want to make user of new features can make use of
the new RTM_GETADDR2 requests.
[1]: https://lists.openwall.net/netdev/2018/09/25/59
[2]: https://lists.openwall.net/netdev/2018/09/25/75
[3]: https://lists.openwall.net/netdev/2018/09/26/166
Signed-off-by: Christian Brauner <christian@brauner.io>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Benc <jbenc@redhat.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
---
security/selinux/nlmsgtab.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
index 74b951f55608..3c45f50e987d 100644
--- a/security/selinux/nlmsgtab.c
+++ b/security/selinux/nlmsgtab.c
@@ -80,6 +80,7 @@ static const struct nlmsg_perm nlmsg_route_perms[] =
{ RTM_NEWSTATS, NETLINK_ROUTE_SOCKET__NLMSG_READ },
{ RTM_GETSTATS, NETLINK_ROUTE_SOCKET__NLMSG_READ },
{ RTM_NEWCACHEREPORT, NETLINK_ROUTE_SOCKET__NLMSG_READ },
+ { RTM_GETADDR2, NETLINK_ROUTE_SOCKET__NLMSG_READ },
};
static const struct nlmsg_perm nlmsg_tcpdiag_perms[] =
@@ -159,7 +160,7 @@ int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm)
switch (sclass) {
case SECCLASS_NETLINK_ROUTE_SOCKET:
/* RTM_MAX always point to RTM_SETxxxx, ie RTM_NEWxxx + 3 */
- BUILD_BUG_ON(RTM_MAX != (RTM_NEWCHAIN + 3));
+ BUILD_BUG_ON(RTM_MAX != (RTM_GETADDR2 + 1));
err = nlmsg_perm(nlmsg_type, perm, nlmsg_route_perms,
sizeof(nlmsg_route_perms));
break;
--
2.17.1
^ permalink raw reply related
* [PATCH net-next] tcp: up initial rmem to 128KB and SYN rwin to around 64KB
From: Yuchung Cheng @ 2018-09-27 18:21 UTC (permalink / raw)
To: davem, edumazet; +Cc: netdev, ncardwell, weiwan, soheil, Yuchung Cheng
Previously TCP initial receive buffer is ~87KB by default and
the initial receive window is ~29KB (20 MSS). This patch changes
the two numbers to 128KB and ~64KB (rounding down to the multiples
of MSS) respectively. The patch also simplifies the calculations s.t.
the two numbers are directly controlled by sysctl tcp_rmem[1]:
1) Initial receiver buffer budget (sk_rcvbuf): while this should
be configured via sysctl tcp_rmem[1], previously tcp_fixup_rcvbuf()
always override and set a larger size when a new connection
establishes.
2) Initial receive window in SYN: previously it is set to 20
packets if MSS <= 1460. The number 20 was based on the initial
congestion window of 10: the receiver needs twice amount to
avoid being limited by the receive window upon out-of-order
delivery in the first window burst. But since this only
applies if the receiving MSS <= 1460, connection using large MTU
(e.g. to utilize receiver zero-copy) may be limited by the
receive window.
With this patch TCP memory configuration is more straight-forward and
more properly sized to modern high-speed networks by default. Several
popular stacks have been announcing 64KB rwin in SYNs as well.
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Soheil Hassas Yeganeh <soheil@google.com>
---
net/ipv4/tcp.c | 4 ++--
net/ipv4/tcp_input.c | 25 ++-----------------------
net/ipv4/tcp_output.c | 25 ++++---------------------
3 files changed, 8 insertions(+), 46 deletions(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 69c236943f56..dcf51fbf5ec7 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3896,8 +3896,8 @@ void __init tcp_init(void)
init_net.ipv4.sysctl_tcp_wmem[2] = max(64*1024, max_wshare);
init_net.ipv4.sysctl_tcp_rmem[0] = SK_MEM_QUANTUM;
- init_net.ipv4.sysctl_tcp_rmem[1] = 87380;
- init_net.ipv4.sysctl_tcp_rmem[2] = max(87380, max_rshare);
+ init_net.ipv4.sysctl_tcp_rmem[1] = 131072;
+ init_net.ipv4.sysctl_tcp_rmem[2] = max(131072, max_rshare);
pr_info("Hash tables configured (established %u bind %u)\n",
tcp_hashinfo.ehash_mask + 1, tcp_hashinfo.bhash_size);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index d703a0b3b6a2..7a59f6a96212 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -426,26 +426,7 @@ static void tcp_grow_window(struct sock *sk, const struct sk_buff *skb)
}
}
-/* 3. Tuning rcvbuf, when connection enters established state. */
-static void tcp_fixup_rcvbuf(struct sock *sk)
-{
- u32 mss = tcp_sk(sk)->advmss;
- int rcvmem;
-
- rcvmem = 2 * SKB_TRUESIZE(mss + MAX_TCP_HEADER) *
- tcp_default_init_rwnd(mss);
-
- /* Dynamic Right Sizing (DRS) has 2 to 3 RTT latency
- * Allow enough cushion so that sender is not limited by our window
- */
- if (sock_net(sk)->ipv4.sysctl_tcp_moderate_rcvbuf)
- rcvmem <<= 2;
-
- if (sk->sk_rcvbuf < rcvmem)
- sk->sk_rcvbuf = min(rcvmem, sock_net(sk)->ipv4.sysctl_tcp_rmem[2]);
-}
-
-/* 4. Try to fixup all. It is made immediately after connection enters
+/* 3. Try to fixup all. It is made immediately after connection enters
* established state.
*/
void tcp_init_buffer_space(struct sock *sk)
@@ -454,8 +435,6 @@ void tcp_init_buffer_space(struct sock *sk)
struct tcp_sock *tp = tcp_sk(sk);
int maxwin;
- if (!(sk->sk_userlocks & SOCK_RCVBUF_LOCK))
- tcp_fixup_rcvbuf(sk);
if (!(sk->sk_userlocks & SOCK_SNDBUF_LOCK))
tcp_sndbuf_expand(sk);
@@ -485,7 +464,7 @@ void tcp_init_buffer_space(struct sock *sk)
tp->snd_cwnd_stamp = tcp_jiffies32;
}
-/* 5. Recalculate window clamp after socket hit its memory bounds. */
+/* 4. Recalculate window clamp after socket hit its memory bounds. */
static void tcp_clamp_window(struct sock *sk)
{
struct tcp_sock *tp = tcp_sk(sk);
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index fe7855b090e4..059b67af28b1 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -195,21 +195,6 @@ static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts,
inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
}
-
-u32 tcp_default_init_rwnd(u32 mss)
-{
- /* Initial receive window should be twice of TCP_INIT_CWND to
- * enable proper sending of new unsent data during fast recovery
- * (RFC 3517, Section 4, NextSeg() rule (2)). Further place a
- * limit when mss is larger than 1460.
- */
- u32 init_rwnd = TCP_INIT_CWND * 2;
-
- if (mss > 1460)
- init_rwnd = max((1460 * init_rwnd) / mss, 2U);
- return init_rwnd;
-}
-
/* Determine a window scaling and initial window to offer.
* Based on the assumption that the given amount of space
* will be offered. Store the results in the tp structure.
@@ -244,7 +229,10 @@ void tcp_select_initial_window(const struct sock *sk, int __space, __u32 mss,
if (sock_net(sk)->ipv4.sysctl_tcp_workaround_signed_windows)
(*rcv_wnd) = min(space, MAX_TCP_WINDOW);
else
- (*rcv_wnd) = space;
+ (*rcv_wnd) = min_t(u32, space, U16_MAX);
+
+ if (init_rcv_wnd)
+ *rcv_wnd = min(*rcv_wnd, init_rcv_wnd * mss);
(*rcv_wscale) = 0;
if (wscale_ok) {
@@ -257,11 +245,6 @@ void tcp_select_initial_window(const struct sock *sk, int __space, __u32 mss,
(*rcv_wscale)++;
}
}
-
- if (!init_rcv_wnd) /* Use default unless specified otherwise */
- init_rcv_wnd = tcp_default_init_rwnd(mss);
- *rcv_wnd = min(*rcv_wnd, init_rcv_wnd * mss);
-
/* Set the clamp no higher than max representable value */
(*window_clamp) = min_t(__u32, U16_MAX << (*rcv_wscale), *window_clamp);
}
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* Re: [PATCH net 01/11] netpoll: do not test NAPI_STATE_SCHED in poll_one_napi()
From: Michael Chan @ 2018-09-27 18:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Netdev, aviad.krawczyk, Song Liu, dougmill,
yisen.zhuang, mst, jasowang, harish.patil, Chopra, Manish,
netanel, linux-net-drivers, tlfalcon
In-Reply-To: <20180927163201.56609-2-edumazet@google.com>
On Thu, Sep 27, 2018 at 9:32 AM Eric Dumazet <edumazet@google.com> wrote:
>
> Since we do no longer require NAPI drivers to provide
> an ndo_poll_controller(), napi_schedule() has not been done
> before poll_one_napi() invocation.
>
> So testing NAPI_STATE_SCHED is likely to cause early returns.
>
> While we are at it, remove outdated comment.
>
> Note to future bisections : This change might surface prior
> bugs in drivers. See commit 73f21c653f93 ("bnxt_en: Fix TX
> timeout during netpoll.") for one occurrence.
>
> Fixes: ac3d9dd034e5 ("netpoll: make ndo_poll_controller() optional")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Tested-by: Song Liu <songliubraving@fb.com>
> Cc: Michael Chan <michael.chan@broadcom.com>
Reviewed-and-tested-by: Michael Chan <michael.chan@broadcom.com>
^ permalink raw reply
* pull request: bluetooth 2018-09-27
From: Johan Hedberg @ 2018-09-27 18:28 UTC (permalink / raw)
To: davem; +Cc: linux-bluetooth, netdev
[-- Attachment #1: Type: text/plain, Size: 934 bytes --]
Hi Dave,
Here's one more Bluetooth fix for 4.19, fixing the handling of an
attempt to unpair a device while pairing is in progress.
Let me know if there are any issues pulling. Thanks.
Johan
---
The following changes since commit 3f32d0be6c16b902b687453c962d17eea5b8ea19:
tipc: lock wakeup & inputq at tipc_link_reset() (2018-09-25 20:48:56 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git for-upstream
for you to fetch changes up to cb28c306b93b71f2741ce1a5a66289db26715f4d:
Bluetooth: SMP: fix crash in unpairing (2018-09-26 12:39:32 +0300)
----------------------------------------------------------------
Matias Karhumaa (1):
Bluetooth: SMP: fix crash in unpairing
net/bluetooth/mgmt.c | 7 ++-----
net/bluetooth/smp.c | 29 +++++++++++++++++++++++++----
net/bluetooth/smp.h | 3 ++-
3 files changed, 29 insertions(+), 10 deletions(-)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH net 01/11] netpoll: do not test NAPI_STATE_SCHED in poll_one_napi()
From: Song Liu @ 2018-09-27 18:33 UTC (permalink / raw)
To: Michael Chan
Cc: Eric Dumazet, David Miller, Netdev, aviad.krawczyk@huawei.com,
dougmill@linux.vnet.ibm.com, yisen.zhuang@huawei.com,
mst@redhat.com, jasowang@redhat.com, harish.patil@cavium.com,
Chopra, Manish, netanel@amazon.com,
linux-net-drivers@solarflare.com, tlfalcon@linux.vnet.ibm.com
In-Reply-To: <CACKFLikRw2a+JW1FCdR4o09Oz0Y0Cy3tNXdrA3K=EHNHykk=Ug@mail.gmail.com>
> On Sep 27, 2018, at 11:25 AM, Michael Chan <michael.chan@broadcom.com> wrote:
>
> On Thu, Sep 27, 2018 at 9:32 AM Eric Dumazet <edumazet@google.com> wrote:
>>
>> Since we do no longer require NAPI drivers to provide
>> an ndo_poll_controller(), napi_schedule() has not been done
>> before poll_one_napi() invocation.
>>
>> So testing NAPI_STATE_SCHED is likely to cause early returns.
>>
>> While we are at it, remove outdated comment.
>>
>> Note to future bisections : This change might surface prior
>> bugs in drivers. See commit 73f21c653f93 ("bnxt_en: Fix TX
>> timeout during netpoll.") for one occurrence.
>>
>> Fixes: ac3d9dd034e5 ("netpoll: make ndo_poll_controller() optional")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Tested-by: Song Liu <songliubraving@fb.com>
>> Cc: Michael Chan <michael.chan@broadcom.com>
>
> Reviewed-and-tested-by: Michael Chan <michael.chan@broadcom.com>
Reviewed-and-tested-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply
* Re: [PATCH net-next v6 23/23] net: WireGuard secure network tunnel
From: Jason A. Donenfeld @ 2018-09-28 1:09 UTC (permalink / raw)
To: Andrew Lunn
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman
In-Reply-To: <CAHmME9q7WRGF3TTRhhy0i_EB4ad2DaSD=tnHM92zfV4Cckyw=A@mail.gmail.com>
On Fri, Sep 28, 2018 at 12:37 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Will do. v7 will include the wg_ prefix.
$ nm *.o | while read a b c; do [[ $b == T ]] && echo $c; done | grep -v ^wg_
cleanup_module
init_module
Success.
^ permalink raw reply
* RE: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
From: Chris Preimesberger @ 2018-09-27 18:56 UTC (permalink / raw)
To: 'Andrew Lunn', Eran Ben Elisha
Cc: Neil Horman, linville@tuxdriver.com, netdev@vger.kernel.org
In-Reply-To: <20180927163805.GI12979@lunn.ch>
I greatly appreciate everyone's work on this. Thank you to all.
I've had Mellanox support case # 00508027 open for this issue,
and just now requested an updated driver from them to resolve,
explaining that really smart ethtool developers figured out this
was due to the Mellanox driver not reporting thresholds to ethtool.
I intend to post back here for posterity if/when I get an updated
driver that fixes the issue.
Thanks again!!
Chris Preimesberger | Test & Validation Engineer
Transition Networks, Inc.
chrisp@transition.com
direct: +1.952.996.1509 | fax: +1.952.941.2322 | www.transition.com
-----Original Message-----
From: Andrew Lunn [mailto:andrew@lunn.ch]
Sent: Thursday, September 27, 2018 11:38 AM
To: Chris Preimesberger
Cc: Eran Ben Elisha; Neil Horman; linville@tuxdriver.com; netdev@vger.kernel.org
Subject: Re: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
On Thu, Sep 27, 2018 at 04:08:24PM +0000, Chris Preimesberger wrote:
> Please correct me if I'm wrong, but...
> It looks like Eran's proposed fix would remove all warning and alarm
> indications from ethtool's output. It's worth mentioning that for me,
> the following fields always reported correctly as Off while no alarm
> condition was present and On while alarm condition(s) were present
> *per the QSFP's true/programmed threshold values* *not per the
> incorrectly reported threshold values*
These alarm values are in the first page. So the information the driver returns does contain this information. What is missing is the thresholds, which are not provided by the driver.
But there is a comment in the code:
/*
* There is no clear identifier to signify the existence of
* optical diagnostics similar to SFF-8472. So checking existence
* of page 3, will provide the gurantee for existence of alarms
* and thresholds
* If pagging support exists, then supports_alarms is marked as 1
*/
These alarm values are optional. The spec says so. So in order to decide if they are implemented, ethtool looks to see if the thresholds are available. If there are thresholds, it makes sense the alarms are implemented.
Unfortunately, the driver never returns the thresholds. So ethtool has no real choice and won't display the alarms since it cannot determine if they are valid.
In order to get alarms, the driver needs to be extended to return all the pages.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v6 00/23] WireGuard: Secure Network Tunnel
From: Eric Biggers @ 2018-09-28 1:17 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: linux-kernel, netdev, linux-crypto, davem, gregkh
In-Reply-To: <20180927213537.GA27576@zx2c4.com>
On Thu, Sep 27, 2018 at 11:35:39PM +0200, Jason A. Donenfeld wrote:
> Hi Eric,
>
> On Thu, Sep 27, 2018 at 8:29 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > Why is Herbert Xu's existing crypto tree being circumvented, especially for
> > future patches (the initial merge isn't quite as important as that's a one-time
> > event)? I like being able to check out cryptodev to test upcoming crypto
> > patches. And currently, changes to APIs, algorithms, tests, and implementations
> > all go through cryptodev, which is convenient for crypto developers.
> >
> > Apparently, you're proposing that someone adding a new algorithm will now have
> > to submit the API portion to one maintainer (Herbert Xu) and the implementation
> > portion to another maintainer (you), and they'll go through separate git trees.
> > That's inconvenient for developers, and it seems that in practice you and
> > Herbert will be stepping on each other's toes a lot.
> >
> > Can you please reach some kind of sane agreement with Herbert so that the
> > development process isn't fractured into two? Perhaps you could review patches,
> > but Herbert could still apply them?
>
> I think you're overthinking it a bit. Zinc will have a few software
> implementations of primitives that are useful in cases where it's nice to call
> the primitive directly. Think: various usages of sha2, siphash, the wireguard
> suite (what this patchset includes), other things in lib/, etc. In so much as
> this winds up duplicating things within the crypto API, I'll work with Herbert
> to build one on top of the other -- as I've done in the two commits in this
> series. But beyond that, think of the two initiatives as orthogonal. I'm
> working on curating a few primitives that are maximally useful throughout
> the kernel for various uses, and doing so in a way that I think brings
> about a certain quality. Meanwhile the crypto API is amassing a huge
> collection of primitives for some things, and that will continue to exist,
> and Herbert will continue to maintain that. I expect for the crossover
> to be fairly isolated and manageable, without too much foreseeable tree-
> conflicts and such. Therefore, Samuel Neves and I plan to maintain the
> codebase we've spent quite some time writing, and maintain our own tree for
> it, which we'll be submitting through Greg. In other words, this is not
> a matter of "circumvention" or "stepping on toes", but rather separate
> efforts. I'm quite certain to the extent they overlap we'll be able to work
> out fairly easily.
>
> Either way, I'll take your suggestion and reach out to Herbert, since at
> least a discussion between the two of us sounds like it could be productive.
So, Zinc will simultaneously replace the current crypto implementations, *and*
be "orthogonal" and "separate" from all the crypto code currently maintained by
Herbert? You can't have your cake and eat it too...
I'm still concerned you're splitting the community in two. It will be unclear
where new algorithms and implementations should go. Some people will choose
Herbert and the current crypto API and conventions, and some people will choose
you and Zinc... I still don't see clear guidelines for what will go where. And
yes, you and Herbert will step on each others' toes and duplicate stuff, as the
efforts are *not* separate, as you've even argued yourself.
Please reach out to Herbert to find a sane solution, ideally one that involves
having a single git tree for crypto development and allows people to continue
crypto development without choosing "sides".
>
> > I'm also wondering about the criteria for making additions and changes to
> > "Zinc". You mentioned before that one of the "advantages" of Zinc is that it
> > doesn't include "cipher modes from 90s cryptographers" -- what does that mean
> > exactly? You've also indicated before that you don't want people modifying the
> > Poly1305 implementations as they are too error-prone. Useful contributions
> > could be blocked or discouraged in the future. Can you please elaborate on
> > your criteria for contributions to Zinc?
> >
> > Also, will you allow algorithms that aren't up to modern security standards but
> > are needed for compatibility reasons, e.g. MD5, SHA-1, and DES? There are
> > existing standards, APIs, and data formats that use these "legacy" algorithms;
> > so implementations of them are often still needed, whether we like it or not.
> >
> > And does it matter who designed the algorithms, e.g. do algorithms from Daniel
> > Bernstein get effectively a free pass, while algorithms from certain countries,
> > governments, or organizations are not allowed? E.g. wireless driver developers
> > may need the SM4 block cipher (which is now supported by the crypto API) as it's
> > specified in a Chinese wireless standard. Will you allow SM4 in Zinc? Or will
> > people have to submit some algorithms to Herbert and some to you due to
> > disagreements about what algorithms should be included?
>
> Similarly here, I think you're over-politicizing everything. Stable address
> generation for IPv6 uses SHA1 -- see net/ipv6/addrconf.c:3203 -- do you think
> that this should use, say, the SM3 chinese hash function instead? No, of
> course not, for a variety of interesting reasons. Rather, it should use some
> simple hash function that's fast in software that we have available in Zinc.
> On the other hand, it seems like parts of the kernel that have pretty high-
> levels of cipher agility -- such as dmcrypt, ipsec, wifi apparently, and
> so on -- will continue to use dynamic-dispatch system like the crypto API,
> since that's what it was made to do and is effective at doing. And so, your
> example of SM4 seems to fit perfectly into what the crypto API is well-suited
> for, and it would fit naturally in there.
>
> In other words, the "political criteria" for what we add to lib/zinc/ will
> mostly be the same as for the rest of lib/: are there things using it that
> benefit from it being there in a direct and obvious way, and does the
> implementation meet certain quality standards.
>
So, crypto implementations and algorithms will go to different maintainers,
source locations, and git trees based purely on whether the current users need
"cipher agility"? Note that usage can change over time; a user that requires a
single cipher could later need multiple, and vice versa.
What if the portion of a wireless driver that needs SM4 doesn't need any other
cipher in the same place, so static dispatch would suffice? Would SM4 be
allowed in Zinc then?
> > to change them yourself, e.g. when you added the part that converts the
> > accumulator from base 26 to base 32. I worry there may be double standards
> > here
>
> We do actually appreciate your concern here. However, there's a lot more that
> went into that short patch than meets the eye:
>
> - It matches exactly what Andy Polyakov's code is doing for the exact
> same reason, so this isn't something that's actually "new". (There
> are paths inside his implementation that branch from the vector code
> to the scalar code.)
Matches Andy's code, where? The reason you had to add the radix conversion is
because his code does *not* handle it...
> - It has been discussed at length with Andy, including what kinds of
> proofs we'll need if we want to chop it down further (to remove that
> final reduction), and why we both don't want to do that yet, and so
> we go with the full carrying for the avoidance of risk.
Sorry, other people don't know about your private discussions. For the rest of
us, why not add a comment to the code explaining what's going on?
> - We've proved its correctness with Z3, actually using an even looser
> constraint on digit size than what Andy mentioned to us, thus resulting
> in a stronger proof result. So we're certain this isn't rubbish.
AFAICS actually it *is* rubbish, because your C code stores the accumulator as
64-bit integers whereas the asm code (at least, the 32-bit version) reads it as
32-bit integers. That won't work correctly on big endian ARM.
> - There's been some considerable computing power sunk into fuzzing it.
>
> There's no doubt about it, we've done our due-diligence here.
Apparently not, given that it's broken on big endian ARM.
> fact the kind of efforts we require of submissions. You could fault us for
> not detailing this in "the commit message" -- except as this is still a
> patch series, we're putting improvements into the 00/XX change log, instead
> of adding fixes and additions on top of the series.
The details of the correctness proofs and fuzzing you claim to have done aren't
explained, even in the cover letter; so for now we just have to trust you on
that point. Of course, having bugs in code which you insist was proven correct
+ fuzzed doesn't exactly inspire trust.
I understand that your standards are still as high or even higher than
Herbert's, which is good; crypto code should be held to high standards! But
based on the evidence, I do worry there's a double standard going on where you
get away with things yourself which you won't allow from others in Zinc. It's
just not honest, and it will make people not want to contribute to Zinc.
Maintainers are supposed to be unbiased and hold all contributions to the same
standard.
We need "Zinc" to be Linux's crypto library, not "Jason's crypto library".
- Eric
^ permalink raw reply
* Re: [PATCH v2 bpf-next] bpf: test_bpf: add init_net to dev for flow_dissector
From: Daniel Borkmann @ 2018-09-27 19:13 UTC (permalink / raw)
To: Willem de Bruijn, Eric Dumazet
Cc: songliubraving, Network Development, Kernel Team,
Willem de Bruijn, Petar Penkov
In-Reply-To: <CAF=yD-K4wxZoNLDwba_-AEqWkN=5wz0XvW0eRrjZnDPqchhCuQ@mail.gmail.com>
On 09/27/2018 06:42 PM, Willem de Bruijn wrote:
> On Thu, Sep 27, 2018 at 12:40 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On 09/27/2018 09:34 AM, Song Liu wrote:
>>> Latest changes in __skb_flow_dissect() assume skb->dev has valid nd_net.
>>> However, this is not true for test_bpf. As a result, test_bpf.ko crashes
>>> the system with the following stack trace:
>>>
>>> This patch fixes tes_bpf by using init_net in the dummy dev.
>>>
>>> Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
>>> Reported-by: Eric Dumazet <edumazet@google.com>
>>> Cc: Willem de Bruijn <willemb@google.com>
>>> Cc: Petar Penkov <ppenkov@google.com>
>>> Signed-off-by: Song Liu <songliubraving@fb.com>
>>> ---
>>
>> Reviewed-by: Eric Dumazet <edumazet@google.com>
>
> Acked-by: Willem de Bruijn <willemb@google.com>
>
> Thanks!
Applied to bpf-next, thanks everyone!
^ permalink raw reply
* Re: WARN_ON in TLP causing RT throttling
From: Yuchung Cheng @ 2018-09-27 19:14 UTC (permalink / raw)
To: Eric Dumazet; +Cc: stranche, soheil@google.com
In-Reply-To: <3c112d25-1105-524f-e210-8a7cb63df1c7@gmail.com>
On Wed, Sep 26, 2018 at 5:09 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 09/26/2018 04:46 PM, stranche@codeaurora.org wrote:
> > Hi Eric,
> >
> > Someone recently reported a crash to us on the 4.14.62 kernel where excessive
> > WARNING prints were spamming the logs and causing watchdog bites. The kernel
> > does have the following commit by Soheil:
> > bffd168c3fc5 "tcp: clear tp->packets_out when purging write queue"
> >
> > Before this bug we see over 1 second of continuous WARN_ON prints from
> > tcp_send_loss_probe() like so:
> >
> > 7795.530450: <2> tcp_send_loss_probe+0x194/0x1b8
> > 7795.534833: <2> tcp_write_timer_handler+0xf8/0x1c4
> > 7795.539492: <2> tcp_write_timer+0x4c/0x74
> > 7795.543348: <2> call_timer_fn+0xc0/0x1b4
> > 7795.547113: <2> run_timer_softirq+0x248/0x81c
> >
> > Specifically, the prints come from the following check:
> >
> > /* Retransmit last segment. */
> > if (WARN_ON(!skb))
> > goto rearm_timer;
> >
> > Since skb is always NULL, we know there's nothing on the write queue or the
> > retransmit queue, so we just keep resetting the timer, waiting for more data
> > to be queued. However, we were able to determine that the TCP socket is in the
> > TCP_FIN_WAIT1 state, so we will no longer be sending any data and these queues
> > remain empty.
> >
> > Would it be appropriate to stop resetting the TLP timer if we detect that the
> > connection is starting to close and we have no more data to send the probe with,
> > or is there some way that this scenario should already be handled?
> >
> > Unfortunately, we don't have a reproducer for this crash.
> >
>
> Something is fishy.
>
> If there is no skb in the queues, then tp->packets_out should be 0,
> therefore tcp_rearm_rto() should simply call inet_csk_clear_xmit_timer(sk, ICSK_TIME_RETRANS);
>
> I have never seen this report before.
Do you use Fast Open? I am wondering if its a bug when a TFO server
closes the socket before the handshake finishes...
Either way, it's pretty safe to just stop TLP if write queue is empty
for any unexpected reason.
>
^ permalink raw reply
* Re: [PATCH bpf-next 0/5] Introduce libbpf_attach_type_by_name
From: Daniel Borkmann @ 2018-09-27 19:21 UTC (permalink / raw)
To: Andrey Ignatov, netdev; +Cc: ast, kernel-team
In-Reply-To: <cover.1538000102.git.rdna@fb.com>
On 09/27/2018 12:24 AM, Andrey Ignatov wrote:
> This patch set introduces libbpf_attach_type_by_name function in libbpf to
> identify attach type by section name.
>
> This is useful to avoid writing same logic over and over again in user
> space applications that leverage libbpf.
>
> Patch 1 has more details on the new function and problem being solved.
> Patches 2 and 3 add support for new section names.
> Patch 4 uses new function in a selftest.
> Patch 5 adds selftest for libbpf_{prog,attach}_type_by_name.
>
> As a side note there are a lot of inconsistencies now between names used by
> libbpf and bpftool (e.g. cgroup/skb vs cgroup_skb, cgroup_device and device
> vs cgroup/dev, sockops vs sock_ops, etc). This patch set does not address
> it but it tries not to make it harder to address it in the future.
>
>
> Andrey Ignatov (5):
> libbpf: Introduce libbpf_attach_type_by_name
> libbpf: Support cgroup_skb/{e,in}gress section names
> libbpf: Support sk_skb/stream_{parser,verdict} section names
> selftests/bpf: Use libbpf_attach_type_by_name in test_socket_cookie
> selftests/bpf: Test libbpf_{prog,attach}_type_by_name
Applied to bpf-next, thanks Andrey!
^ permalink raw reply
* [PATCH v2 net-next 0/2] Add support for Microchip Technology KSZ9131 10/100/1000 Ethernet PHY
From: Yuiko Oshino @ 2018-09-27 20:15 UTC (permalink / raw)
To: davem, robh+dt, devicetree, f.fainelli, andrew
Cc: linux-kernel, mark.rutland, m.felsch, Markus.Niebel, netdev,
UNGLinuxDriver
This is the initial driver for Microchip KSZ9131 10/100/1000 Ethernet PHY
v2:
- Creating a series from two related patches.
Yuiko Oshino (2):
net: phy: micrel: add Microchip KSZ9131 inital driver
dt-bindings: net: add support for Microchip KSZ9131 Ethernet PHY
.../devicetree/bindings/net/micrel-ksz90x1.txt | 29 +++++++++++++++++++-
drivers/net/phy/micrel.c | 32 ++++++++++++++++++++--
include/linux/micrel_phy.h | 1 +
3 files changed, 58 insertions(+), 4 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v2 net-next 1/2] net: phy: micrel: add Microchip KSZ9131 inital driver
From: Yuiko Oshino @ 2018-09-27 20:16 UTC (permalink / raw)
To: davem, robh+dt, devicetree, f.fainelli, andrew
Cc: linux-kernel, mark.rutland, m.felsch, Markus.Niebel, netdev,
UNGLinuxDriver
Add support for Microchip Technology KSZ9131 10/100/1000 Ethernet PHY
Signed-off-by: Yuiko Oshino <yuiko.oshino@microchip.com>
---
drivers/net/phy/micrel.c | 32 +++++++++++++++++++++++++++++---
include/linux/micrel_phy.h | 1 +
2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 3db06b4..a4473cb 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -14,7 +14,7 @@
* option) any later version.
*
* Support : Micrel Phys:
- * Giga phys: ksz9021, ksz9031
+ * Giga phys: ksz9021, ksz9031, ksz9131
* 100/10 Phys : ksz8001, ksz8721, ksz8737, ksz8041
* ksz8021, ksz8031, ksz8051,
* ksz8081, ksz8091,
@@ -425,6 +425,7 @@ static int ksz9021_config_init(struct phy_device *phydev)
#define MII_KSZ9031RN_MMD_REGDATA_REG 0x0e
#define OP_DATA 1
#define KSZ9031_PS_TO_REG 60
+#define KSZ9131_PS_TO_REG 100
/* Extended registers */
/* MMD Address 0x0 */
@@ -470,6 +471,10 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
u16 maxval;
u16 newval;
int i;
+ int pstoreg = KSZ9031_PS_TO_REG;
+
+ if (phydev->drv->phy_id == PHY_ID_KSZ9131)
+ pstoreg = KSZ9131_PS_TO_REG;
for (i = 0; i < numfields; i++)
if (!of_property_read_u32(of_node, field[i], val + i))
@@ -489,7 +494,7 @@ static int ksz9031_of_load_skew_values(struct phy_device *phydev,
mask = 0xffff;
mask ^= maxval << (field_sz * i);
newval = (newval & mask) |
- (((val[i] / KSZ9031_PS_TO_REG) & maxval)
+ (((val[i] / pstoreg) & maxval)
<< (field_sz * i));
}
@@ -602,7 +607,10 @@ static int ksz9031_config_init(struct phy_device *phydev)
}
}
- return ksz9031_center_flp_timing(phydev);
+ if (phydev->drv->phy_id == PHY_ID_KSZ9031)
+ return ksz9031_center_flp_timing(phydev);
+ else
+ return 0;
err_force_master:
phydev_err(phydev, "failed to force the phy to master mode\n");
@@ -975,6 +983,23 @@ static struct phy_driver ksphy_driver[] = {
.suspend = genphy_suspend,
.resume = kszphy_resume,
}, {
+ .phy_id = PHY_ID_KSZ9131,
+ .phy_id_mask = MICREL_PHY_ID_MASK,
+ .name = "Microchip KSZ9131 Gigabit PHY",
+ .features = PHY_GBIT_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
+ .driver_data = &ksz9021_type,
+ .probe = kszphy_probe,
+ .config_init = ksz9031_config_init,
+ .read_status = ksz9031_read_status,
+ .ack_interrupt = kszphy_ack_interrupt,
+ .config_intr = kszphy_config_intr,
+ .get_sset_count = kszphy_get_sset_count,
+ .get_strings = kszphy_get_strings,
+ .get_stats = kszphy_get_stats,
+ .suspend = genphy_suspend,
+ .resume = kszphy_resume,
+}, {
.phy_id = PHY_ID_KSZ8873MLL,
.phy_id_mask = MICREL_PHY_ID_MASK,
.name = "Micrel KSZ8873MLL Switch",
@@ -1022,6 +1047,7 @@ MODULE_LICENSE("GPL");
static struct mdio_device_id __maybe_unused micrel_tbl[] = {
{ PHY_ID_KSZ9021, 0x000ffffe },
{ PHY_ID_KSZ9031, MICREL_PHY_ID_MASK },
+ { PHY_ID_KSZ9131, MICREL_PHY_ID_MASK },
{ PHY_ID_KSZ8001, 0x00fffffc },
{ PHY_ID_KS8737, MICREL_PHY_ID_MASK },
{ PHY_ID_KSZ8021, 0x00ffffff },
diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index 472fa4d..7361cd3 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -31,6 +31,7 @@
#define PHY_ID_KSZ8081 0x00221560
#define PHY_ID_KSZ8061 0x00221570
#define PHY_ID_KSZ9031 0x00221620
+#define PHY_ID_KSZ9131 0x00221640
#define PHY_ID_KSZ886X 0x00221430
#define PHY_ID_KSZ8863 0x00221435
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next] tcp: up initial rmem to 128KB and SYN rwin to around 64KB
From: Yuchung Cheng @ 2018-09-27 19:39 UTC (permalink / raw)
To: David Miller, Eric Dumazet
Cc: netdev, Neal Cardwell, Wei Wang, Soheil Hassas Yeganeh,
Yuchung Cheng
In-Reply-To: <20180927182119.161132-1-ycheng@google.com>
On Thu, Sep 27, 2018 at 11:21 AM, Yuchung Cheng <ycheng@google.com> wrote:
> Previously TCP initial receive buffer is ~87KB by default and
> the initial receive window is ~29KB (20 MSS). This patch changes
> the two numbers to 128KB and ~64KB (rounding down to the multiples
> of MSS) respectively. The patch also simplifies the calculations s.t.
> the two numbers are directly controlled by sysctl tcp_rmem[1]:
>
> 1) Initial receiver buffer budget (sk_rcvbuf): while this should
> be configured via sysctl tcp_rmem[1], previously tcp_fixup_rcvbuf()
> always override and set a larger size when a new connection
> establishes.
>
> 2) Initial receive window in SYN: previously it is set to 20
> packets if MSS <= 1460. The number 20 was based on the initial
> congestion window of 10: the receiver needs twice amount to
> avoid being limited by the receive window upon out-of-order
> delivery in the first window burst. But since this only
> applies if the receiving MSS <= 1460, connection using large MTU
> (e.g. to utilize receiver zero-copy) may be limited by the
> receive window.
>
> With this patch TCP memory configuration is more straight-forward and
> more properly sized to modern high-speed networks by default. Several
> popular stacks have been announcing 64KB rwin in SYNs as well.
Sorry please ignore this patch for now.
We need to adjust rbuf autotuning as well otherwise w/ larger init
rbuf it may increase too slowly during slow start. Will submit a v2
>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: Wei Wang <weiwan@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reviewed-by: Soheil Hassas Yeganeh <soheil@google.com>
> ---
> net/ipv4/tcp.c | 4 ++--
> net/ipv4/tcp_input.c | 25 ++-----------------------
> net/ipv4/tcp_output.c | 25 ++++---------------------
> 3 files changed, 8 insertions(+), 46 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 69c236943f56..dcf51fbf5ec7 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -3896,8 +3896,8 @@ void __init tcp_init(void)
> init_net.ipv4.sysctl_tcp_wmem[2] = max(64*1024, max_wshare);
>
> init_net.ipv4.sysctl_tcp_rmem[0] = SK_MEM_QUANTUM;
> - init_net.ipv4.sysctl_tcp_rmem[1] = 87380;
> - init_net.ipv4.sysctl_tcp_rmem[2] = max(87380, max_rshare);
> + init_net.ipv4.sysctl_tcp_rmem[1] = 131072;
> + init_net.ipv4.sysctl_tcp_rmem[2] = max(131072, max_rshare);
>
> pr_info("Hash tables configured (established %u bind %u)\n",
> tcp_hashinfo.ehash_mask + 1, tcp_hashinfo.bhash_size);
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index d703a0b3b6a2..7a59f6a96212 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -426,26 +426,7 @@ static void tcp_grow_window(struct sock *sk, const struct sk_buff *skb)
> }
> }
>
> -/* 3. Tuning rcvbuf, when connection enters established state. */
> -static void tcp_fixup_rcvbuf(struct sock *sk)
> -{
> - u32 mss = tcp_sk(sk)->advmss;
> - int rcvmem;
> -
> - rcvmem = 2 * SKB_TRUESIZE(mss + MAX_TCP_HEADER) *
> - tcp_default_init_rwnd(mss);
> -
> - /* Dynamic Right Sizing (DRS) has 2 to 3 RTT latency
> - * Allow enough cushion so that sender is not limited by our window
> - */
> - if (sock_net(sk)->ipv4.sysctl_tcp_moderate_rcvbuf)
> - rcvmem <<= 2;
> -
> - if (sk->sk_rcvbuf < rcvmem)
> - sk->sk_rcvbuf = min(rcvmem, sock_net(sk)->ipv4.sysctl_tcp_rmem[2]);
> -}
> -
> -/* 4. Try to fixup all. It is made immediately after connection enters
> +/* 3. Try to fixup all. It is made immediately after connection enters
> * established state.
> */
> void tcp_init_buffer_space(struct sock *sk)
> @@ -454,8 +435,6 @@ void tcp_init_buffer_space(struct sock *sk)
> struct tcp_sock *tp = tcp_sk(sk);
> int maxwin;
>
> - if (!(sk->sk_userlocks & SOCK_RCVBUF_LOCK))
> - tcp_fixup_rcvbuf(sk);
> if (!(sk->sk_userlocks & SOCK_SNDBUF_LOCK))
> tcp_sndbuf_expand(sk);
>
> @@ -485,7 +464,7 @@ void tcp_init_buffer_space(struct sock *sk)
> tp->snd_cwnd_stamp = tcp_jiffies32;
> }
>
> -/* 5. Recalculate window clamp after socket hit its memory bounds. */
> +/* 4. Recalculate window clamp after socket hit its memory bounds. */
> static void tcp_clamp_window(struct sock *sk)
> {
> struct tcp_sock *tp = tcp_sk(sk);
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index fe7855b090e4..059b67af28b1 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -195,21 +195,6 @@ static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts,
> inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
> }
>
> -
> -u32 tcp_default_init_rwnd(u32 mss)
> -{
> - /* Initial receive window should be twice of TCP_INIT_CWND to
> - * enable proper sending of new unsent data during fast recovery
> - * (RFC 3517, Section 4, NextSeg() rule (2)). Further place a
> - * limit when mss is larger than 1460.
> - */
> - u32 init_rwnd = TCP_INIT_CWND * 2;
> -
> - if (mss > 1460)
> - init_rwnd = max((1460 * init_rwnd) / mss, 2U);
> - return init_rwnd;
> -}
> -
> /* Determine a window scaling and initial window to offer.
> * Based on the assumption that the given amount of space
> * will be offered. Store the results in the tp structure.
> @@ -244,7 +229,10 @@ void tcp_select_initial_window(const struct sock *sk, int __space, __u32 mss,
> if (sock_net(sk)->ipv4.sysctl_tcp_workaround_signed_windows)
> (*rcv_wnd) = min(space, MAX_TCP_WINDOW);
> else
> - (*rcv_wnd) = space;
> + (*rcv_wnd) = min_t(u32, space, U16_MAX);
> +
> + if (init_rcv_wnd)
> + *rcv_wnd = min(*rcv_wnd, init_rcv_wnd * mss);
>
> (*rcv_wscale) = 0;
> if (wscale_ok) {
> @@ -257,11 +245,6 @@ void tcp_select_initial_window(const struct sock *sk, int __space, __u32 mss,
> (*rcv_wscale)++;
> }
> }
> -
> - if (!init_rcv_wnd) /* Use default unless specified otherwise */
> - init_rcv_wnd = tcp_default_init_rwnd(mss);
> - *rcv_wnd = min(*rcv_wnd, init_rcv_wnd * mss);
> -
> /* Set the clamp no higher than max representable value */
> (*window_clamp) = min_t(__u32, U16_MAX << (*rcv_wscale), *window_clamp);
> }
> --
> 2.19.0.605.g01d371f741-goog
>
^ permalink raw reply
* Re: [PATCH net-next v6 00/23] WireGuard: Secure Network Tunnel
From: Jason A. Donenfeld @ 2018-09-28 2:35 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-kernel, netdev, linux-crypto, davem, gregkh
In-Reply-To: <20180928011726.GA98113@gmail.com>
Hi Eric,
On Thu, Sep 27, 2018 at 06:17:27PM -0700, Eric Biggers wrote:
> So, Zinc will simultaneously replace the current crypto implementations, *and*
> be "orthogonal" and "separate" from all the crypto code currently maintained by
> Herbert? You can't have your cake and eat it too...
The plan is for it to replace many uses of the crypto API where it makes
sense, but not replace uses where it doesn't make sense. Perhaps in the
long run, over time, its usage will grow to cover those cases too, or,
perhaps instead, Zinc will form a simple basis of software crypto
algorithms in whatever future API designs crop up. In other words, like
most changes in kernel development, things happen gradually, starting
with a few good cases, and gradually growing as the need and design
arise.
> I'm still concerned you're splitting the community in two. It will be unclear
> where new algorithms and implementations should go. Some people will choose
> Herbert and the current crypto API and conventions, and some people will choose
> you and Zinc... I still don't see clear guidelines for what will go where.
I can try to work out some explicit guidelines and write these up for
Documentation/, if that'd make a difference for you. I don't think this
is a matter of "community splitting". On the contrary, I think Zinc will
bring communities together, inviting the larger cryptography community
to take an interest in improving the state of crypto in Linux. Either
way, the litmus test for where code should go remains pretty similar to
how it's been working so far. Are you tempted to stick it in lib/
because that fits your programming paradigm and because you think it's
generally useful? If so, submit it to lib/zinc/. Conversely, is it only
something used in the large array of options provided by dmcrypt, ipsec,
afalg, etc? Submit it to the crypto API.
If you think this criteria is lacking, I'm amenable to adjusting that
and changing it, especially as situations and designs change and morph
over time. But that seems like a fairly decent starting point.
> Please reach out to Herbert to find a sane solution
> crypto development without choosing "sides".
Please, don't politicize this. This has nothing to do with "sides". This
has to do with which paradigm makes sense for implementing a particular
algorithm. And everything that goes in Zinc gets to be used seamlessly
by the crypto API anyway, through use of the trivial stub glue code,
like what I've shown in the two commits in this series. Again, if it's
something that will work well as a direct function call, then it seems
like Zinc makes sense as a home for it.
With that said, I've reached out to Herbert, and we'll of course discuss
and reach a good conclusion together.
> Note that usage can change over time; a user that requires a
> single cipher could later need multiple, and vice versa.
I think this depends on the design of the driver and the style it's
implemented in. For example, I could imagine something like this:
encrypt_stuff_with_morus(obj, key);
evolving over time to:
if (obj->type == MORUS_TYPE)
encrypt_stuff_with_morus(obj, key);
else if (obj->type == AEGIS_TYPE)
encrypt_stuff_with_aegis(obj, key);
On the other hand, if the developer has good reason to use the crypto
API's dynamic dispatch and async API and so forth, then perhaps it just
changes from:
static const char *cipher_name = "morus";
to
static const char *cipher_name_type_1 = "morus";
static const char *cipher_name_type_2 = "aegis";
I can imagine both programming styles and evolutions being desirable for
different reasons.
> > - It matches exactly what Andy Polyakov's code is doing for the exact
> > same reason, so this isn't something that's actually "new". (There
> > are paths inside his implementation that branch from the vector code
> > to the scalar code.)
>
> Matches Andy's code, where? The reason you had to add the radix conversion is
> because his code does *not* handle it...
For example, check out the avx blocks function. The radix conversion
happens in a few different places throughout. The reason we need it
separately here is because, unlike userspace, it's possible the kernel
code will transition from 2^26 back to 2^64 as a result of the FPU
context changing.
As well, AndyP seems to like the idea of including this logic in the
assembly instead of in C, if I understood our discussions correctly, so
there's a decent chance this will migrate out of the glue code and into
the assembly properly, which is probably a better place for it.
> > - It has been discussed at length with Andy, including what kinds of
> > proofs we'll need if we want to chop it down further (to remove that
> > final reduction), and why we both don't want to do that yet, and so
> > we go with the full carrying for the avoidance of risk.
>
> Sorry, other people don't know about your private discussions. For the rest of
> us, why not add a comment to the code explaining what's going on?
That's a good idea. I can include some discussion about this as well in
the commit message that introduces the glue code, too, I guess? I've
been hesitant to fill these commit messages up even more, given there
are already so many walls of text and whatnot, but if you think that'd
be useful, I'll do that for v7, and also add comments.
> > - We've proved its correctness with Z3, actually using an even looser
> > constraint on digit size than what Andy mentioned to us, thus resulting
> > in a stronger proof result. So we're certain this isn't rubbish.
> AFAICS actually it *is* rubbish, because your C code stores the accumulator as
> 64-bit integers whereas the asm code (at least, the 32-bit version) reads it as
> 32-bit integers. That won't work correctly on big endian ARM.
> > There's no doubt about it, we've done our due-diligence here.
> Apparently not, given that it's broken on big endian ARM.
> Of course, having bugs in code which you insist was proven correct
> + fuzzed doesn't exactly inspire trust.
What's with the snark? It's not rubbish. I'm not sure if you noticed it in
the development trees (both the WireGuard module tree and my kernel.org
integration tree for this patch), but the big endian ARM support was fixed
pretty shortly after I jumped the gun posting v6. Like, super soon after.
That, and other big endian fixes (on aarch64 as well) are already queued up
for v7. And now build.wireguard.com has more big endian running in CI.
> The details of the correctness proofs and fuzzing you claim to have done aren't
> explained, even in the cover letter; so for now we just have to trust you on
> that point.
"Claim to have done", "trust you on that point" -- I think there's no
reason to doubt the integrity of my "claims", and I don't appreciate the
phrasing that appears to call that into question.
Regardless, sure, we can expand the "wall-of-text" commit messages even
further, if you want, and include the verbatim Z3 scripts for reproduction.
> I understand that your standards are still as high or even higher than
> Herbert's, which is good; crypto code should be held to high standards! But
> based on the evidence, I do worry there's a double standard going on where you
> get away with things yourself which you won't allow from others in Zinc. It's
> just not honest, and it will make people not want to contribute to Zinc.
> Maintainers are supposed to be unbiased and hold all contributions to the same
> standard.
This is complete and utter garbage, and I find its insinuations insulting
and ridiculous. There is absolutely no lack of honesty and no double
standard being applied whatsoever. Your attempt to cast doubt about the
quality of standards applied and the integrity of the process is wholly
inappropriate. When I tell you that high standards were applied and that
due-diligence was done in developing a particular patch, I mean what I
say.
> We need "Zinc" to be Linux's crypto library, not "Jason's crypto library".
This very much is a project directed toward the benefit of the kernel in
a general sense. It's been this way from the start, and there's nothing
in its goals or plans to the contrary of that. Please leave this vague
and unproductive rhetoric aside.
Jason
^ permalink raw reply
* RE: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
From: Chris Preimesberger @ 2018-09-27 20:17 UTC (permalink / raw)
To: 'Andrew Lunn', 'Eran Ben Elisha'
Cc: 'Neil Horman', 'linville@tuxdriver.com',
'netdev@vger.kernel.org'
In-Reply-To: <82CEAF9FFBA4DD428B132074FB91DF7D5F648BA5@CSI-MAILSRV.csicompanies.internal>
Update for posterity-
Mellanox support provided a work-around of using mlxcables instead of
ethtool to read alarm/warning info for an installed transceiver.
I was told that a couple of their engineers are currently looking into the
discrepancy between threshold reporting by mlxcables and ethtool, and
that they are deciding what to do about it...
Work-around steps:
1. add a cable with "sudo mst cable add".
2. find the cable name with "sudo mlxcables". The name of my cable is
01:00.0_cable_0 so I copy that name for insertion into the next command.
3. probe the cable for DDM with "sudo mlxcables -d 01:00.0_cable_0 --DDM".
Example copied/pasted from my CLI here.
All reported thresholds appear to be correct.
tech1@D7:~$
tech1@D7:~$
tech1@D7:~$ sudo mst cable add
-I- Added 1 cable devices ..
tech1@D7:~$ sudo mlxcables
Querying Cables ....
Cable #1:
---------
Cable name : 01:00.0_cable_0
>> No FW data to show
-------- Cable EEPROM --------
Identifier : QSFP28 (11h)
Technology : 850 nm VCSEL (00h)
Compliance : Extended Specification Compliance is valid, 100GBASE-SR4 or 25GBASE-SR
Wavelength : 850 nm
OUI : 0x00c0f2
Vendor : TRANSITION
Serial number : TN02000263
Part number : TN-QSFP-100G-SR4
Revision : 02
Temperature : 34 C
Length : 50 m
tech1@D7:~$ sudo mlxcables -d 01:00.0_cable_0 --DDM
Cable DDM:
----------
Temperature : 34C
Voltage : 3.2918V
Channel 1:
RX Power : 0.1695dBm
TX Power : 0.8622dBm
TX Bias : 7.0720mA
Channel 2:
RX Power : 0.1355dBm
TX Power : 1.1042dBm
TX Bias : 6.9240mA
Channel 3:
RX Power : -0.1592dBm
TX Power : 0.6547dBm
TX Bias : 6.9420mA
Channel 4:
RX Power : -0.1300dBm
TX Power : 0.4653dBm
TX Bias : 6.9120mA
----- Thresholds -----
Temperature:
High Warning : 70C
Low Warning : 0C
High Alarm : 75C
Low Alarm : -5C
Warning mask : 0
Alarm mask : 0
Voltage:
High Warning : 3.4600V
Low Warning : 3.1300V
High Alarm : 3.6300V
Low Alarm : 2.9700V
Warning mask : 0
Alarm mask : 0
Channel 1:
RX Power high warn : 2.4000dBm
RX Power low warn : -9.5001dBm
RX Power high alarm : 5.4103dBm
RX Power low alarm : -12.5104dBm
RX Power Warning mask: 0
RX Power Alarm mask : 0
TX Power high warn : 2.4000dBm
TX Power low warn : -7.6020dBm
TX Power high alarm : 3.1917dBm
TX Power low alarm : -8.5699dBm
TX Power Warning mask: 0
TX Power Alarm mask : 0
TX Bias high warn : 12.0000mA
TX Bias low warn : 2.0000mA
TX Bias high alarm : 15.0000mA
TX Bias low alarm : 1.0000mA
TX Bias Warning mask : 0
TX Bias Alarm mask : 0
Channel 2:
RX Power high warn : 2.4000dBm
RX Power low warn : -9.5001dBm
RX Power high alarm : 5.4103dBm
RX Power low alarm : -12.5104dBm
RX Power Warning mask: 0
RX Power Alarm mask : 0
TX Power high warn : 2.4000dBm
TX Power low warn : -7.6020dBm
TX Power high alarm : 3.1917dBm
TX Power low alarm : -8.5699dBm
TX Power Warning mask: 0
TX Power Alarm mask : 0
TX Bias high warn : 12.0000mA
TX Bias low warn : 2.0000mA
TX Bias high alarm : 15.0000mA
TX Bias low alarm : 1.0000mA
TX Bias Warning mask : 0
TX Bias Alarm mask : 0
Channel 3:
RX Power high warn : 2.4000dBm
RX Power low warn : -9.5001dBm
RX Power high alarm : 5.4103dBm
RX Power low alarm : -12.5104dBm
RX Power Warning mask: 0
RX Power Alarm mask : 0
TX Power high warn : 2.4000dBm
TX Power low warn : -7.6020dBm
TX Power high alarm : 3.1917dBm
TX Power low alarm : -8.5699dBm
TX Power Warning mask: 0
TX Power Alarm mask : 0
TX Bias high warn : 12.0000mA
TX Bias low warn : 2.0000mA
TX Bias high alarm : 15.0000mA
TX Bias low alarm : 1.0000mA
TX Bias Warning mask : 0
TX Bias Alarm mask : 0
Channel 4:
RX Power high warn : 2.4000dBm
RX Power low warn : -9.5001dBm
RX Power high alarm : 5.4103dBm
RX Power low alarm : -12.5104dBm
RX Power Warning mask: 0
RX Power Alarm mask : 0
TX Power high warn : 2.4000dBm
TX Power low warn : -7.6020dBm
TX Power high alarm : 3.1917dBm
TX Power low alarm : -8.5699dBm
TX Power Warning mask: 0
TX Power Alarm mask : 0
TX Bias high warn : 12.0000mA
TX Bias low warn : 2.0000mA
TX Bias high alarm : 15.0000mA
TX Bias low alarm : 1.0000mA
TX Bias Warning mask : 0
TX Bias Alarm mask : 0
tech1@D7:~$
tech1@D7:~$
tech1@D7:~$
Chris Preimesberger
^ permalink raw reply
* Re: [PATCH net-next 0/7] rtnetlink: add RTM_GETADDR2
From: Christian Brauner @ 2018-09-27 20:31 UTC (permalink / raw)
To: David Ahern, jbenc, davem, stephen, netdev, linux-kernel
In-Reply-To: <e7ec6202-7d4b-8dc1-1b25-76325129ba6f@gmail.com>
On September 27, 2018 10:24:36 PM GMT+02:00, David Ahern <dsahern@gmail.com> wrote:
>On 9/27/18 11:58 AM, Christian Brauner wrote:
>> Various userspace programs (e.g. iproute2) have sent RTM_GETADDR
>> requests with struct ifinfomsg. This is wrong and should have been
>> struct ifaddrmsg all along as mandated by the manpages. However, dump
>> requests so far didn't parse the netlink message that was sent and
>> succeeded even when a wrong struct was passed along.
>
>...
>
>> The correct solution at this point seems to me to introduce a new
>> RTM_GETADDR2 request. This way we can parse the message and fail hard
>if
>> the struct is not struct ifaddrmsg and can safely extend it in the
>> future. Userspace tools that rely on the buggy RTM_GETADDR API will
>> still keep working without even having to see any log messages and
>new
>> userspace tools that want to make user of new features can make use
>of
>> the new RTM_GETADDR2 requests.
>
>First, I think this is the wrong precedent when all we need is a single
>bit flag that userspace can use to tell the kernel "I have a clue and I
>am passing in the proper header for this dump request".
That had been NAKed previously but if you have an idea that will be accepted all the more power to you.
>
>Second, you are not addressing the problems of the past by requiring
>the
>proper header and checking values passed in it.
I don't follow. RTM_GETADDR requests are absolutely unchanged. The full legacy behavior is restored by this patchset.
And requiring that RTM_GETADDR2 requests always pass the correct header is absolutely fine. We don't want built invalid legacy behavior into a new request type.
>
>I have another idea. I'll send an RFC patch soon.
^ permalink raw reply
* [Patch net-next] net_sched: fix an extack message in tcf_block_find()
From: Cong Wang @ 2018-09-27 20:42 UTC (permalink / raw)
To: netdev; +Cc: jiri, jhs, vladbu, Cong Wang
It is clearly a copy-n-paste.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_api.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 3de47e99b788..8dd7f8af6d54 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -655,7 +655,7 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
*q = qdisc_refcount_inc_nz(*q);
if (!*q) {
- NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists");
+ NL_SET_ERR_MSG(extack, "Can't increase Qdisc refcount");
err = -EINVAL;
goto errout_rcu;
}
--
2.14.4
^ permalink raw reply related
* [Patch net-next] net_sched: fix a crash in tc_new_tfilter()
From: Cong Wang @ 2018-09-27 20:42 UTC (permalink / raw)
To: netdev; +Cc: jiri, jhs, vladbu, Cong Wang
In-Reply-To: <20180927204219.17846-1-xiyou.wangcong@gmail.com>
When tcf_block_find() fails, it already rollbacks the qdisc refcnt,
so its caller doesn't need to clean up this again. Avoid calling
qdisc_put() again by resetting qdisc to NULL for callers.
Reported-by: syzbot+37b8770e6d5a8220a039@syzkaller.appspotmail.com
Fixes: e368fdb61d8e ("net: sched: use Qdisc rcu API instead of relying on rtnl lock")
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_api.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 8dd7f8af6d54..a4167ec0a220 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -717,8 +717,10 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
errout_rcu:
rcu_read_unlock();
errout_qdisc:
- if (*q)
+ if (*q) {
qdisc_put(*q);
+ *q = NULL;
+ }
return ERR_PTR(err);
}
--
2.14.4
^ permalink raw reply related
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