* pull-request: can-next 2016-03-10
From: Marc Kleine-Budde @ 2016-03-10 9:33 UTC (permalink / raw)
To: netdev; +Cc: David Miller, kernel@pengutronix.de, linux-can@vger.kernel.org
[-- Attachment #1.1: Type: text/plain, Size: 1807 bytes --]
Hello David,
this is a pull request of 5 patch for net-next/master.
Marek Vasut contributes 4 patches for the ifi CAN driver, which makes
it work on real hardware. There is one patch by Ramesh Shanmugasundaram
for the rcar_can driver that adds support for the 3rd generation IP
core.
Marc
---
The following changes since commit 3b8377dca1fd1974d245b2a04a708fc434761c65:
Merge branch 'variable-length-ll-headers' (2016-03-09 22:13:01 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git tags/linux-can-next-for-4.6-20160310
for you to fetch changes up to e481ab23c57b37c989fa27e0a6b3e941a908775a:
can: rcar_can: Add r8a7795 support (2016-03-10 10:30:21 +0100)
----------------------------------------------------------------
linux-can-next-for-4.6-20160310
----------------------------------------------------------------
Marek Vasut (4):
can: ifi: Fix clock generator configuration
can: ifi: Fix TX DLC configuration
can: ifi: Fix RX and TX ID mask
can: ifi: Add obscure bit swap for EFF frame IDs
Ramesh Shanmugasundaram (1):
can: rcar_can: Add r8a7795 support
.../devicetree/bindings/net/can/rcar_can.txt | 11 +++
drivers/net/can/Kconfig | 2 +-
drivers/net/can/ifi_canfd/ifi_canfd.c | 97 ++++++++++++++--------
drivers/net/can/rcar_can.c | 1 +
4 files changed, 75 insertions(+), 36 deletions(-)
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH nf-next v8 1/8] netfilter: Remove IP_CT_NEW_REPLY definition.
From: Or Gerlitz @ 2016-03-10 9:16 UTC (permalink / raw)
To: Jarno Rajahalme; +Cc: netfilter-devel, Linux Netdev List, dev@openvswitch.org
In-Reply-To: <1457483084-13944-1-git-send-email-jarno@ovn.org>
On Wed, Mar 9, 2016 at 2:24 AM, Jarno Rajahalme <jarno@ovn.org> wrote:
> Remove the definition of IP_CT_NEW_REPLY from the kernel as it does
> not make sense. This allows the definition of IP_CT_NUMBER to be
> simplified as well.
I just realized that after V7 you stopped sending cover letter (patch 0/N)
with this series.
Maybe you send it and this misses the list? we need to be able to see
differences from earlier versions.
Or.
^ permalink raw reply
* Re: [PATCH v3 0/8] arm64: rockchip: Initial GeekBox enablement
From: Giuseppe CAVALLARO @ 2016-03-10 9:13 UTC (permalink / raw)
To: Dinh Nguyen
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Heiko Stübner, netdev-u79uwXL29TY76Z2rM5mHXA,
Gabriel Fernandez, LKML, Frank Schäfer,
open list:ARM/Rockchip SoC..., LAKML, Fabrice GASNIER,
Andreas Färber, Tomeu Vizoso, Alexandre TORGUE
In-Reply-To: <CADhT+wcMpzA_Coj8Z5OQZsc3bF3a7yOmsbazQMPtcdUWkJSuCA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 3/9/2016 5:31 PM, Dinh Nguyen wrote:
> On Wed, Mar 9, 2016 at 8:53 AM, Giuseppe CAVALLARO
> <peppe.cavallaro-qxv4g6HH51o@public.gmane.org> wrote:
>> Hi Tomeu, Dinh, Andreas
>>
>> I need a sum and help from you to go ahead on the
>> tx timeout.
>>
>> The "stmmac: MDIO fixes" seems to be the candidate to
>> fix the phy connection and I will send the V2 asap (Andreas' comment).
>>
>> So, supposing the probe is ok and phy is connected,
>> I need your input ...
>>
>> Tomeu: after revering the 0e80bdc9a72d (stmmac: first frame
>> prep at the end of xmit routine) the network is
>> not stable and there is a timeout after a while.
>> The box has 3.50 with normal desc settings.
>>
>> Dinh: the network is ok, I wonder if you can share a boot
>> log just to understand if the normal or enhanced
>> descriptors are used.
>>
>
> Here it is:
...
> [ 0.850523] stmmac - user ID: 0x10, Synopsys ID: 0x37
> [ 0.855570] Ring mode enabled
> [ 0.858611] DMA HW capability register supported
> [ 0.863128] Enhanced/Alternate descriptors
> [ 0.867482] Enabled extended descriptors
> [ 0.871482] RX Checksum Offload Engine supported (type 2)
> [ 0.876948] TX Checksum insertion supported
> [ 0.881204] Enable RX Mitigation via HW Watchdog Timer
> [ 0.886863] socfpga-dwmac ff702000.ethernet eth0: No MDIO subnode found
> [ 0.899090] libphy: stmmac: probed
> [ 0.902484] eth0: PHY ID 00221611 at 4 IRQ POLL (stmmac-0:04) active
Thx Dinh, so you are using the Enhanced/Alternate descriptors
I am debugging on my side on a setup with normal descriptors, I let you
know
peppe
^ permalink raw reply
* Re: [PATCH] bpf: make bpf_stackmap_copy conditionally called
From: Arnd Bergmann @ 2016-03-10 9:11 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, David S. Miller, Daniel Borkmann, netdev,
linux-kernel
In-Reply-To: <20160310033845.GA62491@ast-mbp.thefacebook.com>
On Thursday 10 March 2016, Alexei Starovoitov wrote:
> yes. this is also ok-ish fix.
> I've sent different version already:
> http://patchwork.ozlabs.org/patch/595617/
>
> I considered the option like yours but it's relying on gcc doing
> dead code elimination of 'if (false) {}' branch and though kernel
> is never compiled with -O0. I didn't want to take the risk.
> I'm fine with either approach though.
Ok, and I see yours is already applied, so that's fine.
In general, I don't like __weak symbols in the kernel as they
make it less clear what is actually getting called, and I think my
version would have been safe, we rely on building with -O2 or -Os
in a lot of places because of similar things.
Arnd
^ permalink raw reply
* [net-next] netconf: add macro to represent all attributes
From: Zhang Shengju @ 2016-03-10 8:55 UTC (permalink / raw)
To: davem, netdev
This patch adds macro NETCONFA_ALL to represent all type of netconf
attributes for IPv4 and IPv6.
Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
---
include/uapi/linux/netconf.h | 1 +
net/ipv4/devinet.c | 40 +++++++++++++++++++++++-----------------
net/ipv6/addrconf.c | 36 +++++++++++++++++++++---------------
3 files changed, 45 insertions(+), 32 deletions(-)
diff --git a/include/uapi/linux/netconf.h b/include/uapi/linux/netconf.h
index 23cbd34..45dfad5 100644
--- a/include/uapi/linux/netconf.h
+++ b/include/uapi/linux/netconf.h
@@ -19,6 +19,7 @@ enum {
__NETCONFA_MAX
};
#define NETCONFA_MAX (__NETCONFA_MAX - 1)
+#define NETCONFA_ALL -1
#define NETCONFA_IFINDEX_ALL -1
#define NETCONFA_IFINDEX_DEFAULT -2
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 8c3df2c..65e76a4 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1753,17 +1753,20 @@ static int inet_netconf_msgsize_devconf(int type)
{
int size = NLMSG_ALIGN(sizeof(struct netconfmsg))
+ nla_total_size(4); /* NETCONFA_IFINDEX */
+ bool all = false;
- /* type -1 is used for ALL */
- if (type == -1 || type == NETCONFA_FORWARDING)
+ if (type == NETCONFA_ALL)
+ all = true;
+
+ if (all || type == NETCONFA_FORWARDING)
size += nla_total_size(4);
- if (type == -1 || type == NETCONFA_RP_FILTER)
+ if (all || type == NETCONFA_RP_FILTER)
size += nla_total_size(4);
- if (type == -1 || type == NETCONFA_MC_FORWARDING)
+ if (all || type == NETCONFA_MC_FORWARDING)
size += nla_total_size(4);
- if (type == -1 || type == NETCONFA_PROXY_NEIGH)
+ if (all || type == NETCONFA_PROXY_NEIGH)
size += nla_total_size(4);
- if (type == -1 || type == NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN)
+ if (all || type == NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN)
size += nla_total_size(4);
return size;
@@ -1776,36 +1779,39 @@ static int inet_netconf_fill_devconf(struct sk_buff *skb, int ifindex,
{
struct nlmsghdr *nlh;
struct netconfmsg *ncm;
+ bool all = false;
nlh = nlmsg_put(skb, portid, seq, event, sizeof(struct netconfmsg),
flags);
if (!nlh)
return -EMSGSIZE;
+ if (type == NETCONFA_ALL)
+ all = true;
+
ncm = nlmsg_data(nlh);
ncm->ncm_family = AF_INET;
if (nla_put_s32(skb, NETCONFA_IFINDEX, ifindex) < 0)
goto nla_put_failure;
- /* type -1 is used for ALL */
- if ((type == -1 || type == NETCONFA_FORWARDING) &&
+ if ((all || type == NETCONFA_FORWARDING) &&
nla_put_s32(skb, NETCONFA_FORWARDING,
IPV4_DEVCONF(*devconf, FORWARDING)) < 0)
goto nla_put_failure;
- if ((type == -1 || type == NETCONFA_RP_FILTER) &&
+ if ((all || type == NETCONFA_RP_FILTER) &&
nla_put_s32(skb, NETCONFA_RP_FILTER,
IPV4_DEVCONF(*devconf, RP_FILTER)) < 0)
goto nla_put_failure;
- if ((type == -1 || type == NETCONFA_MC_FORWARDING) &&
+ if ((all || type == NETCONFA_MC_FORWARDING) &&
nla_put_s32(skb, NETCONFA_MC_FORWARDING,
IPV4_DEVCONF(*devconf, MC_FORWARDING)) < 0)
goto nla_put_failure;
- if ((type == -1 || type == NETCONFA_PROXY_NEIGH) &&
+ if ((all || type == NETCONFA_PROXY_NEIGH) &&
nla_put_s32(skb, NETCONFA_PROXY_NEIGH,
IPV4_DEVCONF(*devconf, PROXY_ARP)) < 0)
goto nla_put_failure;
- if ((type == -1 || type == NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN) &&
+ if ((all || type == NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN) &&
nla_put_s32(skb, NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN,
IPV4_DEVCONF(*devconf, IGNORE_ROUTES_WITH_LINKDOWN)) < 0)
goto nla_put_failure;
@@ -1893,14 +1899,14 @@ static int inet_netconf_get_devconf(struct sk_buff *in_skb,
}
err = -ENOBUFS;
- skb = nlmsg_new(inet_netconf_msgsize_devconf(-1), GFP_ATOMIC);
+ skb = nlmsg_new(inet_netconf_msgsize_devconf(NETCONFA_ALL), GFP_ATOMIC);
if (!skb)
goto errout;
err = inet_netconf_fill_devconf(skb, ifindex, devconf,
NETLINK_CB(in_skb).portid,
nlh->nlmsg_seq, RTM_NEWNETCONF, 0,
- -1);
+ NETCONFA_ALL);
if (err < 0) {
/* -EMSGSIZE implies BUG in inet_netconf_msgsize_devconf() */
WARN_ON(err == -EMSGSIZE);
@@ -1944,7 +1950,7 @@ static int inet_netconf_dump_devconf(struct sk_buff *skb,
cb->nlh->nlmsg_seq,
RTM_NEWNETCONF,
NLM_F_MULTI,
- -1) < 0) {
+ NETCONFA_ALL) < 0) {
rcu_read_unlock();
goto done;
}
@@ -1960,7 +1966,7 @@ cont:
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
RTM_NEWNETCONF, NLM_F_MULTI,
- -1) < 0)
+ NETCONFA_ALL) < 0)
goto done;
else
h++;
@@ -1971,7 +1977,7 @@ cont:
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
RTM_NEWNETCONF, NLM_F_MULTI,
- -1) < 0)
+ NETCONFA_ALL) < 0)
goto done;
else
h++;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 8c0dab2..27aed1a 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -473,18 +473,21 @@ static int inet6_netconf_msgsize_devconf(int type)
{
int size = NLMSG_ALIGN(sizeof(struct netconfmsg))
+ nla_total_size(4); /* NETCONFA_IFINDEX */
+ bool all = false;
- /* type -1 is used for ALL */
- if (type == -1 || type == NETCONFA_FORWARDING)
+ if (type == NETCONFA_ALL)
+ all = true;
+
+ if (all || type == NETCONFA_FORWARDING)
size += nla_total_size(4);
#ifdef CONFIG_IPV6_MROUTE
- if (type == -1 || type == NETCONFA_MC_FORWARDING)
+ if (all || type == NETCONFA_MC_FORWARDING)
size += nla_total_size(4);
#endif
- if (type == -1 || type == NETCONFA_PROXY_NEIGH)
+ if (all || type == NETCONFA_PROXY_NEIGH)
size += nla_total_size(4);
- if (type == -1 || type == NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN)
+ if (all || type == NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN)
size += nla_total_size(4);
return size;
@@ -497,33 +500,36 @@ static int inet6_netconf_fill_devconf(struct sk_buff *skb, int ifindex,
{
struct nlmsghdr *nlh;
struct netconfmsg *ncm;
+ bool all = false;
nlh = nlmsg_put(skb, portid, seq, event, sizeof(struct netconfmsg),
flags);
if (!nlh)
return -EMSGSIZE;
+ if (type == NETCONFA_ALL)
+ all = true;
+
ncm = nlmsg_data(nlh);
ncm->ncm_family = AF_INET6;
if (nla_put_s32(skb, NETCONFA_IFINDEX, ifindex) < 0)
goto nla_put_failure;
- /* type -1 is used for ALL */
- if ((type == -1 || type == NETCONFA_FORWARDING) &&
+ if ((all || type == NETCONFA_FORWARDING) &&
nla_put_s32(skb, NETCONFA_FORWARDING, devconf->forwarding) < 0)
goto nla_put_failure;
#ifdef CONFIG_IPV6_MROUTE
- if ((type == -1 || type == NETCONFA_MC_FORWARDING) &&
+ if ((all || type == NETCONFA_MC_FORWARDING) &&
nla_put_s32(skb, NETCONFA_MC_FORWARDING,
devconf->mc_forwarding) < 0)
goto nla_put_failure;
#endif
- if ((type == -1 || type == NETCONFA_PROXY_NEIGH) &&
+ if ((all || type == NETCONFA_PROXY_NEIGH) &&
nla_put_s32(skb, NETCONFA_PROXY_NEIGH, devconf->proxy_ndp) < 0)
goto nla_put_failure;
- if ((type == -1 || type == NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN) &&
+ if ((all || type == NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN) &&
nla_put_s32(skb, NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN,
devconf->ignore_routes_with_linkdown) < 0)
goto nla_put_failure;
@@ -609,14 +615,14 @@ static int inet6_netconf_get_devconf(struct sk_buff *in_skb,
}
err = -ENOBUFS;
- skb = nlmsg_new(inet6_netconf_msgsize_devconf(-1), GFP_ATOMIC);
+ skb = nlmsg_new(inet6_netconf_msgsize_devconf(NETCONFA_ALL), GFP_ATOMIC);
if (!skb)
goto errout;
err = inet6_netconf_fill_devconf(skb, ifindex, devconf,
NETLINK_CB(in_skb).portid,
nlh->nlmsg_seq, RTM_NEWNETCONF, 0,
- -1);
+ NETCONFA_ALL);
if (err < 0) {
/* -EMSGSIZE implies BUG in inet6_netconf_msgsize_devconf() */
WARN_ON(err == -EMSGSIZE);
@@ -660,7 +666,7 @@ static int inet6_netconf_dump_devconf(struct sk_buff *skb,
cb->nlh->nlmsg_seq,
RTM_NEWNETCONF,
NLM_F_MULTI,
- -1) < 0) {
+ NETCONFA_ALL) < 0) {
rcu_read_unlock();
goto done;
}
@@ -676,7 +682,7 @@ cont:
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
RTM_NEWNETCONF, NLM_F_MULTI,
- -1) < 0)
+ NETCONFA_ALL) < 0)
goto done;
else
h++;
@@ -687,7 +693,7 @@ cont:
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
RTM_NEWNETCONF, NLM_F_MULTI,
- -1) < 0)
+ NETCONFA_ALL) < 0)
goto done;
else
h++;
--
1.8.3.1
^ permalink raw reply related
* Re: [patch net 2/2] net: hns: fixes a bug of RSS
From: Andy Shevchenko @ 2016-03-10 8:24 UTC (permalink / raw)
To: Kejian Yan, davem
Cc: yisen.zhuang, salil.mehta, liguozhu, huangdaode, arnd, andrew,
chenny.xu, ivecera, lisheng011, fengguang.wu, haifeng.wei, netdev,
linux-kernel, linux-arm-kernel, linuxarm
In-Reply-To: <1457576189-22924-3-git-send-email-yankejian@huawei.com>
On Thu, 2016-03-10 at 10:16 +0800, Kejian Yan wrote:
> If trying to get receive flow hash indirection table by ethtool, it
> needs
> to call .get_rxnfc to get ring number first. So this patch implements
> the
> .get_rxnfc of ethtool. And the data type of rss_indir_table is u32,
> it has
> to be multiply by the width of data type when using memcpy.
+static int hns_get_rxnfc(struct net_device *netdev,
> + struct ethtool_rxnfc *cmd,
> + u32 *rule_locs)
> +{
> + struct hns_nic_priv *priv = netdev_priv(netdev);
> + int ret = 0;
> +
> + switch (cmd->cmd) {
> + case ETHTOOL_GRXRINGS:
> + cmd->data = priv->ae_handle->q_num;
> + break;
> + default:
> + ret = -EOPNOTSUPP;
> + break;
> + }
> +
> + return ret;
Redundant ret variable.
switch (value) {
case X:
break;
default:
return -ERRNO;
}
return 0;
> +}
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [patch net 1/2] net: hns: fix return value of the function about rss
From: Andy Shevchenko @ 2016-03-10 8:11 UTC (permalink / raw)
To: Kejian Yan, davem
Cc: yisen.zhuang, salil.mehta, liguozhu, huangdaode, arnd, andrew,
chenny.xu, ivecera, lisheng011, fengguang.wu, haifeng.wei, netdev,
linux-kernel, linux-arm-kernel, linuxarm
In-Reply-To: <1457576189-22924-2-git-send-email-yankejian@huawei.com>
On Thu, 2016-03-10 at 10:16 +0800, Kejian Yan wrote:
> Both .get_rxfh and .get_rxfh are always return 0, it should return
> result
> from hardware when getting or setting rss. And the rss function
> should
> return the correct data type.
>
@@ -1213,7 +1213,7 @@ hns_get_rss(struct net_device *netdev, u32
> *indir, u8 *key, u8 *hfunc)
>
>
> ret = ops->get_rss(priv->ae_handle, indir, key, hfunc);
>
> - return 0;
> + return ret;
All three can be one line.
> @@ -1241,7 +1241,7 @@ hns_set_rss(struct net_device *netdev, const
> u32 *indir, const u8 *key,
>
> ret = ops->set_rss(priv->ae_handle, indir, key, hfunc);
>
> - return 0;
> + return ret;
Ditto.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* [PATCH net] sctp: fix the transports round robin issue when init is retransmitted
From: Xin Long @ 2016-03-10 7:31 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Vlad Yasevich, daniel
prior to this patch, at the beginning if we have two paths in one assoc,
they may have the same params other than the last_time_heard, it will try
the paths like this:
1st cycle
try trans1 fail.
then trans2 is selected.(cause it's last_time_heard is after trans1).
2nd cycle:
try trans2 fail
then trans2 is selected.(cause it's last_time_heard is after trans1).
3rd cycle:
try trans2 fail
then trans2 is selected.(cause it's last_time_heard is after trans1).
....
trans1 will never have change to be selected, which is not what we expect.
we should keeping round robin all the paths if they are just added at the
beginning.
So at first every tranport's last_time_heard should be initialized 0, so
that we ensure they have the same value at the beginning, only by this,
all the transports could get equal chance to be selected.
Then for sctp_trans_elect_best, it should return the trans_next one when
*trans == *trans_next, so that we can try next if it fails, but now it
always return trans. so we can fix it by exchanging these two params when
we calls sctp_trans_elect_tie().
Fixes: 4c47af4d5eb2 ('net: sctp: rework multihoming retransmission path selection to rfc4960')
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/associola.c | 2 +-
net/sctp/transport.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 2bf8ec9..cd87344 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1263,7 +1263,7 @@ static struct sctp_transport *sctp_trans_elect_best(struct sctp_transport *curr,
if (score_curr > score_best)
return curr;
else if (score_curr == score_best)
- return sctp_trans_elect_tie(curr, best);
+ return sctp_trans_elect_tie(best, curr);
else
return best;
}
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index a431c14..d517153 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -72,7 +72,7 @@ static struct sctp_transport *sctp_transport_init(struct net *net,
*/
peer->rto = msecs_to_jiffies(net->sctp.rto_initial);
- peer->last_time_heard = ktime_get();
+ peer->last_time_heard = ktime_set(0, 0);
peer->last_time_ecne_reduced = jiffies;
peer->param_flags = SPP_HB_DISABLE |
--
2.1.0
^ permalink raw reply related
* Re: [PATCH V4 0/3] basic busy polling support for vhost_net
From: Michael Rapoport @ 2016-03-10 6:48 UTC (permalink / raw)
To: Greg Kurz
Cc: borntraeger, Jason Wang, kvm, linux-kernel, mst, netdev,
virtualization, yang.zhang.wz
In-Reply-To: <20160309202645.030ad7b2@bahia.lab.toulouse-stg.fr.ibm.com>
Hi Greg,
> Greg Kurz <gkurz@linux.vnet.ibm.com> wrote on 03/09/2016 09:26:45 PM:
> > On Fri, 4 Mar 2016 06:24:50 -0500
> > Jason Wang <jasowang@redhat.com> wrote:
>
> > This series tries to add basic busy polling for vhost net. The idea is
> > simple: at the end of tx/rx processing, busy polling for new tx added
> > descriptor and rx receive socket for a while. The maximum number of
> > time (in us) could be spent on busy polling was specified ioctl.
> >
> > Test A were done through:
> >
> > - 50 us as busy loop timeout
> > - Netperf 2.6
> > - Two machines with back to back connected mlx4
>
> Hi Jason,
>
> Could this also improve performance if both VMs are
> on the same host system ?
I've experimented a little with Jason's patches and guest-to-guest netperf
when both guests were on the same host, and I saw improvements for that
case.
> > - Guest with 1 vcpus and 1 queue
> >
> > Results:
> > - Obvious improvements (%5 - 20%) for latency (TCP_RR).
> > - Get a better or minor regression on most of the TX tests, but see
> > some regression on 4096 size.
> > - Except for 8 sessions of 4096 size RX, have a better or same
> > performance.
> > - CPU utilization were incrased as expected.
> >
> > TCP_RR:
> > size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
> > 1/ 1/ +8%/ -32%/ +8%/ +8%/ +7%
> > 1/ 50/ +7%/ -19%/ +7%/ +7%/ +1%
> > 1/ 100/ +5%/ -21%/ +5%/ +5%/ 0%
> > 1/ 200/ +5%/ -21%/ +7%/ +7%/ +1%
> > 64/ 1/ +11%/ -29%/ +11%/ +11%/ +10%
> > 64/ 50/ +7%/ -19%/ +8%/ +8%/ +2%
> > 64/ 100/ +8%/ -18%/ +9%/ +9%/ +2%
> > 64/ 200/ +6%/ -19%/ +6%/ +6%/ 0%
> > 256/ 1/ +7%/ -33%/ +7%/ +7%/ +6%
> > 256/ 50/ +7%/ -18%/ +7%/ +7%/ 0%
> > 256/ 100/ +9%/ -18%/ +8%/ +8%/ +2%
> > 256/ 200/ +9%/ -18%/ +10%/ +10%/ +3%
> > 1024/ 1/ +20%/ -28%/ +20%/ +20%/ +19%
> > 1024/ 50/ +8%/ -18%/ +9%/ +9%/ +2%
> > 1024/ 100/ +6%/ -19%/ +5%/ +5%/ 0%
> > 1024/ 200/ +8%/ -18%/ +9%/ +9%/ +2%
> > Guest TX:
> > size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
> > 64/ 1/ -5%/ -28%/ +11%/ +12%/ +10%
> > 64/ 4/ -2%/ -26%/ +13%/ +13%/ +13%
> > 64/ 8/ -6%/ -29%/ +9%/ +10%/ +10%
> > 512/ 1/ +15%/ -7%/ +13%/ +11%/ +3%
> > 512/ 4/ +17%/ -6%/ +18%/ +13%/ +11%
> > 512/ 8/ +14%/ -7%/ +13%/ +7%/ +7%
> > 1024/ 1/ +27%/ -2%/ +26%/ +29%/ +12%
> > 1024/ 4/ +8%/ -9%/ +6%/ +1%/ +6%
> > 1024/ 8/ +41%/ +12%/ +34%/ +20%/ -3%
> > 4096/ 1/ -22%/ -21%/ -36%/ +81%/+1360%
> > 4096/ 4/ -57%/ -58%/ +286%/ +15%/+2074%
> > 4096/ 8/ +67%/ +70%/ -45%/ -8%/ +63%
> > 16384/ 1/ -2%/ -5%/ +5%/ -3%/ +80%
> > 16384/ 4/ 0%/ 0%/ 0%/ +4%/ +138%
> > 16384/ 8/ 0%/ 0%/ 0%/ +1%/ +41%
> > 65535/ 1/ -3%/ -6%/ +2%/ +11%/ +113%
> > 65535/ 4/ -2%/ -1%/ -2%/ -3%/ +484%
> > 65535/ 8/ 0%/ +1%/ 0%/ +2%/ +40%
> > Guest RX:
> > size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
> > 64/ 1/ +31%/ -3%/ +8%/ +8%/ +8%
> > 64/ 4/ +11%/ -17%/ +13%/ +14%/ +15%
> > 64/ 8/ +4%/ -23%/ +11%/ +11%/ +12%
> > 512/ 1/ +24%/ 0%/ +18%/ +14%/ -8%
> > 512/ 4/ +4%/ -15%/ +6%/ +5%/ +6%
> > 512/ 8/ +26%/ 0%/ +21%/ +10%/ +3%
> > 1024/ 1/ +88%/ +47%/ +69%/ +44%/ -30%
> > 1024/ 4/ +18%/ -5%/ +19%/ +16%/ +2%
> > 1024/ 8/ +15%/ -4%/ +13%/ +8%/ +1%
> > 4096/ 1/ -3%/ -5%/ +2%/ -2%/ +41%
> > 4096/ 4/ +2%/ +3%/ -20%/ -14%/ -24%
> > 4096/ 8/ -43%/ -45%/ +69%/ -24%/ +94%
> > 16384/ 1/ -3%/ -11%/ +23%/ +7%/ +42%
> > 16384/ 4/ -3%/ -3%/ -4%/ +5%/ +115%
> > 16384/ 8/ -1%/ 0%/ -1%/ -3%/ +32%
> > 65535/ 1/ +1%/ 0%/ +2%/ 0%/ +66%
> > 65535/ 4/ -1%/ -1%/ 0%/ +4%/ +492%
> > 65535/ 8/ 0%/ -1%/ -1%/ +4%/ +38%
> >
> > Changes from V3:
> > - drop single_task_running()
> > - use cpu_relax_lowlatency() instead of cpu_relax()
> >
> > Changes from V2:
> > - rename vhost_vq_more_avail() to vhost_vq_avail_empty(). And return
> > false we __get_user() fails.
> > - do not bother premmptions/timers for good path.
> > - use vhost_vring_state as ioctl parameter instead of reinveting a new
> > one.
> > - add the unit of timeout (us) to the comment of new added ioctls
> >
> > Changes from V1:
> > - remove the buggy vq_error() in vhost_vq_more_avail().
> > - leave vhost_enable_notify() untouched.
> >
> > Changes from RFC V3:
> > - small tweak on the code to avoid multiple duplicate conditions in
> > critical path when busy loop is not enabled.
> > - add the test result of multiple VMs
> >
> > Changes from RFC V2:
> > - poll also at the end of rx handling
> > - factor out the polling logic and optimize the code a little bit
> > - add two ioctls to get and set the busy poll timeout
> > - test on ixgbe (which can give more stable and reproducable numbers)
> > instead of mlx4.
> >
> > Changes from RFC V1:
> > - add a comment for vhost_has_work() to explain why it could be
> > lockless
> > - add param description for busyloop_timeout
> > - split out the busy polling logic into a new helper
> > - check and exit the loop when there's a pending signal
> > - disable preemption during busy looping to make sure lock_clock() was
> > correctly used.
> >
> > Jason Wang (3):
> > vhost: introduce vhost_has_work()
> > vhost: introduce vhost_vq_avail_empty()
> > vhost_net: basic polling support
> >
> > drivers/vhost/net.c | 78
+++++++++++++++++++++++++++++++++++++++++++---
> > drivers/vhost/vhost.c | 35 +++++++++++++++++++++
> > drivers/vhost/vhost.h | 3 ++
> > include/uapi/linux/vhost.h | 6 ++++
> > 4 files changed, 117 insertions(+), 5 deletions(-)
> >
>
^ permalink raw reply
* Re: [PATCH V4 0/3] basic busy polling support for vhost_net
From: Michael Rapoport @ 2016-03-10 6:48 UTC (permalink / raw)
To: Greg Kurz
Cc: borntraeger, Jason Wang, kvm, linux-kernel, mst, netdev,
virtualization, yang.zhang.wz
In-Reply-To: <20160309202645.030ad7b2@bahia.lab.toulouse-stg.fr.ibm.com>
Hi Greg,
> Greg Kurz <gkurz@linux.vnet.ibm.com> wrote on 03/09/2016 09:26:45 PM:
> > On Fri, 4 Mar 2016 06:24:50 -0500
> > Jason Wang <jasowang@redhat.com> wrote:
>
> > This series tries to add basic busy polling for vhost net. The idea is
> > simple: at the end of tx/rx processing, busy polling for new tx added
> > descriptor and rx receive socket for a while. The maximum number of
> > time (in us) could be spent on busy polling was specified ioctl.
> >
> > Test A were done through:
> >
> > - 50 us as busy loop timeout
> > - Netperf 2.6
> > - Two machines with back to back connected mlx4
>
> Hi Jason,
>
> Could this also improve performance if both VMs are
> on the same host system ?
I've experimented a little with Jason's patches and guest-to-guest netperf
when both guests were on the same host, and I saw improvements for that
case.
> > - Guest with 1 vcpus and 1 queue
> >
> > Results:
> > - Obvious improvements (%5 - 20%) for latency (TCP_RR).
> > - Get a better or minor regression on most of the TX tests, but see
> > some regression on 4096 size.
> > - Except for 8 sessions of 4096 size RX, have a better or same
> > performance.
> > - CPU utilization were incrased as expected.
> >
> > TCP_RR:
> > size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
> > 1/ 1/ +8%/ -32%/ +8%/ +8%/ +7%
> > 1/ 50/ +7%/ -19%/ +7%/ +7%/ +1%
> > 1/ 100/ +5%/ -21%/ +5%/ +5%/ 0%
> > 1/ 200/ +5%/ -21%/ +7%/ +7%/ +1%
> > 64/ 1/ +11%/ -29%/ +11%/ +11%/ +10%
> > 64/ 50/ +7%/ -19%/ +8%/ +8%/ +2%
> > 64/ 100/ +8%/ -18%/ +9%/ +9%/ +2%
> > 64/ 200/ +6%/ -19%/ +6%/ +6%/ 0%
> > 256/ 1/ +7%/ -33%/ +7%/ +7%/ +6%
> > 256/ 50/ +7%/ -18%/ +7%/ +7%/ 0%
> > 256/ 100/ +9%/ -18%/ +8%/ +8%/ +2%
> > 256/ 200/ +9%/ -18%/ +10%/ +10%/ +3%
> > 1024/ 1/ +20%/ -28%/ +20%/ +20%/ +19%
> > 1024/ 50/ +8%/ -18%/ +9%/ +9%/ +2%
> > 1024/ 100/ +6%/ -19%/ +5%/ +5%/ 0%
> > 1024/ 200/ +8%/ -18%/ +9%/ +9%/ +2%
> > Guest TX:
> > size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
> > 64/ 1/ -5%/ -28%/ +11%/ +12%/ +10%
> > 64/ 4/ -2%/ -26%/ +13%/ +13%/ +13%
> > 64/ 8/ -6%/ -29%/ +9%/ +10%/ +10%
> > 512/ 1/ +15%/ -7%/ +13%/ +11%/ +3%
> > 512/ 4/ +17%/ -6%/ +18%/ +13%/ +11%
> > 512/ 8/ +14%/ -7%/ +13%/ +7%/ +7%
> > 1024/ 1/ +27%/ -2%/ +26%/ +29%/ +12%
> > 1024/ 4/ +8%/ -9%/ +6%/ +1%/ +6%
> > 1024/ 8/ +41%/ +12%/ +34%/ +20%/ -3%
> > 4096/ 1/ -22%/ -21%/ -36%/ +81%/+1360%
> > 4096/ 4/ -57%/ -58%/ +286%/ +15%/+2074%
> > 4096/ 8/ +67%/ +70%/ -45%/ -8%/ +63%
> > 16384/ 1/ -2%/ -5%/ +5%/ -3%/ +80%
> > 16384/ 4/ 0%/ 0%/ 0%/ +4%/ +138%
> > 16384/ 8/ 0%/ 0%/ 0%/ +1%/ +41%
> > 65535/ 1/ -3%/ -6%/ +2%/ +11%/ +113%
> > 65535/ 4/ -2%/ -1%/ -2%/ -3%/ +484%
> > 65535/ 8/ 0%/ +1%/ 0%/ +2%/ +40%
> > Guest RX:
> > size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
> > 64/ 1/ +31%/ -3%/ +8%/ +8%/ +8%
> > 64/ 4/ +11%/ -17%/ +13%/ +14%/ +15%
> > 64/ 8/ +4%/ -23%/ +11%/ +11%/ +12%
> > 512/ 1/ +24%/ 0%/ +18%/ +14%/ -8%
> > 512/ 4/ +4%/ -15%/ +6%/ +5%/ +6%
> > 512/ 8/ +26%/ 0%/ +21%/ +10%/ +3%
> > 1024/ 1/ +88%/ +47%/ +69%/ +44%/ -30%
> > 1024/ 4/ +18%/ -5%/ +19%/ +16%/ +2%
> > 1024/ 8/ +15%/ -4%/ +13%/ +8%/ +1%
> > 4096/ 1/ -3%/ -5%/ +2%/ -2%/ +41%
> > 4096/ 4/ +2%/ +3%/ -20%/ -14%/ -24%
> > 4096/ 8/ -43%/ -45%/ +69%/ -24%/ +94%
> > 16384/ 1/ -3%/ -11%/ +23%/ +7%/ +42%
> > 16384/ 4/ -3%/ -3%/ -4%/ +5%/ +115%
> > 16384/ 8/ -1%/ 0%/ -1%/ -3%/ +32%
> > 65535/ 1/ +1%/ 0%/ +2%/ 0%/ +66%
> > 65535/ 4/ -1%/ -1%/ 0%/ +4%/ +492%
> > 65535/ 8/ 0%/ -1%/ -1%/ +4%/ +38%
> >
> > Changes from V3:
> > - drop single_task_running()
> > - use cpu_relax_lowlatency() instead of cpu_relax()
> >
> > Changes from V2:
> > - rename vhost_vq_more_avail() to vhost_vq_avail_empty(). And return
> > false we __get_user() fails.
> > - do not bother premmptions/timers for good path.
> > - use vhost_vring_state as ioctl parameter instead of reinveting a new
> > one.
> > - add the unit of timeout (us) to the comment of new added ioctls
> >
> > Changes from V1:
> > - remove the buggy vq_error() in vhost_vq_more_avail().
> > - leave vhost_enable_notify() untouched.
> >
> > Changes from RFC V3:
> > - small tweak on the code to avoid multiple duplicate conditions in
> > critical path when busy loop is not enabled.
> > - add the test result of multiple VMs
> >
> > Changes from RFC V2:
> > - poll also at the end of rx handling
> > - factor out the polling logic and optimize the code a little bit
> > - add two ioctls to get and set the busy poll timeout
> > - test on ixgbe (which can give more stable and reproducable numbers)
> > instead of mlx4.
> >
> > Changes from RFC V1:
> > - add a comment for vhost_has_work() to explain why it could be
> > lockless
> > - add param description for busyloop_timeout
> > - split out the busy polling logic into a new helper
> > - check and exit the loop when there's a pending signal
> > - disable preemption during busy looping to make sure lock_clock() was
> > correctly used.
> >
> > Jason Wang (3):
> > vhost: introduce vhost_has_work()
> > vhost: introduce vhost_vq_avail_empty()
> > vhost_net: basic polling support
> >
> > drivers/vhost/net.c | 78
+++++++++++++++++++++++++++++++++++++++++++---
> > drivers/vhost/vhost.c | 35 +++++++++++++++++++++
> > drivers/vhost/vhost.h | 3 ++
> > include/uapi/linux/vhost.h | 6 ++++
> > 4 files changed, 117 insertions(+), 5 deletions(-)
> >
>
^ permalink raw reply
* Re: [PATCH V4 0/3] basic busy polling support for vhost_net
From: Michael Rapoport @ 2016-03-10 6:48 UTC (permalink / raw)
To: Greg Kurz
Cc: borntraeger, Jason Wang, kvm, linux-kernel, mst, netdev,
virtualization, yang.zhang.wz
In-Reply-To: <20160309202645.030ad7b2@bahia.lab.toulouse-stg.fr.ibm.com>
Hi Greg,
> Greg Kurz <gkurz@linux.vnet.ibm.com> wrote on 03/09/2016 09:26:45 PM:
> > On Fri, 4 Mar 2016 06:24:50 -0500
> > Jason Wang <jasowang@redhat.com> wrote:
>
> > This series tries to add basic busy polling for vhost net. The idea is
> > simple: at the end of tx/rx processing, busy polling for new tx added
> > descriptor and rx receive socket for a while. The maximum number of
> > time (in us) could be spent on busy polling was specified ioctl.
> >
> > Test A were done through:
> >
> > - 50 us as busy loop timeout
> > - Netperf 2.6
> > - Two machines with back to back connected mlx4
>
> Hi Jason,
>
> Could this also improve performance if both VMs are
> on the same host system ?
I've experimented a little with Jason's patches and guest-to-guest netperf
when both guests were on the same host, and I saw improvements for that
case.
> > - Guest with 1 vcpus and 1 queue
> >
> > Results:
> > - Obvious improvements (%5 - 20%) for latency (TCP_RR).
> > - Get a better or minor regression on most of the TX tests, but see
> > some regression on 4096 size.
> > - Except for 8 sessions of 4096 size RX, have a better or same
> > performance.
> > - CPU utilization were incrased as expected.
> >
> > TCP_RR:
> > size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
> > 1/ 1/ +8%/ -32%/ +8%/ +8%/ +7%
> > 1/ 50/ +7%/ -19%/ +7%/ +7%/ +1%
> > 1/ 100/ +5%/ -21%/ +5%/ +5%/ 0%
> > 1/ 200/ +5%/ -21%/ +7%/ +7%/ +1%
> > 64/ 1/ +11%/ -29%/ +11%/ +11%/ +10%
> > 64/ 50/ +7%/ -19%/ +8%/ +8%/ +2%
> > 64/ 100/ +8%/ -18%/ +9%/ +9%/ +2%
> > 64/ 200/ +6%/ -19%/ +6%/ +6%/ 0%
> > 256/ 1/ +7%/ -33%/ +7%/ +7%/ +6%
> > 256/ 50/ +7%/ -18%/ +7%/ +7%/ 0%
> > 256/ 100/ +9%/ -18%/ +8%/ +8%/ +2%
> > 256/ 200/ +9%/ -18%/ +10%/ +10%/ +3%
> > 1024/ 1/ +20%/ -28%/ +20%/ +20%/ +19%
> > 1024/ 50/ +8%/ -18%/ +9%/ +9%/ +2%
> > 1024/ 100/ +6%/ -19%/ +5%/ +5%/ 0%
> > 1024/ 200/ +8%/ -18%/ +9%/ +9%/ +2%
> > Guest TX:
> > size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
> > 64/ 1/ -5%/ -28%/ +11%/ +12%/ +10%
> > 64/ 4/ -2%/ -26%/ +13%/ +13%/ +13%
> > 64/ 8/ -6%/ -29%/ +9%/ +10%/ +10%
> > 512/ 1/ +15%/ -7%/ +13%/ +11%/ +3%
> > 512/ 4/ +17%/ -6%/ +18%/ +13%/ +11%
> > 512/ 8/ +14%/ -7%/ +13%/ +7%/ +7%
> > 1024/ 1/ +27%/ -2%/ +26%/ +29%/ +12%
> > 1024/ 4/ +8%/ -9%/ +6%/ +1%/ +6%
> > 1024/ 8/ +41%/ +12%/ +34%/ +20%/ -3%
> > 4096/ 1/ -22%/ -21%/ -36%/ +81%/+1360%
> > 4096/ 4/ -57%/ -58%/ +286%/ +15%/+2074%
> > 4096/ 8/ +67%/ +70%/ -45%/ -8%/ +63%
> > 16384/ 1/ -2%/ -5%/ +5%/ -3%/ +80%
> > 16384/ 4/ 0%/ 0%/ 0%/ +4%/ +138%
> > 16384/ 8/ 0%/ 0%/ 0%/ +1%/ +41%
> > 65535/ 1/ -3%/ -6%/ +2%/ +11%/ +113%
> > 65535/ 4/ -2%/ -1%/ -2%/ -3%/ +484%
> > 65535/ 8/ 0%/ +1%/ 0%/ +2%/ +40%
> > Guest RX:
> > size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
> > 64/ 1/ +31%/ -3%/ +8%/ +8%/ +8%
> > 64/ 4/ +11%/ -17%/ +13%/ +14%/ +15%
> > 64/ 8/ +4%/ -23%/ +11%/ +11%/ +12%
> > 512/ 1/ +24%/ 0%/ +18%/ +14%/ -8%
> > 512/ 4/ +4%/ -15%/ +6%/ +5%/ +6%
> > 512/ 8/ +26%/ 0%/ +21%/ +10%/ +3%
> > 1024/ 1/ +88%/ +47%/ +69%/ +44%/ -30%
> > 1024/ 4/ +18%/ -5%/ +19%/ +16%/ +2%
> > 1024/ 8/ +15%/ -4%/ +13%/ +8%/ +1%
> > 4096/ 1/ -3%/ -5%/ +2%/ -2%/ +41%
> > 4096/ 4/ +2%/ +3%/ -20%/ -14%/ -24%
> > 4096/ 8/ -43%/ -45%/ +69%/ -24%/ +94%
> > 16384/ 1/ -3%/ -11%/ +23%/ +7%/ +42%
> > 16384/ 4/ -3%/ -3%/ -4%/ +5%/ +115%
> > 16384/ 8/ -1%/ 0%/ -1%/ -3%/ +32%
> > 65535/ 1/ +1%/ 0%/ +2%/ 0%/ +66%
> > 65535/ 4/ -1%/ -1%/ 0%/ +4%/ +492%
> > 65535/ 8/ 0%/ -1%/ -1%/ +4%/ +38%
> >
> > Changes from V3:
> > - drop single_task_running()
> > - use cpu_relax_lowlatency() instead of cpu_relax()
> >
> > Changes from V2:
> > - rename vhost_vq_more_avail() to vhost_vq_avail_empty(). And return
> > false we __get_user() fails.
> > - do not bother premmptions/timers for good path.
> > - use vhost_vring_state as ioctl parameter instead of reinveting a new
> > one.
> > - add the unit of timeout (us) to the comment of new added ioctls
> >
> > Changes from V1:
> > - remove the buggy vq_error() in vhost_vq_more_avail().
> > - leave vhost_enable_notify() untouched.
> >
> > Changes from RFC V3:
> > - small tweak on the code to avoid multiple duplicate conditions in
> > critical path when busy loop is not enabled.
> > - add the test result of multiple VMs
> >
> > Changes from RFC V2:
> > - poll also at the end of rx handling
> > - factor out the polling logic and optimize the code a little bit
> > - add two ioctls to get and set the busy poll timeout
> > - test on ixgbe (which can give more stable and reproducable numbers)
> > instead of mlx4.
> >
> > Changes from RFC V1:
> > - add a comment for vhost_has_work() to explain why it could be
> > lockless
> > - add param description for busyloop_timeout
> > - split out the busy polling logic into a new helper
> > - check and exit the loop when there's a pending signal
> > - disable preemption during busy looping to make sure lock_clock() was
> > correctly used.
> >
> > Jason Wang (3):
> > vhost: introduce vhost_has_work()
> > vhost: introduce vhost_vq_avail_empty()
> > vhost_net: basic polling support
> >
> > drivers/vhost/net.c | 78
+++++++++++++++++++++++++++++++++++++++++++---
> > drivers/vhost/vhost.c | 35 +++++++++++++++++++++
> > drivers/vhost/vhost.h | 3 ++
> > include/uapi/linux/vhost.h | 6 ++++
> > 4 files changed, 117 insertions(+), 5 deletions(-)
> >
>
^ permalink raw reply
* Re: [PATCH V4 0/3] basic busy polling support for vhost_net
From: Michael Rapoport @ 2016-03-10 6:48 UTC (permalink / raw)
To: Greg Kurz
Cc: yang.zhang.wz, kvm, mst, netdev, linux-kernel, virtualization,
borntraeger
In-Reply-To: <20160309202645.030ad7b2@bahia.lab.toulouse-stg.fr.ibm.com>
Hi Greg,
> Greg Kurz <gkurz@linux.vnet.ibm.com> wrote on 03/09/2016 09:26:45 PM:
> > On Fri, 4 Mar 2016 06:24:50 -0500
> > Jason Wang <jasowang@redhat.com> wrote:
>
> > This series tries to add basic busy polling for vhost net. The idea is
> > simple: at the end of tx/rx processing, busy polling for new tx added
> > descriptor and rx receive socket for a while. The maximum number of
> > time (in us) could be spent on busy polling was specified ioctl.
> >
> > Test A were done through:
> >
> > - 50 us as busy loop timeout
> > - Netperf 2.6
> > - Two machines with back to back connected mlx4
>
> Hi Jason,
>
> Could this also improve performance if both VMs are
> on the same host system ?
I've experimented a little with Jason's patches and guest-to-guest netperf
when both guests were on the same host, and I saw improvements for that
case.
> > - Guest with 1 vcpus and 1 queue
> >
> > Results:
> > - Obvious improvements (%5 - 20%) for latency (TCP_RR).
> > - Get a better or minor regression on most of the TX tests, but see
> > some regression on 4096 size.
> > - Except for 8 sessions of 4096 size RX, have a better or same
> > performance.
> > - CPU utilization were incrased as expected.
> >
> > TCP_RR:
> > size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
> > 1/ 1/ +8%/ -32%/ +8%/ +8%/ +7%
> > 1/ 50/ +7%/ -19%/ +7%/ +7%/ +1%
> > 1/ 100/ +5%/ -21%/ +5%/ +5%/ 0%
> > 1/ 200/ +5%/ -21%/ +7%/ +7%/ +1%
> > 64/ 1/ +11%/ -29%/ +11%/ +11%/ +10%
> > 64/ 50/ +7%/ -19%/ +8%/ +8%/ +2%
> > 64/ 100/ +8%/ -18%/ +9%/ +9%/ +2%
> > 64/ 200/ +6%/ -19%/ +6%/ +6%/ 0%
> > 256/ 1/ +7%/ -33%/ +7%/ +7%/ +6%
> > 256/ 50/ +7%/ -18%/ +7%/ +7%/ 0%
> > 256/ 100/ +9%/ -18%/ +8%/ +8%/ +2%
> > 256/ 200/ +9%/ -18%/ +10%/ +10%/ +3%
> > 1024/ 1/ +20%/ -28%/ +20%/ +20%/ +19%
> > 1024/ 50/ +8%/ -18%/ +9%/ +9%/ +2%
> > 1024/ 100/ +6%/ -19%/ +5%/ +5%/ 0%
> > 1024/ 200/ +8%/ -18%/ +9%/ +9%/ +2%
> > Guest TX:
> > size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
> > 64/ 1/ -5%/ -28%/ +11%/ +12%/ +10%
> > 64/ 4/ -2%/ -26%/ +13%/ +13%/ +13%
> > 64/ 8/ -6%/ -29%/ +9%/ +10%/ +10%
> > 512/ 1/ +15%/ -7%/ +13%/ +11%/ +3%
> > 512/ 4/ +17%/ -6%/ +18%/ +13%/ +11%
> > 512/ 8/ +14%/ -7%/ +13%/ +7%/ +7%
> > 1024/ 1/ +27%/ -2%/ +26%/ +29%/ +12%
> > 1024/ 4/ +8%/ -9%/ +6%/ +1%/ +6%
> > 1024/ 8/ +41%/ +12%/ +34%/ +20%/ -3%
> > 4096/ 1/ -22%/ -21%/ -36%/ +81%/+1360%
> > 4096/ 4/ -57%/ -58%/ +286%/ +15%/+2074%
> > 4096/ 8/ +67%/ +70%/ -45%/ -8%/ +63%
> > 16384/ 1/ -2%/ -5%/ +5%/ -3%/ +80%
> > 16384/ 4/ 0%/ 0%/ 0%/ +4%/ +138%
> > 16384/ 8/ 0%/ 0%/ 0%/ +1%/ +41%
> > 65535/ 1/ -3%/ -6%/ +2%/ +11%/ +113%
> > 65535/ 4/ -2%/ -1%/ -2%/ -3%/ +484%
> > 65535/ 8/ 0%/ +1%/ 0%/ +2%/ +40%
> > Guest RX:
> > size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
> > 64/ 1/ +31%/ -3%/ +8%/ +8%/ +8%
> > 64/ 4/ +11%/ -17%/ +13%/ +14%/ +15%
> > 64/ 8/ +4%/ -23%/ +11%/ +11%/ +12%
> > 512/ 1/ +24%/ 0%/ +18%/ +14%/ -8%
> > 512/ 4/ +4%/ -15%/ +6%/ +5%/ +6%
> > 512/ 8/ +26%/ 0%/ +21%/ +10%/ +3%
> > 1024/ 1/ +88%/ +47%/ +69%/ +44%/ -30%
> > 1024/ 4/ +18%/ -5%/ +19%/ +16%/ +2%
> > 1024/ 8/ +15%/ -4%/ +13%/ +8%/ +1%
> > 4096/ 1/ -3%/ -5%/ +2%/ -2%/ +41%
> > 4096/ 4/ +2%/ +3%/ -20%/ -14%/ -24%
> > 4096/ 8/ -43%/ -45%/ +69%/ -24%/ +94%
> > 16384/ 1/ -3%/ -11%/ +23%/ +7%/ +42%
> > 16384/ 4/ -3%/ -3%/ -4%/ +5%/ +115%
> > 16384/ 8/ -1%/ 0%/ -1%/ -3%/ +32%
> > 65535/ 1/ +1%/ 0%/ +2%/ 0%/ +66%
> > 65535/ 4/ -1%/ -1%/ 0%/ +4%/ +492%
> > 65535/ 8/ 0%/ -1%/ -1%/ +4%/ +38%
> >
> > Changes from V3:
> > - drop single_task_running()
> > - use cpu_relax_lowlatency() instead of cpu_relax()
> >
> > Changes from V2:
> > - rename vhost_vq_more_avail() to vhost_vq_avail_empty(). And return
> > false we __get_user() fails.
> > - do not bother premmptions/timers for good path.
> > - use vhost_vring_state as ioctl parameter instead of reinveting a new
> > one.
> > - add the unit of timeout (us) to the comment of new added ioctls
> >
> > Changes from V1:
> > - remove the buggy vq_error() in vhost_vq_more_avail().
> > - leave vhost_enable_notify() untouched.
> >
> > Changes from RFC V3:
> > - small tweak on the code to avoid multiple duplicate conditions in
> > critical path when busy loop is not enabled.
> > - add the test result of multiple VMs
> >
> > Changes from RFC V2:
> > - poll also at the end of rx handling
> > - factor out the polling logic and optimize the code a little bit
> > - add two ioctls to get and set the busy poll timeout
> > - test on ixgbe (which can give more stable and reproducable numbers)
> > instead of mlx4.
> >
> > Changes from RFC V1:
> > - add a comment for vhost_has_work() to explain why it could be
> > lockless
> > - add param description for busyloop_timeout
> > - split out the busy polling logic into a new helper
> > - check and exit the loop when there's a pending signal
> > - disable preemption during busy looping to make sure lock_clock() was
> > correctly used.
> >
> > Jason Wang (3):
> > vhost: introduce vhost_has_work()
> > vhost: introduce vhost_vq_avail_empty()
> > vhost_net: basic polling support
> >
> > drivers/vhost/net.c | 78
+++++++++++++++++++++++++++++++++++++++++++---
> > drivers/vhost/vhost.c | 35 +++++++++++++++++++++
> > drivers/vhost/vhost.h | 3 ++
> > include/uapi/linux/vhost.h | 6 ++++
> > 4 files changed, 117 insertions(+), 5 deletions(-)
> >
>
^ permalink raw reply
* Re: Hung up was occurred after v4.4-rc1 during IPv6 Ready Logo Conformance Test
From: Martin KaFai Lau @ 2016-03-10 6:39 UTC (permalink / raw)
To: Yuki Machida; +Cc: netdev
In-Reply-To: <56E0F631.7010906@jp.fujitsu.com>
On Thu, Mar 10, 2016 at 01:21:05PM +0900, Yuki Machida wrote:
> Hi all,
>
> Hung up was occurred at Linux Kernel after v4.4-rc1 during IPv6 Ready Logo Conformance Test.
> Not Fix a bug in v4.5-rc7 yet.
>
> Currently, it is under investigation.
>
> The following are the details:
I think you forgot to paste the details?
>
> IPv6 Ready Logo
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ipv6ready.org_&d=CwICJg&c=5VD0RTtNlTh3ycd41b3MUw&r=VQnoQ7LvghIj0gVEaiQSUw&m=BzjeED6xB_NfQBjateVt8Gh7IK7hST7tSmPGXEAtRdQ&s=oaM204vOhRnGb6aTD1wxTZu9ss_bCzFAeZU0EmvVe6I&e=
>
> I ran the IPv6 Ready Logo Core Conformance Test on Intel D510MO (Atom D510).
> It is using userland build with yocto project.
>
> Test Environment
> Test Specification : 4.0.6
> Tool Version : REL_3_3_2
> Test Program Version : V6LC_5_0_0
> Target Device : Intel D510MO (Atom D510)
>
> Regards,
> Yuki Machida
^ permalink raw reply
* Re: [PATCH net-next] bpf: avoid copying junk bytes in bpf_get_current_comm()
From: David Miller @ 2016-03-10 4:28 UTC (permalink / raw)
To: ast; +Cc: daniel, tobias, brendan.d.gregg, netdev, linux-kernel,
kernel-team
In-Reply-To: <1457582553-395600-1-git-send-email-ast@fb.com>
From: Alexei Starovoitov <ast@fb.com>
Date: Wed, 9 Mar 2016 20:02:33 -0800
> Lots of places in the kernel use memcpy(buf, comm, TASK_COMM_LEN); but
> the result is typically passed to print("%s", buf) and extra bytes
> after zero don't cause any harm.
> In bpf the result of bpf_get_current_comm() is used as the part of
> map key and was causing spurious hash map mismatches.
> Use strlcpy() to guarantee zero-terminated string.
> bpf verifier checks that output buffer is zero-initialized,
> so even for short task names the output buffer don't have junk bytes.
> Note it's not a security concern, since kprobe+bpf is root only.
>
> Fixes: ffeedafbf023 ("bpf: introduce current->pid, tgid, uid, gid, comm accessors")
> Reported-by: Tobias Waldekranz <tobias@waldekranz.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> Targeting net-next, since it's too late for net.
> I think it makes sense for stable as well.
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next] bpf: bpf_stackmap_copy depends on CONFIG_PERF_EVENTS
From: David Miller @ 2016-03-10 4:28 UTC (permalink / raw)
To: ast; +Cc: daniel, fengguang.wu, netdev
In-Reply-To: <1457578609-277357-1-git-send-email-ast@fb.com>
From: Alexei Starovoitov <ast@fb.com>
Date: Wed, 9 Mar 2016 18:56:49 -0800
> 0-day bot reported build error:
> kernel/built-in.o: In function `map_lookup_elem':
>>> kernel/bpf/.tmp_syscall.o:(.text+0x329b3c): undefined reference to `bpf_stackmap_copy'
> when CONFIG_BPF_SYSCALL is set and CONFIG_PERF_EVENTS is not.
> Add weak definition to resolve it.
> This code path in map_lookup_elem() is never taken
> when CONFIG_PERF_EVENTS is not set.
>
> Fixes: 557c0c6e7df8 ("bpf: convert stackmap to pre-allocation")
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/3] validate variable length ll headers
From: David Miller @ 2016-03-10 4:26 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, willemb
In-Reply-To: <CAF=yD-+RirnCYnCcREHx0G-a6MWMSYhx4zwzKxKOUSGyHRQsdw@mail.gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Wed, 9 Mar 2016 23:22:59 -0500
> On Wed, Mar 9, 2016 at 10:13 PM, David Miller <davem@davemloft.net> wrote:
>> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
>> Date: Wed, 9 Mar 2016 21:58:31 -0500
>>
>>> Allow device-specific validation of link layer headers. Existing
>>> checks drop all packets shorter than hard_header_len. For variable
>>> length protocols, such packets can be valid.
>>>
>>> patch 1 adds header_ops.validate and dev_validate_header
>>> patch 2 implements the protocol specific callback for AX25
>>> patch 3 replaces ll_header_truncated with dev_validate_header
>>
>> Series applied and queued up for -stable, thanks!
>
> Thanks, David. Please don't queue the earlier net patchset for stable.
> I spotted an issue while converting to net-next, I'm afraid. I can send a
> net v2 (as net-next won't apply), or a separate stable patch once 4.5 is cut.
I'll use the net-next patches as the basis for the backport I do,
thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/3] validate variable length ll headers
From: Willem de Bruijn @ 2016-03-10 4:22 UTC (permalink / raw)
To: David Miller; +Cc: Network Development, Willem de Bruijn
In-Reply-To: <20160309.221340.1476836223911285635.davem@davemloft.net>
On Wed, Mar 9, 2016 at 10:13 PM, David Miller <davem@davemloft.net> wrote:
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Date: Wed, 9 Mar 2016 21:58:31 -0500
>
>> Allow device-specific validation of link layer headers. Existing
>> checks drop all packets shorter than hard_header_len. For variable
>> length protocols, such packets can be valid.
>>
>> patch 1 adds header_ops.validate and dev_validate_header
>> patch 2 implements the protocol specific callback for AX25
>> patch 3 replaces ll_header_truncated with dev_validate_header
>
> Series applied and queued up for -stable, thanks!
Thanks, David. Please don't queue the earlier net patchset for stable.
I spotted an issue while converting to net-next, I'm afraid. I can send a
net v2 (as net-next won't apply), or a separate stable patch once 4.5 is cut.
^ permalink raw reply
* Hung up was occurred after v4.4-rc1 during IPv6 Ready Logo Conformance Test
From: Yuki Machida @ 2016-03-10 4:21 UTC (permalink / raw)
To: netdev
Hi all,
Hung up was occurred at Linux Kernel after v4.4-rc1 during IPv6 Ready Logo Conformance Test.
Not Fix a bug in v4.5-rc7 yet.
Currently, it is under investigation.
The following are the details:
IPv6 Ready Logo
https://www.ipv6ready.org/
I ran the IPv6 Ready Logo Core Conformance Test on Intel D510MO (Atom D510).
It is using userland build with yocto project.
Test Environment
Test Specification : 4.0.6
Tool Version : REL_3_3_2
Test Program Version : V6LC_5_0_0
Target Device : Intel D510MO (Atom D510)
Regards,
Yuki Machida
^ permalink raw reply
* [PATCH net-next] bpf: avoid copying junk bytes in bpf_get_current_comm()
From: Alexei Starovoitov @ 2016-03-10 4:02 UTC (permalink / raw)
To: David S . Miller
Cc: Daniel Borkmann, Tobias Waldekranz, Brendan Gregg, netdev,
linux-kernel, kernel-team
Lots of places in the kernel use memcpy(buf, comm, TASK_COMM_LEN); but
the result is typically passed to print("%s", buf) and extra bytes
after zero don't cause any harm.
In bpf the result of bpf_get_current_comm() is used as the part of
map key and was causing spurious hash map mismatches.
Use strlcpy() to guarantee zero-terminated string.
bpf verifier checks that output buffer is zero-initialized,
so even for short task names the output buffer don't have junk bytes.
Note it's not a security concern, since kprobe+bpf is root only.
Fixes: ffeedafbf023 ("bpf: introduce current->pid, tgid, uid, gid, comm accessors")
Reported-by: Tobias Waldekranz <tobias@waldekranz.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
Targeting net-next, since it's too late for net.
I think it makes sense for stable as well.
kernel/bpf/helpers.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 4504ca66118d..50da680c479f 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -166,7 +166,7 @@ static u64 bpf_get_current_comm(u64 r1, u64 size, u64 r3, u64 r4, u64 r5)
if (!task)
return -EINVAL;
- memcpy(buf, task->comm, min_t(size_t, size, sizeof(task->comm)));
+ strlcpy(buf, task->comm, min_t(size_t, size, sizeof(task->comm)));
return 0;
}
--
2.8.0.rc1
^ permalink raw reply related
* Re: [PATCH nf-next v9 8/8] openvswitch: Interface with NAT.
From: Joe Stringer @ 2016-03-10 3:47 UTC (permalink / raw)
To: Jarno Rajahalme, Thomas Graf
Cc: ovs dev, netdev, netfilter-devel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1457565024-27300-8-git-send-email-jarno-LZ6Gd1LRuIk@public.gmane.org>
Hi Jarno,
Thanks for working on this. Mostly just a few style things around #ifdefs below.
On 9 March 2016 at 15:10, Jarno Rajahalme <jarno@ovn.org> wrote:
> Extend OVS conntrack interface to cover NAT. New nested
> OVS_CT_ATTR_NAT attribute may be used to include NAT with a CT action.
> A bare OVS_CT_ATTR_NAT only mangles existing and expected connections.
> If OVS_NAT_ATTR_SRC or OVS_NAT_ATTR_DST is included within the nested
> attributes, new (non-committed/non-confirmed) connections are mangled
> according to the rest of the nested attributes.
>
> The corresponding OVS userspace patch series includes test cases (in
> tests/system-traffic.at) that also serve as example uses.
>
> This work extends on a branch by Thomas Graf at
> https://github.com/tgraf/ovs/tree/nat.
Thomas, I guess there was not signoff in these patches so Jarno does
not have your signoff in this patch.
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> ---
> v9: Fixed module dependencies.
>
> include/uapi/linux/openvswitch.h | 49 ++++
> net/openvswitch/Kconfig | 3 +-
> net/openvswitch/conntrack.c | 523 +++++++++++++++++++++++++++++++++++++--
> net/openvswitch/conntrack.h | 3 +-
> 4 files changed, 551 insertions(+), 27 deletions(-)
<snip>
> diff --git a/net/openvswitch/Kconfig b/net/openvswitch/Kconfig
> index cd5fd9d..23471a4 100644
> --- a/net/openvswitch/Kconfig
> +++ b/net/openvswitch/Kconfig
> @@ -6,7 +6,8 @@ config OPENVSWITCH
> tristate "Open vSwitch"
> depends on INET
> depends on !NF_CONNTRACK || \
> - (NF_CONNTRACK && (!NF_DEFRAG_IPV6 || NF_DEFRAG_IPV6))
> + (NF_CONNTRACK && ((!NF_DEFRAG_IPV6 || NF_DEFRAG_IPV6) && \
> + (!NF_NAT || NF_NAT)))
Whitespace.
> select LIBCRC32C
> select MPLS
> select NET_MPLS_GSO
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 5711f80..6455237 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
<snip>
> struct ovs_ct_len_tbl {
> - size_t maxlen;
> - size_t minlen;
> + int maxlen;
> + int minlen;
> };
Are these changed for a specific reason, or just to use INT_MAX rather
than SIZE_MAX in ovs_ct_len_tbl?
> /* Metadata mark for masked write to conntrack mark */
> @@ -42,15 +52,29 @@ struct md_labels {
> struct ovs_key_ct_labels mask;
> };
>
> +#ifdef CONFIG_NF_NAT_NEEDED
> +enum ovs_ct_nat {
> + OVS_CT_NAT = 1 << 0, /* NAT for committed connections only. */
> + OVS_CT_SRC_NAT = 1 << 1, /* Source NAT for NEW connections. */
> + OVS_CT_DST_NAT = 1 << 2, /* Destination NAT for NEW connections. */
> +};
> +#endif
Here...
> /* Conntrack action context for execution. */
> struct ovs_conntrack_info {
> struct nf_conntrack_helper *helper;
> struct nf_conntrack_zone zone;
> struct nf_conn *ct;
> u8 commit : 1;
> +#ifdef CONFIG_NF_NAT_NEEDED
> + u8 nat : 3; /* enum ovs_ct_nat */
> +#endif
and here.. I wonder if we can trim more of these #ifdefs, for
readability and more compiler coverage if the feature is disabled.
> u16 family;
> struct md_mark mark;
> struct md_labels labels;
> +#ifdef CONFIG_NF_NAT_NEEDED
> + struct nf_nat_range range; /* Only present for SRC NAT and DST NAT. */
> +#endif
> };
(probably not this one, though)
> static void __ovs_ct_free_action(struct ovs_conntrack_info *ct_info);
> @@ -137,12 +161,15 @@ static void __ovs_ct_update_key(struct sw_flow_key *key, u8 state,
> ovs_ct_get_labels(ct, &key->ct.labels);
> }
>
> -/* Update 'key' based on skb->nfct. If 'post_ct' is true, then OVS has
> - * previously sent the packet to conntrack via the ct action.
> +/* Update 'key' based on skb->nfct. If 'post_ct' is true, then OVS has
> + * previously sent the packet to conntrack via the ct action. If
> + * 'keep_nat_flags' is true, the existing NAT flags retained, else they are
> + * initialized from the connection status.
> */
> static void ovs_ct_update_key(const struct sk_buff *skb,
> const struct ovs_conntrack_info *info,
> - struct sw_flow_key *key, bool post_ct)
> + struct sw_flow_key *key, bool post_ct,
> + bool keep_nat_flags)
> {
> const struct nf_conntrack_zone *zone = &nf_ct_zone_dflt;
> enum ip_conntrack_info ctinfo;
> @@ -160,6 +187,16 @@ static void ovs_ct_update_key(const struct sk_buff *skb,
> */
> if (ct->master)
> state |= OVS_CS_F_RELATED;
> +#ifdef CONFIG_NF_NAT_NEEDED
> + if (keep_nat_flags) {
> + state |= key->ct.state & OVS_CS_F_NAT_MASK;
> + } else {
> + if (ct->status & IPS_SRC_NAT)
> + state |= OVS_CS_F_SRC_NAT;
> + if (ct->status & IPS_DST_NAT)
> + state |= OVS_CS_F_DST_NAT;
> + }
> +#endif
This could be nested within something like if (CONFIG_NF_NAT_NEEDED)
{...} or if (IS_ENABLED(...)) ... to always run the code through the
compiler (which would likely compile out the whole check).
<snip>
> +#ifdef CONFIG_NF_NAT_NEEDED
> + /* Adjust seqs after helper. This is needed due to some helpers (e.g.,
> + * FTP with NAT) adusting the TCP payload size when mangling IP
> + * addresses and/or port numbers in the text-based control connection.
> + */
> + if (test_bit(IPS_SEQ_ADJUST_BIT, &ct->status) &&
> + !nf_ct_seq_adjust(skb, ct, ctinfo, protoff))
> + return NF_DROP;
> +#endif
> + return NF_ACCEPT;
> }
I suspect this would still compile and work correctly without the ifdef?
<snip>
> +/* Returns NF_DROP if the packet should be dropped, NF_ACCEPT otherwise. */
> +static int ovs_ct_nat(struct net *net, struct sw_flow_key *key,
> + const struct ovs_conntrack_info *info,
> + struct sk_buff *skb, struct nf_conn *ct,
> + enum ip_conntrack_info ctinfo)
> +{
> + enum nf_nat_manip_type maniptype;
> + int err;
> +
> + if (nf_ct_is_untracked(ct)) {
> + /* A NAT action may only be performed on tracked packets. */
> + return NF_ACCEPT;
> + }
> +
> + /* Add NAT extension if not confirmed yet. */
> + if (!nf_ct_is_confirmed(ct) && !nf_ct_nat_ext_add(ct))
> + return NF_ACCEPT; /* Can't NAT. */
> +
> + /* Determine NAT type.
> + * Check if the NAT type can be deduced from the tracked connection.
> + * Make sure expected traffic is NATted only when committing.
> + */
> + if (info->nat & OVS_CT_NAT && ctinfo != IP_CT_NEW &&
> + ct->status & IPS_NAT_MASK &&
> + (!(ct->status & IPS_EXPECTED_BIT) || info->commit)) {
> + /* NAT an established or related connection like before. */
> + if (CTINFO2DIR(ctinfo) == IP_CT_DIR_REPLY)
> + /* This is the REPLY direction for a connection
> + * for which NAT was applied in the forward
> + * direction. Do the reverse NAT.
> + */
> + maniptype = ct->status & IPS_SRC_NAT
> + ? NF_NAT_MANIP_DST : NF_NAT_MANIP_SRC;
> + else
> + maniptype = ct->status & IPS_SRC_NAT
> + ? NF_NAT_MANIP_SRC : NF_NAT_MANIP_DST;
> + } else if (info->nat & OVS_CT_SRC_NAT) {
> + maniptype = NF_NAT_MANIP_SRC;
> + } else if (info->nat & OVS_CT_DST_NAT) {
> + maniptype = NF_NAT_MANIP_DST;
> + } else {
> + return NF_ACCEPT; /* Connection is not NATed. */
> + }
> + err = ovs_ct_nat_execute(skb, ct, ctinfo, &info->range, maniptype);
> +
> + /* Mark NAT done if successful and update the flow key. */
> + if (err == NF_ACCEPT)
> + ovs_nat_update_key(key, skb, maniptype);
> +
> + return err;
> +}
> +#endif
<snip>
> - /* Call the helper only if we did nf_conntrack_in() above ('!cached')
> - * for confirmed connections, but only when committing for unconfirmed
> - * connections.
> - */
> ct = nf_ct_get(skb, &ctinfo);
> - if (ct && (nf_ct_is_confirmed(ct) ? !cached : info->commit) &&
> - ovs_ct_helper(skb, info->family) != NF_ACCEPT) {
> - WARN_ONCE(1, "helper rejected packet");
> - return -EINVAL;
> + if (ct) {
> +#ifdef CONFIG_NF_NAT_NEEDED
> + /* Packets starting a new connection must be NATted before the
> + * helper, so that the helper knows about the NAT. We enforce
> + * this by delaying both NAT and helper calls for unconfirmed
> + * connections until the committing CT action. For later
> + * packets NAT and Helper may be called in either order.
> + *
> + * NAT will be done only if the CT action has NAT, and only
> + * once per packet (per zone), as guarded by the NAT bits in
> + * the key->ct.state.
> + */
> + if (info->nat && !(key->ct.state & OVS_CS_F_NAT_MASK) &&
> + (nf_ct_is_confirmed(ct) || info->commit) &&
> + ovs_ct_nat(net, key, info, skb, ct, ctinfo) != NF_ACCEPT) {
> + return -EINVAL;
> + }
> +#endif
Again, I think the #ifdefs are probably unnecessary. Currently
ovs_ct_nat() is conditionally compiled out, but we could provide a
stub version for the !CONFIG_NF_NAT_NEEDED case instead, placed
immediately below the actual implementation further up:
...
#else /* !CONFIG_NF_NAT_NEEDED */
static int ovs_ct_nat(struct net *net, struct sw_flow_key *key,
const struct ovs_conntrack_info *info,
struct sk_buff *skb, struct nf_conn *ct,
enum ip_conntrack_info ctinfo)
{
return NF_ACCEPT;
}
#endif
Thanks,
Joe
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [PATCH] bpf: make bpf_stackmap_copy conditionally called
From: Alexei Starovoitov @ 2016-03-10 3:38 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Alexei Starovoitov, David S. Miller, Daniel Borkmann, netdev,
linux-kernel
In-Reply-To: <1457574245-2998842-1-git-send-email-arnd@arndb.de>
On Thu, Mar 10, 2016 at 02:43:42AM +0100, Arnd Bergmann wrote:
> Changing the bpf syscall to use the new bpf_stackmap_copy() helper for
> BPF_MAP_TYPE_STACK_TRACE causes a link error when CONFIG_PERF_EVENTS
> is disabled:
>
> kernel/built-in.o: In function `map_lookup_elem':
> :(.text+0x7fca4): undefined reference to `bpf_stackmap_copy'
>
> This patch simply avoids handling that case, which may or may not
> be the correct answer here.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 557c0c6e7df8 ("bpf: convert stackmap to pre-allocation")
...
> - } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE) {
> + } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE &&
> + IS_ENABLED(CONFIG_PERF_EVENTS)) {
> err = bpf_stackmap_copy(map, key, value);
> } else {
yes. this is also ok-ish fix.
I've sent different version already:
http://patchwork.ozlabs.org/patch/595617/
I considered the option like yours but it's relying on gcc doing
dead code elimination of 'if (false) {}' branch and though kernel
is never compiled with -O0. I didn't want to take the risk.
I'm fine with either approach though.
^ permalink raw reply
* [PATCH] bpf: make bpf_stackmap_copy conditionally called
From: Arnd Bergmann @ 2016-03-10 1:43 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Arnd Bergmann, David S. Miller, Daniel Borkmann, netdev,
linux-kernel
Changing the bpf syscall to use the new bpf_stackmap_copy() helper for
BPF_MAP_TYPE_STACK_TRACE causes a link error when CONFIG_PERF_EVENTS
is disabled:
kernel/built-in.o: In function `map_lookup_elem':
:(.text+0x7fca4): undefined reference to `bpf_stackmap_copy'
This patch simply avoids handling that case, which may or may not
be the correct answer here.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 557c0c6e7df8 ("bpf: convert stackmap to pre-allocation")
---
kernel/bpf/syscall.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 2978d0d08869..b014b64b6116 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -290,7 +290,8 @@ static int map_lookup_elem(union bpf_attr *attr)
err = bpf_percpu_hash_copy(map, key, value);
} else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
err = bpf_percpu_array_copy(map, key, value);
- } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE) {
+ } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE &&
+ IS_ENABLED(CONFIG_PERF_EVENTS)) {
err = bpf_stackmap_copy(map, key, value);
} else {
rcu_read_lock();
--
2.7.0
^ permalink raw reply related
* Re: [PATCH net-next 0/3] validate variable length ll headers
From: David Miller @ 2016-03-10 3:13 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, willemb
In-Reply-To: <1457578714-36125-1-git-send-email-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Wed, 9 Mar 2016 21:58:31 -0500
> Allow device-specific validation of link layer headers. Existing
> checks drop all packets shorter than hard_header_len. For variable
> length protocols, such packets can be valid.
>
> patch 1 adds header_ops.validate and dev_validate_header
> patch 2 implements the protocol specific callback for AX25
> patch 3 replaces ll_header_truncated with dev_validate_header
Series applied and queued up for -stable, thanks!
^ permalink raw reply
* [PATCH net-next 3/3] packet: validate variable length ll headers
From: Willem de Bruijn @ 2016-03-10 2:58 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
In-Reply-To: <1457578714-36125-1-git-send-email-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemb@google.com>
Replace link layer header validation check ll_header_truncate with
more generic dev_validate_header.
Validation based on hard_header_len incorrectly drops valid packets
in variable length protocols, such as AX25. dev_validate_header
calls header_ops.validate for such protocols to ensure correctness
below hard_header_len.
See also http://comments.gmane.org/gmane.linux.network/401064
Fixes 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/packet/af_packet.c | 43 ++++++++++++++++++-------------------------
1 file changed, 18 insertions(+), 25 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index d41b107..1ecfa71 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1915,6 +1915,10 @@ retry:
goto retry;
}
+ if (!dev_validate_header(dev, skb->data, len)) {
+ err = -EINVAL;
+ goto out_unlock;
+ }
if (len > (dev->mtu + dev->hard_header_len + extra_len) &&
!packet_extra_vlan_len_allowed(dev, skb)) {
err = -EMSGSIZE;
@@ -2393,18 +2397,6 @@ static void tpacket_destruct_skb(struct sk_buff *skb)
sock_wfree(skb);
}
-static bool ll_header_truncated(const struct net_device *dev, int len)
-{
- /* net device doesn't like empty head */
- if (unlikely(len < dev->hard_header_len)) {
- net_warn_ratelimited("%s: packet size is too short (%d < %d)\n",
- current->comm, len, dev->hard_header_len);
- return true;
- }
-
- return false;
-}
-
static void tpacket_set_protocol(const struct net_device *dev,
struct sk_buff *skb)
{
@@ -2522,16 +2514,20 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
if (unlikely(err < 0))
return -EINVAL;
} else if (copylen) {
+ int hdrlen = min_t(int, copylen, tp_len);
+
skb_push(skb, dev->hard_header_len);
skb_put(skb, copylen - dev->hard_header_len);
- err = skb_store_bits(skb, 0, data, copylen);
+ err = skb_store_bits(skb, 0, data, hdrlen);
if (unlikely(err))
return err;
+ if (!dev_validate_header(dev, skb->data, hdrlen))
+ return -EINVAL;
if (!skb->protocol)
tpacket_set_protocol(dev, skb);
- data += copylen;
- to_write -= copylen;
+ data += hdrlen;
+ to_write -= hdrlen;
}
offset = offset_in_page(data);
@@ -2703,13 +2699,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
copylen = __virtio16_to_cpu(vio_le(),
vnet_hdr->hdr_len);
}
- if (dev->hard_header_len) {
- if (ll_header_truncated(dev, tp_len)) {
- tp_len = -EINVAL;
- goto tpacket_error;
- }
- copylen = max_t(int, copylen, dev->hard_header_len);
- }
+ copylen = max_t(int, copylen, dev->hard_header_len);
skb = sock_alloc_send_skb(&po->sk,
hlen + tlen + sizeof(struct sockaddr_ll) +
(copylen - dev->hard_header_len),
@@ -2905,9 +2895,6 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
offset = dev_hard_header(skb, dev, ntohs(proto), addr, NULL, len);
if (unlikely(offset < 0))
goto out_free;
- } else {
- if (ll_header_truncated(dev, len))
- goto out_free;
}
/* Returns -EFAULT on error */
@@ -2915,6 +2902,12 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
if (err)
goto out_free;
+ if (sock->type == SOCK_RAW &&
+ !dev_validate_header(dev, skb->data, len)) {
+ err = -EINVAL;
+ goto out_free;
+ }
+
sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags);
if (!vnet_hdr.gso_type && (len > dev->mtu + reserve + extra_len) &&
--
2.7.0.rc3.207.g0ac5344
^ permalink raw reply related
* [PATCH net-next 2/3] ax25: add link layer header validation function
From: Willem de Bruijn @ 2016-03-10 2:58 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
In-Reply-To: <1457578714-36125-1-git-send-email-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemb@google.com>
As variable length protocol, AX25 fails link layer header validation
tests based on a minimum length. header_ops.validate allows protocols
to validate headers that are shorter than hard_header_len. Implement
this callback for AX25.
See also http://comments.gmane.org/gmane.linux.network/401064
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/ax25/ax25_ip.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/net/ax25/ax25_ip.c b/net/ax25/ax25_ip.c
index b563a3f..2fa3be9 100644
--- a/net/ax25/ax25_ip.c
+++ b/net/ax25/ax25_ip.c
@@ -228,8 +228,23 @@ netdev_tx_t ax25_ip_xmit(struct sk_buff *skb)
}
#endif
+static bool ax25_validate_header(const char *header, unsigned int len)
+{
+ ax25_digi digi;
+
+ if (!len)
+ return false;
+
+ if (header[0])
+ return true;
+
+ return ax25_addr_parse(header + 1, len - 1, NULL, NULL, &digi, NULL,
+ NULL);
+}
+
const struct header_ops ax25_header_ops = {
.create = ax25_hard_header,
+ .validate = ax25_validate_header,
};
EXPORT_SYMBOL(ax25_header_ops);
--
2.7.0.rc3.207.g0ac5344
^ 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