Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH bpf-next 3/4] selftests/bpf: test_progs: remove global fail/success counts
From: Andrii Nakryiko @ 2019-08-14 20:22 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Stanislav Fomichev, Networking, bpf, David S. Miller,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
In-Reply-To: <20190814200751.GM2820@mini-arch>

On Wed, Aug 14, 2019 at 1:07 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 08/14, Andrii Nakryiko wrote:
> > On Wed, Aug 14, 2019 at 9:48 AM Stanislav Fomichev <sdf@google.com> wrote:
> > >
> > > Now that we have a global per-test/per-environment state, there
> > > is no longer the need to have global fail/success counters
> > > (and there is no need to save/get the diff before/after the
> > > test).
> > >
> > > Cc: Andrii Nakryiko <andriin@fb.com>
> > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > > ---

[...]

> > > +void __test__fail(const char *file, int line)
> > > +{
> > > +       if (env.test->subtest_name)
> > > +               fprintf(stderr, "%s:%s failed at %s:%d, errno=%d\n",
> >
> > Nit: let's keep <test>/<subtest> convention here as well?
> >
> > Failure doesn't always set errno, this will be quite confusing
> > sometimes. Especially for higher-level libbpf APIs, which don't set
> > errno at all.
> > If test wants to log additional information, let it do it explicitly.
> SG. Maybe we can adapt log_err from cgroup_helpers.h for error reporting
> once I move sockopt tests into test_progs.
>
> > Also, _CHECK already logs error message, so this is going to be
> > double-verbose for typical case. I'd say let's drop these error
> > messages and instead slightly extend _CHECK one with line number (it
> > already logs func name).
> Not everything goes through the _CHECK macro unfortunately, see

Well, it should (at least eventually). If existing macro doesn't cover
typical use case, we can add one that does cover.

> all the cases where I did s/error_cnt++/test__fail/. How about I
> remove the error message from _CHECK and leave it in __test_fail?

I'd keep test__fail() and test__success() as a low-level building
block. And move all the logging into corresponding high-level macro.
This still gives flexibility to do one-off crazy tests, if necessary,
while having consistent approach for everything else.

>
> > > +                       env.test->test_name, env.test->subtest_name,
> > > +                       file, line, errno);
> > > +       else
> > > +               fprintf(stderr, "%s failed at %s:%d, errno=%d\n",
> > > +                       env.test->test_name, file, line, errno);
> > > +
> > > +       env.test->fail_cnt++;
> > > +}
> > > +
> > >  struct ipv4_packet pkt_v4 = {
> > >         .eth.h_proto = __bpf_constant_htons(ETH_P_IP),
> > >         .iph.ihl = 5,
> > > @@ -150,7 +145,7 @@ int bpf_find_map(const char *test, struct bpf_object *obj, const char *name)
> > >         map = bpf_object__find_map_by_name(obj, name);
> > >         if (!map) {
> > >                 printf("%s:FAIL:map '%s' not found\n", test, name);
> > > -               error_cnt++;
> > > +               test__fail();
> > >                 return -1;
> > >         }
> > >         return bpf_map__fd(map);
> > > @@ -509,8 +504,6 @@ int main(int argc, char **argv)
> > >         stdio_hijack();
> > >         for (i = 0; i < prog_test_cnt; i++) {
> > >                 struct prog_test_def *test = &prog_test_defs[i];
> > > -               int old_pass_cnt = pass_cnt;
> > > -               int old_error_cnt = error_cnt;
> > >
> > >                 env.test = test;
> > >                 test->test_num = i + 1;
> > > @@ -525,14 +518,12 @@ int main(int argc, char **argv)
> > >                         test__end_subtest();
> > >
> > >                 test->tested = true;
> > > -               test->pass_cnt = pass_cnt - old_pass_cnt;
> > > -               test->error_cnt = error_cnt - old_error_cnt;
> > > -               if (test->error_cnt)
> > > +               if (test->fail_cnt)
> > >                         env.fail_cnt++;
> > >                 else
> > >                         env.succ_cnt++;
> > >
> > > -               dump_test_log(test, test->error_cnt);
> > > +               dump_test_log(test, test->fail_cnt);
> > >
> > >                 fprintf(env.stdout, "#%3d     %4s %s\n",
> > >                         test->test_num,
> > > @@ -546,5 +537,5 @@ int main(int argc, char **argv)
> > >         free(env.test_selector.num_set);
> > >         free(env.subtest_selector.num_set);
> > >
> > > -       return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
> > > +       return env.fail_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
> > >  }
> > > diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
> > > index 9defd35cb6c0..7b05921784a4 100644
> > > --- a/tools/testing/selftests/bpf/test_progs.h
> > > +++ b/tools/testing/selftests/bpf/test_progs.h
> > > @@ -38,7 +38,23 @@ typedef __u16 __sum16;
> > >  #include "trace_helpers.h"
> > >  #include "flow_dissector_load.h"
> > >
> > > -struct prog_test_def;
> > > +struct prog_test_def {
> > > +       const char *test_name;
> > > +       int test_num;
> > > +       void (*run_test)(void);
> > > +       bool force_log;
> > > +       bool tested;
> > > +
> > > +       const char *subtest_name;
> > > +       int subtest_num;
> > > +
> > > +       int succ_cnt;
> > > +       int fail_cnt;
> >
> > So I'm neutral on this rename, I even considered it myself initially.
> > But keep in mind, that succ/fail in env means number of tests, while
> > test->succ/fail means number of assertions. We don't report total
> > number of failed checks anymore, so it doesn't matter, but if we ever
> > want to keep track of that at env level, it will be very confusing and
> > inconvenient.
> Point taken, I didn't think about it, let me undo the rename. I'll
> try to add a comment instead to highlight the difference.
>
> > > +
> > > +       /* store counts before subtest started */
> > > +       int old_succ_cnt;
> > > +       int old_fail_cnt;
> > > +};
> >
> > Did you move it here just to access env.test->succ_cnt in _CHECK()?
> > Maybe add test__success() counterpart to test__fail() instead?
> Yeah, good point, will do.
>
> > >
> > >  struct test_selector {
> > >         const char *name;
> > > @@ -67,13 +83,13 @@ struct test_env {
> > >         int skip_cnt; /* skipped tests */
> > >  };
> > >
> > > -extern int error_cnt;
> > > -extern int pass_cnt;
> > >  extern struct test_env env;
> > >
> > >  extern void test__force_log();
> > >  extern bool test__start_subtest(const char *name);
> > >  extern void test__skip(void);
> > > +#define test__fail() __test__fail(__FILE__, __LINE__)
> > > +extern void __test__fail(const char *file, int line);
> >
> > Given my comment above about too verbose logging, I'd say let's keep
> > this simple and have just
> >
> > extern void test__fail()
> >
> > And let _CHECK log file:line.
> See above about test__fail without _CHECK. Maybe we should do QCHECK
> as you suggested in the other email.
>
> So those lonely:
>
>         if (err) {
>                 error_cnt++;
>                 return;
>         }
>
> checks can instead be converted to:
>
>         if (QCHECK(err))
>                 return;
>
> Let me play with it a bit and see how it goes.

Yeah, give it a go. Try keeping file:line logging in macro, where it's
more natural, IMO.

>
> > >
> > >  #define MAGIC_BYTES 123
> > >
> > > @@ -96,11 +112,11 @@ extern struct ipv6_packet pkt_v6;
> > >  #define _CHECK(condition, tag, duration, format...) ({                 \
> > >         int __ret = !!(condition);                                      \
> > >         if (__ret) {                                                    \
> > > -               error_cnt++;                                            \
> > > +               test__fail();                                           \
> > >                 printf("%s:FAIL:%s ", __func__, tag);                   \
> > >                 printf(format);                                         \
> > >         } else {                                                        \
> > > -               pass_cnt++;                                             \
> > > +               env.test->succ_cnt++;                                   \
> > >                 printf("%s:PASS:%s %d nsec\n",                          \
> > >                        __func__, tag, duration);                        \
> > >         }                                                               \
> > > --
> > > 2.23.0.rc1.153.gdeed80330f-goog
> > >

^ permalink raw reply

* Re: [PATCH net-next] mcast: ensure L-L IPv6 packets are accepted by bridge
From: Nikolay Aleksandrov @ 2019-08-14 20:34 UTC (permalink / raw)
  To: Linus Lüssing, Patrick Ruddy; +Cc: bridge, Ido Schimmel, netdev, roopa
In-Reply-To: <20190814201138.GE2431@otheros>

On 8/14/19 11:11 PM, Linus Lüssing wrote:
> On Wed, Aug 14, 2019 at 05:40:58PM +0100, Patrick Ruddy wrote:
>> The group is being joined by MLD at the L3 level but the packets are
>> not being passed up to the l3 interface becasue there is a MLD querier
>> on the network
>>
>> snippet from /proc/net/igmp6
>> ...
>> 40   sw1             ff0200000000000000000001ff008700     1 00000004 0
>> 40   sw1             ff020000000000000000000000000002     1 00000004 0
>> 40   sw1             ff020000000000000000000000000001     1 0000000C 0
>> 40   sw1             ff010000000000000000000000000001     1 00000008 0
>> 41   lo1             ff020000000000000000000000000001     1 0000000C 0
>> 41   lo1             ff010000000000000000000000000001     1 00000008 0
>> 42   sw1.1           ff020000000000000000000000000006     1 00000004 0
>> 42   sw1.1           ff020000000000000000000000000005     1 00000004 0
>> 42   sw1.1           ff0200000000000000000001ff000000     2 00000004 0
>> 42   sw1.1           ff0200000000000000000001ff008700     1 00000004 0
>> 42   sw1.1           ff0200000000000000000001ff000099     1 00000004 0
>> 42   sw1.1           ff020000000000000000000000000002     1 00000004 0
>> 42   sw1.1           ff020000000000000000000000000001     1 0000000C 0
>> 42   sw1.1           ff010000000000000000000000000001     1 00000008 0
>> ...
>>
>> the bridge is sw1 and the l3 intervace is sw1.1
> 
> What kind of interface is sw1.1 exactly? Is it a VLAN or a VRF
> interface? Something else?
> 
+1

> Could you also post the output of bridge mdb show?
> 
> Regards, Linus
> 
> 
> PS: Also please include the bridge mailinglist in the future.
> 

Note that if you'd like to debug a host joined group currently bridge mdb show
will not dump it and if the group is host-joined only it
can even be empty. You can use my latest set (not applied yet):
http://patchwork.ozlabs.org/project/netdev/list/?series=125169

Alternatively you could monitor the mdb events, it will show up there even
today without any changes (bridge monitor mdb) and you can check if it's
getting deleted.

Cheers,
 Nik

^ permalink raw reply

* RE: [PATCH] hv_netvsc: Fix a memory leak bug
From: Haiyang Zhang @ 2019-08-14 20:35 UTC (permalink / raw)
  To: Wenwen Wang
  Cc: KY Srinivasan, Stephen Hemminger, Sasha Levin, David S. Miller,
	open list:Hyper-V CORE AND DRIVERS, open list:NETWORKING DRIVERS,
	open list
In-Reply-To: <1565813771-8967-1-git-send-email-wenwen@cs.uga.edu>



> -----Original Message-----
> From: Wenwen Wang <wenwen@cs.uga.edu>
> Sent: Wednesday, August 14, 2019 4:16 PM
> To: Wenwen Wang <wenwen@cs.uga.edu>
> Cc: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; Stephen Hemminger
> <sthemmin@microsoft.com>; Sasha Levin <sashal@kernel.org>; David S.
> Miller <davem@davemloft.net>; open list:Hyper-V CORE AND DRIVERS
> <linux-hyperv@vger.kernel.org>; open list:NETWORKING DRIVERS
> <netdev@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>
> Subject: [PATCH] hv_netvsc: Fix a memory leak bug
> 
> In rndis_filter_device_add(), 'rndis_device' is allocated through kzalloc()
> by invoking get_rndis_device(). In the following execution, if an error
> occurs, the execution will go to the 'err_dev_remv' label. However, the
> allocated 'rndis_device' is not deallocated, leading to a memory leak bug.
> 
> Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
> ---
>  drivers/net/hyperv/rndis_filter.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/hyperv/rndis_filter.c
> b/drivers/net/hyperv/rndis_filter.c
> index 317dbe9..ed35085 100644
> --- a/drivers/net/hyperv/rndis_filter.c
> +++ b/drivers/net/hyperv/rndis_filter.c
> @@ -1420,6 +1420,7 @@ struct netvsc_device
> *rndis_filter_device_add(struct hv_device *dev,
> 
>  err_dev_remv:
>  	rndis_filter_device_remove(dev, net_device);
> +	kfree(rndis_device);

The kfree() is not necessary here. 
Because it is already freed by --
rndis_filter_device_remove() --> netvsc_device_remove() 
--> free_netvsc_device_rcu() --> free_netvsc_device()
--> kfree(nvdev->extension);  //This frees rndis_device.

Thanks,
- Haiyang

^ permalink raw reply

* Re: [PATCH] hv_netvsc: Fix a memory leak bug
From: Stephen Hemminger @ 2019-08-14 20:37 UTC (permalink / raw)
  To: Wenwen Wang
  Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
	David S. Miller, open list:Hyper-V CORE AND DRIVERS,
	open list:NETWORKING DRIVERS, open list
In-Reply-To: <1565813771-8967-1-git-send-email-wenwen@cs.uga.edu>

On Wed, 14 Aug 2019 15:16:11 -0500
Wenwen Wang <wenwen@cs.uga.edu> wrote:

> In rndis_filter_device_add(), 'rndis_device' is allocated through kzalloc()
> by invoking get_rndis_device(). In the following execution, if an error
> occurs, the execution will go to the 'err_dev_remv' label. However, the
> allocated 'rndis_device' is not deallocated, leading to a memory leak bug.
> 
> Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
> ---
>  drivers/net/hyperv/rndis_filter.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
> index 317dbe9..ed35085 100644
> --- a/drivers/net/hyperv/rndis_filter.c
> +++ b/drivers/net/hyperv/rndis_filter.c
> @@ -1420,6 +1420,7 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
>  
>  err_dev_remv:
>  	rndis_filter_device_remove(dev, net_device);
> +	kfree(rndis_device);
>  	return ERR_PTR(ret);
>  }
>  

The rndis_device is already freed by:

rndis_filter_device_remove
	netvsc_device_remove
		free_netvsc_device_rcu

free_netvsc_device called by rcu

static void free_netvsc_device(struct rcu_head *head)
{
	struct netvsc_device *nvdev
		= container_of(head, struct netvsc_device, rcu);
	int i;

	kfree(nvdev->extension);  << here

^ permalink raw reply

* Re: [PATCH net-next, 3/6] net/mlx5: Add wrappers for HyperV PCIe operations
From: Mark Bloch @ 2019-08-14 20:41 UTC (permalink / raw)
  To: haiyangz, sashal@kernel.org, davem@davemloft.net, Saeed Mahameed,
	leon@kernel.org, Eran Ben Elisha, lorenzo.pieralisi@arm.com,
	bhelgaas@google.com, linux-pci@vger.kernel.org,
	linux-hyperv@vger.kernel.org, netdev@vger.kernel.org
  Cc: kys, Stephen Hemminger, linux-kernel@vger.kernel.org
In-Reply-To: <1565809632-39138-4-git-send-email-haiyangz@microsoft.com>



On 8/14/19 12:08 PM, Haiyang Zhang wrote:
> From: Eran Ben Elisha <eranbe@mellanox.com>
> 
> Add wrapper functions for HyperV PCIe read / write /
> block_invalidate_register operations.  This will be used as an
> infrastructure in the downstream patch for software communication.
> 
> This will be enabled by default if CONFIG_PCI_HYPERV_MINI is set.
> 
> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/Makefile |  1 +
>  drivers/net/ethernet/mellanox/mlx5/core/lib/hv.c | 64 ++++++++++++++++++++++++
>  drivers/net/ethernet/mellanox/mlx5/core/lib/hv.h | 22 ++++++++
>  3 files changed, 87 insertions(+)
>  create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/lib/hv.c
>  create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/lib/hv.h
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
> index 8b7edaa..a8950b1 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
> @@ -45,6 +45,7 @@ mlx5_core-$(CONFIG_MLX5_ESWITCH)   += eswitch.o eswitch_offloads.o eswitch_offlo
>  mlx5_core-$(CONFIG_MLX5_MPFS)      += lib/mpfs.o
>  mlx5_core-$(CONFIG_VXLAN)          += lib/vxlan.o
>  mlx5_core-$(CONFIG_PTP_1588_CLOCK) += lib/clock.o
> +mlx5_core-$(CONFIG_PCI_HYPERV_MINI)     += lib/hv.o
>  
>  #
>  # Ipoib netdev
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv.c
> new file mode 100644
> index 0000000..cf08d02
> --- /dev/null
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv.c
> @@ -0,0 +1,64 @@
> +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
> +// Copyright (c) 2018 Mellanox Technologies
> +
> +#include <linux/hyperv.h>
> +#include "mlx5_core.h"
> +#include "lib/hv.h"
> +
> +static int mlx5_hv_config_common(struct mlx5_core_dev *dev, void *buf, int len,
> +				 int offset, bool read)
> +{
> +	int rc = -EOPNOTSUPP;
> +	int bytes_returned;
> +	int block_id;
> +
> +	if (offset % HV_CONFIG_BLOCK_SIZE_MAX || len % HV_CONFIG_BLOCK_SIZE_MAX)
> +		return -EINVAL;
> +
> +	block_id = offset / HV_CONFIG_BLOCK_SIZE_MAX;
> +
> +	rc = read ?
> +	     hyperv_read_cfg_blk(dev->pdev, buf,
> +				 HV_CONFIG_BLOCK_SIZE_MAX, block_id,
> +				 &bytes_returned) :
> +	     hyperv_write_cfg_blk(dev->pdev, buf,
> +				  HV_CONFIG_BLOCK_SIZE_MAX, block_id);
> +
> +	/* Make sure len bytes were read successfully  */
> +	if (read)
> +		rc |= !(len == bytes_returned);
> +
> +	if (rc) {
> +		mlx5_core_err(dev, "Failed to %s hv config, err = %d, len = %d, offset = %d\n",
> +			      read ? "read" : "write", rc, len,
> +			      offset);
> +		return rc;
> +	}
> +
> +	return 0;
> +}

This seems out of place why not expose this function as part of hyperv and mlx5
will just pass the pdev.

> +
> +int mlx5_hv_read_config(struct mlx5_core_dev *dev, void *buf, int len,
> +			int offset)
> +{
> +	return mlx5_hv_config_common(dev, buf, len, offset, true);
> +}
> +
> +int mlx5_hv_write_config(struct mlx5_core_dev *dev, void *buf, int len,
> +			 int offset)
> +{
> +	return mlx5_hv_config_common(dev, buf, len, offset, false);
> +}
> +
> +int mlx5_hv_register_invalidate(struct mlx5_core_dev *dev, void *context,
> +				void (*block_invalidate)(void *context,
> +							 u64 block_mask))
> +{
> +	return hyperv_reg_block_invalidate(dev->pdev, context,
> +					   block_invalidate);
> +}
> +
> +void mlx5_hv_unregister_invalidate(struct mlx5_core_dev *dev)
> +{
> +	hyperv_reg_block_invalidate(dev->pdev, NULL, NULL);
> +}
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv.h b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv.h
> new file mode 100644
> index 0000000..7f69771
> --- /dev/null
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
> +/* Copyright (c) 2019 Mellanox Technologies. */
> +
> +#ifndef __LIB_HV_H__
> +#define __LIB_HV_H__
> +
> +#if IS_ENABLED(CONFIG_PCI_HYPERV_MINI)
> +
> +#include <linux/hyperv.h>
> +#include <linux/mlx5/driver.h>
> +
> +int mlx5_hv_read_config(struct mlx5_core_dev *dev, void *buf, int len,
> +			int offset);
> +int mlx5_hv_write_config(struct mlx5_core_dev *dev, void *buf, int len,
> +			 int offset);
> +int mlx5_hv_register_invalidate(struct mlx5_core_dev *dev, void *context,
> +				void (*block_invalidate)(void *context,
> +							 u64 block_mask));
> +void mlx5_hv_unregister_invalidate(struct mlx5_core_dev *dev);
> +#endif
> +
> +#endif /* __LIB_HV_H__ */
> 

Mark

^ permalink raw reply

* Re: [PATCH net-next, 5/6] net/mlx5: Add HV VHCA control agent
From: Mark Bloch @ 2019-08-14 20:41 UTC (permalink / raw)
  To: haiyangz, sashal@kernel.org, davem@davemloft.net, Saeed Mahameed,
	leon@kernel.org, Eran Ben Elisha, lorenzo.pieralisi@arm.com,
	bhelgaas@google.com, linux-pci@vger.kernel.org,
	linux-hyperv@vger.kernel.org, netdev@vger.kernel.org
  Cc: kys, Stephen Hemminger, linux-kernel@vger.kernel.org
In-Reply-To: <1565809632-39138-6-git-send-email-haiyangz@microsoft.com>



On 8/14/19 12:09 PM, Haiyang Zhang wrote:
> From: Eran Ben Elisha <eranbe@mellanox.com>
> 
> Control agent is responsible over of the control block (ID 0). It should
> update the PF via this block about every capability change. In addition,
> upon block 0 invalidate, it should activate all other supported agents
> with data requests from the PF.
> 
> Upon agent create/destroy, the invalidate callback of the control agent
> is being called in order to update the PF driver about this change.
> 
> The control agent is an integral part of HV VHCA and will be created
> and destroy as part of the HV VHCA init/cleanup flow.
> 
> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
>  .../net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c  | 122 ++++++++++++++++++++-
>  .../net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h  |   1 +
>  2 files changed, 121 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
> index b2eebdf..3c7fffa 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
> @@ -110,22 +110,131 @@ void mlx5_hv_vhca_invalidate(void *context, u64 block_mask)
>  	queue_work(hv_vhca->work_queue, &work->invalidate_work);
>  }
>  
> +#define AGENT_MASK(type) (type ? BIT(type - 1) : 0 /* control */)
> +
> +static void mlx5_hv_vhca_agents_control(struct mlx5_hv_vhca *hv_vhca,
> +					struct mlx5_hv_vhca_control_block *block)
> +{
> +	int i;
> +
> +	for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++) {
> +		struct mlx5_hv_vhca_agent *agent = hv_vhca->agents[i];
> +
> +		if (!agent || !agent->control)
> +			continue;
> +
> +		if (!(AGENT_MASK(agent->type) & block->control))
> +			continue;
> +
> +		agent->control(agent, block);
> +	}
> +}
> +
> +static void mlx5_hv_vhca_capabilities(struct mlx5_hv_vhca *hv_vhca,
> +				      u32 *capabilities)
> +{
> +	int i;
> +
> +	for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++) {
> +		struct mlx5_hv_vhca_agent *agent = hv_vhca->agents[i];
> +
> +		if (agent)
> +			*capabilities |= AGENT_MASK(agent->type);
> +	}
> +}
> +
> +static void
> +mlx5_hv_vhca_control_agent_invalidate(struct mlx5_hv_vhca_agent *agent,
> +				      u64 block_mask)
> +{
> +	struct mlx5_hv_vhca *hv_vhca = agent->hv_vhca;
> +	struct mlx5_core_dev *dev = hv_vhca->dev;
> +	struct mlx5_hv_vhca_control_block *block;
> +	u32 capabilities = 0;
> +	int err;
> +
> +	block = kzalloc(sizeof(*block), GFP_KERNEL);
> +	if (!block)
> +		return;
> +
> +	err = mlx5_hv_read_config(dev, block, sizeof(*block), 0);
> +	if (err)
> +		goto free_block;
> +
> +	mlx5_hv_vhca_capabilities(hv_vhca, &capabilities);
> +
> +	/* In case no capabilities, send empty block in return */
> +	if (!capabilities) {
> +		memset(block, 0, sizeof(*block));
> +		goto write;
> +	}
> +
> +	if (block->capabilities != capabilities)
> +		block->capabilities = capabilities;
> +
> +	if (block->control & ~capabilities)
> +		goto free_block;
> +
> +	mlx5_hv_vhca_agents_control(hv_vhca, block);
> +	block->command_ack = block->command;
> +
> +write:
> +	mlx5_hv_write_config(dev, block, sizeof(*block), 0);
> +
> +free_block:
> +	kfree(block);
> +}
> +
> +static struct mlx5_hv_vhca_agent *
> +mlx5_hv_vhca_control_agent_create(struct mlx5_hv_vhca *hv_vhca)
> +{
> +	return mlx5_hv_vhca_agent_create(hv_vhca, MLX5_HV_VHCA_AGENT_CONTROL,
> +					 NULL,
> +					 mlx5_hv_vhca_control_agent_invalidate,
> +					 NULL, NULL);
> +}
> +
> +static void mlx5_hv_vhca_control_agent_destroy(struct mlx5_hv_vhca_agent *agent)
> +{
> +	mlx5_hv_vhca_agent_destroy(agent);
> +}
> +
>  int mlx5_hv_vhca_init(struct mlx5_hv_vhca *hv_vhca)
>  {
> +	struct mlx5_hv_vhca_agent *agent;
> +	int err;
> +
>  	if (IS_ERR_OR_NULL(hv_vhca))
>  		return IS_ERR_OR_NULL(hv_vhca);
>  
> -	return mlx5_hv_register_invalidate(hv_vhca->dev, hv_vhca,
> -					   mlx5_hv_vhca_invalidate);
> +	err = mlx5_hv_register_invalidate(hv_vhca->dev, hv_vhca,
> +					  mlx5_hv_vhca_invalidate);
> +	if (err)
> +		return err;
> +
> +	agent = mlx5_hv_vhca_control_agent_create(hv_vhca);
> +	if (IS_ERR_OR_NULL(agent)) {
> +		mlx5_hv_unregister_invalidate(hv_vhca->dev);
> +		return IS_ERR_OR_NULL(agent);
> +	}
> +
> +	hv_vhca->agents[MLX5_HV_VHCA_AGENT_CONTROL] = agent;
> +
> +	return 0;
>  }
>  
>  void mlx5_hv_vhca_cleanup(struct mlx5_hv_vhca *hv_vhca)
>  {
> +	struct mlx5_hv_vhca_agent *agent;
>  	int i;
>  
>  	if (IS_ERR_OR_NULL(hv_vhca))
>  		return;
>  
> +	agent = hv_vhca->agents[MLX5_HV_VHCA_AGENT_CONTROL];
> +	if (!IS_ERR_OR_NULL(agent))
> +		mlx5_hv_vhca_control_agent_destroy(agent);

Can the agent be err ptr here?

> +
>  	mutex_lock(&hv_vhca->agents_lock);
>  	for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++)
>  		WARN_ON(hv_vhca->agents[i]);

With the comment above in mind, here you check only for not null

> @@ -135,6 +244,11 @@ void mlx5_hv_vhca_cleanup(struct mlx5_hv_vhca *hv_vhca)
>  	mlx5_hv_unregister_invalidate(hv_vhca->dev);
>  }
>  
> +static void mlx5_hv_vhca_agents_update(struct mlx5_hv_vhca *hv_vhca)
> +{
> +	mlx5_hv_vhca_invalidate(hv_vhca, BIT(MLX5_HV_VHCA_AGENT_CONTROL));
> +}
> +
>  struct mlx5_hv_vhca_agent *
>  mlx5_hv_vhca_agent_create(struct mlx5_hv_vhca *hv_vhca,
>  			  enum mlx5_hv_vhca_agent_type type,
> @@ -168,6 +282,8 @@ struct mlx5_hv_vhca_agent *
>  	hv_vhca->agents[type] = agent;
>  	mutex_unlock(&hv_vhca->agents_lock);
>  
> +	mlx5_hv_vhca_agents_update(hv_vhca);
> +
>  	return agent;
>  }
>  
> @@ -189,6 +305,8 @@ void mlx5_hv_vhca_agent_destroy(struct mlx5_hv_vhca_agent *agent)
>  		agent->cleanup(agent);
>  
>  	kfree(agent);
> +
> +	mlx5_hv_vhca_agents_update(hv_vhca);
>  }
>  
>  static int mlx5_hv_vhca_data_block_prepare(struct mlx5_hv_vhca_agent *agent,
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
> index fa7ee85..6f4bfb1 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
> @@ -12,6 +12,7 @@
>  struct mlx5_hv_vhca_control_block;
>  
>  enum mlx5_hv_vhca_agent_type {
> +	MLX5_HV_VHCA_AGENT_CONTROL = 0,

No need to start value

>  	MLX5_HV_VHCA_AGENT_MAX = 32,
>  };
>  
> 

Mark

^ permalink raw reply

* Re: [PATCH net-next, 4/6] net/mlx5: Add HV VHCA infrastructure
From: Mark Bloch @ 2019-08-14 20:41 UTC (permalink / raw)
  To: haiyangz, sashal@kernel.org, davem@davemloft.net, Saeed Mahameed,
	leon@kernel.org, Eran Ben Elisha, lorenzo.pieralisi@arm.com,
	bhelgaas@google.com, linux-pci@vger.kernel.org,
	linux-hyperv@vger.kernel.org, netdev@vger.kernel.org
  Cc: kys, Stephen Hemminger, linux-kernel@vger.kernel.org
In-Reply-To: <1565809632-39138-5-git-send-email-haiyangz@microsoft.com>



On 8/14/19 12:08 PM, Haiyang Zhang wrote:
> From: Eran Ben Elisha <eranbe@mellanox.com>
> 
> HV VHCA is a layer which provides PF to VF communication channel based on
> HyperV PCI config channel. It implements Mellanox's Inter VHCA control
> communication protocol. The protocol contains control block in order to
> pass messages between the PF and VF drivers, and data blocks in order to
> pass actual data.
> 
> The infrastructure is agent based. Each agent will be responsible of
> contiguous buffer blocks in the VHCA config space. This infrastructure will
> bind agents to their blocks, and those agents can only access read/write
> the buffer blocks assigned to them. Each agent will provide three
> callbacks (control, invalidate, cleanup). Control will be invoked when
> block-0 is invalidated with a command that concerns this agent. Invalidate
> callback will be invoked if one of the blocks assigned to this agent was
> invalidated. Cleanup will be invoked before the agent is being freed in
> order to clean all of its open resources or deferred works.
> 
> Block-0 serves as the control block. All execution commands from the PF
> will be written by the PF over this block. VF will ack on those by
> writing on block-0 as well. Its format is described by struct
> mlx5_hv_vhca_control_block layout.
> 
> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/Makefile   |   2 +-
>  .../net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c  | 247 +++++++++++++++++++++
>  .../net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h  | 102 +++++++++
>  drivers/net/ethernet/mellanox/mlx5/core/main.c     |   7 +
>  include/linux/mlx5/driver.h                        |   2 +
>  5 files changed, 359 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
>  create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
> index a8950b1..e0a1056 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
> @@ -45,7 +45,7 @@ mlx5_core-$(CONFIG_MLX5_ESWITCH)   += eswitch.o eswitch_offloads.o eswitch_offlo
>  mlx5_core-$(CONFIG_MLX5_MPFS)      += lib/mpfs.o
>  mlx5_core-$(CONFIG_VXLAN)          += lib/vxlan.o
>  mlx5_core-$(CONFIG_PTP_1588_CLOCK) += lib/clock.o
> -mlx5_core-$(CONFIG_PCI_HYPERV_MINI)     += lib/hv.o
> +mlx5_core-$(CONFIG_PCI_HYPERV_MINI)+= lib/hv.o lib/hv_vhca.o
>  
>  #
>  # Ipoib netdev
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
> new file mode 100644
> index 0000000..b2eebdf
> --- /dev/null
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
> @@ -0,0 +1,247 @@
> +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
> +// Copyright (c) 2018 Mellanox Technologies
> +
> +#include <linux/hyperv.h>
> +#include "mlx5_core.h"
> +#include "lib/hv.h"
> +#include "lib/hv_vhca.h"
> +
> +struct mlx5_hv_vhca {
> +	struct mlx5_core_dev       *dev;
> +	struct workqueue_struct    *work_queue;
> +	struct mlx5_hv_vhca_agent  *agents[MLX5_HV_VHCA_AGENT_MAX];
> +	struct mutex                agents_lock; /* Protect agents array */
> +};
> +
> +struct mlx5_hv_vhca_work {
> +	struct work_struct     invalidate_work;
> +	struct mlx5_hv_vhca   *hv_vhca;
> +	u64                    block_mask;
> +};
> +
> +struct mlx5_hv_vhca_data_block {
> +	u16     sequence;
> +	u16     offset;
> +	u8      reserved[4];
> +	u64     data[15];
> +};
> +
> +struct mlx5_hv_vhca_agent {
> +	enum mlx5_hv_vhca_agent_type	 type;
> +	struct mlx5_hv_vhca		*hv_vhca;
> +	void				*priv;
> +	int                              seq;
Why is this int? and in data block is u16?

> +	void (*control)(struct mlx5_hv_vhca_agent *agent,
> +			struct mlx5_hv_vhca_control_block *block);
> +	void (*invalidate)(struct mlx5_hv_vhca_agent *agent,
> +			   u64 block_mask);
> +	void (*cleanup)(struct mlx5_hv_vhca_agent *agent);
> +};
> +
> +struct mlx5_hv_vhca *mlx5_hv_vhca_create(struct mlx5_core_dev *dev)
> +{
> +	struct mlx5_hv_vhca *hv_vhca = NULL;
> +
> +	hv_vhca = kzalloc(sizeof(*hv_vhca), GFP_KERNEL);
> +	if (!hv_vhca)
> +		return ERR_PTR(-ENOMEM);
> +
> +	hv_vhca->work_queue = create_singlethread_workqueue("mlx5_hv_vhca");
> +	if (!hv_vhca->work_queue) {
> +		kfree(hv_vhca);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	hv_vhca->dev = dev;
> +	mutex_init(&hv_vhca->agents_lock);
> +
> +	return hv_vhca;
> +}
> +
> +void mlx5_hv_vhca_destroy(struct mlx5_hv_vhca *hv_vhca)
> +{
> +	if (IS_ERR_OR_NULL(hv_vhca))
> +		return;
> +
> +	flush_workqueue(hv_vhca->work_queue);
> +	destroy_workqueue(hv_vhca->work_queue);

Why not just destroy?

> +	kfree(hv_vhca);
> +}
> +
> +static void mlx5_hv_vhca_invalidate_work(struct work_struct *work)
> +{
> +	struct mlx5_hv_vhca_work *hwork;
> +	struct mlx5_hv_vhca *hv_vhca;
> +	int i;
> +
> +	hwork = container_of(work, struct mlx5_hv_vhca_work, invalidate_work);
> +	hv_vhca = hwork->hv_vhca;
> +
> +	mutex_lock(&hv_vhca->agents_lock);
> +	for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++) {
> +		struct mlx5_hv_vhca_agent *agent = hv_vhca->agents[i];
> +
> +		if (!agent || !agent->invalidate)
> +			continue;
> +
> +		if (!(BIT(agent->type) & hwork->block_mask))
> +			continue;
> +
> +		agent->invalidate(agent, hwork->block_mask);
> +	}
> +	mutex_unlock(&hv_vhca->agents_lock);
> +
> +	kfree(hwork);
> +}
> +
> +void mlx5_hv_vhca_invalidate(void *context, u64 block_mask)
> +{
> +	struct mlx5_hv_vhca *hv_vhca = (struct mlx5_hv_vhca *)context;
> +	struct mlx5_hv_vhca_work *work;
> +
> +	work = kzalloc(sizeof(*work), GFP_ATOMIC);
> +	if (!work)
> +		return;
> +
> +	INIT_WORK(&work->invalidate_work, mlx5_hv_vhca_invalidate_work);
> +	work->hv_vhca    = hv_vhca;
> +	work->block_mask = block_mask;
> +
> +	queue_work(hv_vhca->work_queue, &work->invalidate_work);
> +}
> +
> +int mlx5_hv_vhca_init(struct mlx5_hv_vhca *hv_vhca)
> +{
> +	if (IS_ERR_OR_NULL(hv_vhca))
> +		return IS_ERR_OR_NULL(hv_vhca);
> +
> +	return mlx5_hv_register_invalidate(hv_vhca->dev, hv_vhca,
> +					   mlx5_hv_vhca_invalidate);
> +}
> +
> +void mlx5_hv_vhca_cleanup(struct mlx5_hv_vhca *hv_vhca)
> +{
> +	int i;
> +
> +	if (IS_ERR_OR_NULL(hv_vhca))
> +		return;
> +
> +	mutex_lock(&hv_vhca->agents_lock);
> +	for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++)
> +		WARN_ON(hv_vhca->agents[i]);
> +
> +	mutex_unlock(&hv_vhca->agents_lock);
> +
> +	mlx5_hv_unregister_invalidate(hv_vhca->dev);
> +}
> +
> +struct mlx5_hv_vhca_agent *
> +mlx5_hv_vhca_agent_create(struct mlx5_hv_vhca *hv_vhca,
> +			  enum mlx5_hv_vhca_agent_type type,
> +			  void (*control)(struct mlx5_hv_vhca_agent*,
> +					  struct mlx5_hv_vhca_control_block *block),
> +			  void (*invalidate)(struct mlx5_hv_vhca_agent*,
> +					     u64 block_mask),
> +			  void (*cleaup)(struct mlx5_hv_vhca_agent *agent),
> +			  void *priv)
> +{
> +	struct mlx5_hv_vhca_agent *agent;
> +
> +	if (IS_ERR_OR_NULL(hv_vhca))
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (hv_vhca->agents[type])
> +		return ERR_PTR(-EINVAL);> +
> +	agent = kzalloc(sizeof(*agent), GFP_KERNEL);
> +	if (!agent)
> +		return ERR_PTR(-ENOMEM);
> +
> +	agent->type      = type;
> +	agent->hv_vhca   = hv_vhca;
> +	agent->priv      = priv;
> +	agent->control   = control;
> +	agent->invalidate = invalidate;
> +	agent->cleanup   = cleaup;
> +
> +	mutex_lock(&hv_vhca->agents_lock);
> +	hv_vhca->agents[type] = agent;
> +	mutex_unlock(&hv_vhca->agents_lock);

You have a check for this not under a lock a few lines up,
but assign under a lock?

Mark

> +
> +	return agent;
> +}
> +
> +void mlx5_hv_vhca_agent_destroy(struct mlx5_hv_vhca_agent *agent)
> +{
> +	struct mlx5_hv_vhca *hv_vhca = agent->hv_vhca;
> +
> +	mutex_lock(&hv_vhca->agents_lock);
> +
> +	if (WARN_ON(agent != hv_vhca->agents[agent->type])) {
> +		mutex_unlock(&hv_vhca->agents_lock);
> +		return;
> +	}
> +
> +	hv_vhca->agents[agent->type] = NULL;
> +	mutex_unlock(&hv_vhca->agents_lock);
> +
> +	if (agent->cleanup)
> +		agent->cleanup(agent);
> +
> +	kfree(agent);
> +}
> +
> +static int mlx5_hv_vhca_data_block_prepare(struct mlx5_hv_vhca_agent *agent,
> +					   struct mlx5_hv_vhca_data_block *data_block,
> +					   void *src, int len, int *offset)
> +{
> +	int bytes = min_t(int, (int)sizeof(data_block->data), len);
> +
> +	data_block->sequence = agent->seq;
> +	data_block->offset   = (*offset)++;
> +	memcpy(data_block->data, src, bytes);
> +
> +	return bytes;
> +}
> +
> +static void mlx5_hv_vhca_agent_seq_update(struct mlx5_hv_vhca_agent *agent)
> +{
> +	agent->seq++;
> +}
> +
> +int mlx5_hv_vhca_agent_write(struct mlx5_hv_vhca_agent *agent,
> +			     void *buf, int len)
> +{
> +	int offset = agent->type * HV_CONFIG_BLOCK_SIZE_MAX;
> +	int block_offset = 0;
> +	int total = 0;
> +	int err;
> +
> +	while (len) {
> +		struct mlx5_hv_vhca_data_block data_block = {0};
> +		int bytes;
> +
> +		bytes = mlx5_hv_vhca_data_block_prepare(agent, &data_block,
> +							buf + total,
> +							len, &block_offset);
> +		if (!bytes)
> +			return -ENOMEM;
> +
> +		err = mlx5_hv_write_config(agent->hv_vhca->dev, &data_block,
> +					   sizeof(data_block), offset);
> +		if (err)
> +			return err;
> +
> +		total += bytes;
> +		len   -= bytes;
> +	}
> +
> +	mlx5_hv_vhca_agent_seq_update(agent);
> +
> +	return 0;
> +}
> +
> +void *mlx5_hv_vhca_agent_priv(struct mlx5_hv_vhca_agent *agent)
> +{
> +	return agent->priv;
> +}
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
> new file mode 100644
> index 0000000..fa7ee85
> --- /dev/null
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
> @@ -0,0 +1,102 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
> +/* Copyright (c) 2019 Mellanox Technologies. */
> +
> +#ifndef __LIB_HV_VHCA_H__
> +#define __LIB_HV_VHCA_H__
> +
> +#include "en.h"
> +#include "lib/hv.h"
> +
> +struct mlx5_hv_vhca_agent;
> +struct mlx5_hv_vhca;
> +struct mlx5_hv_vhca_control_block;
> +
> +enum mlx5_hv_vhca_agent_type {
> +	MLX5_HV_VHCA_AGENT_MAX = 32,
> +};
> +
> +#if IS_ENABLED(CONFIG_PCI_HYPERV_MINI)
> +
> +struct mlx5_hv_vhca_control_block {
> +	u32     capabilities;
> +	u32     control;
> +	u16     command;
> +	u16     command_ack;
> +	u16     version;
> +	u16     rings;
> +	u32     reserved1[28];
> +};
> +
> +struct mlx5_hv_vhca *mlx5_hv_vhca_create(struct mlx5_core_dev *dev);
> +void mlx5_hv_vhca_destroy(struct mlx5_hv_vhca *hv_vhca);
> +int mlx5_hv_vhca_init(struct mlx5_hv_vhca *hv_vhca);
> +void mlx5_hv_vhca_cleanup(struct mlx5_hv_vhca *hv_vhca);
> +void mlx5_hv_vhca_invalidate(void *context, u64 block_mask);
> +
> +struct mlx5_hv_vhca_agent *
> +mlx5_hv_vhca_agent_create(struct mlx5_hv_vhca *hv_vhca,
> +			  enum mlx5_hv_vhca_agent_type type,
> +			  void (*control)(struct mlx5_hv_vhca_agent*,
> +					  struct mlx5_hv_vhca_control_block *block),
> +			  void (*invalidate)(struct mlx5_hv_vhca_agent*,
> +					     u64 block_mask),
> +			  void (*cleanup)(struct mlx5_hv_vhca_agent *agent),
> +			  void *context);
> +
> +void mlx5_hv_vhca_agent_destroy(struct mlx5_hv_vhca_agent *agent);
> +int mlx5_hv_vhca_agent_write(struct mlx5_hv_vhca_agent *agent,
> +			     void *buf, int len);
> +void *mlx5_hv_vhca_agent_priv(struct mlx5_hv_vhca_agent *agent);
> +
> +#else
> +
> +static inline struct mlx5_hv_vhca *
> +mlx5_hv_vhca_create(struct mlx5_core_dev *dev)
> +{
> +	return NULL;
> +}
> +
> +static inline void mlx5_hv_vhca_destroy(struct mlx5_hv_vhca *hv_vhca)
> +{
> +}
> +
> +static inline int mlx5_hv_vhca_init(struct mlx5_hv_vhca *hv_vhca)
> +{
> +	return 0;
> +}
> +
> +static inline void mlx5_hv_vhca_cleanup(struct mlx5_hv_vhca *hv_vhca)
> +{
> +}
> +
> +static inline void mlx5_hv_vhca_invalidate(void *context,
> +					   u64 block_mask)
> +{
> +}
> +
> +static inline struct mlx5_hv_vhca_agent *
> +mlx5_hv_vhca_agent_create(struct mlx5_hv_vhca *hv_vhca,
> +			  enum mlx5_hv_vhca_agent_type type,
> +			  void (*control)(struct mlx5_hv_vhca_agent*,
> +					  struct mlx5_hv_vhca_control_block *block),
> +			  void (*invalidate)(struct mlx5_hv_vhca_agent*,
> +					     u64 block_mask),
> +			  void (*cleanup)(struct mlx5_hv_vhca_agent *agent),
> +			  void *context)
> +{
> +	return NULL;
> +}
> +
> +static inline void mlx5_hv_vhca_agent_destroy(struct mlx5_hv_vhca_agent *agent)
> +{
> +}
> +
> +static inline int
> +mlx5_hv_vhca_write_agent(struct mlx5_hv_vhca_agent *agent,
> +			 void *buf, int len)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#endif /* __LIB_HV_VHCA_H__ */
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> index 4cc90eb..50ee38b 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> @@ -69,6 +69,7 @@
>  #include "lib/pci_vsc.h"
>  #include "diag/fw_tracer.h"
>  #include "ecpf.h"
> +#include "lib/hv_vhca.h"
>  
>  MODULE_AUTHOR("Eli Cohen <eli@mellanox.com>");
>  MODULE_DESCRIPTION("Mellanox 5th generation network adapters (ConnectX series) core driver");
> @@ -872,6 +873,7 @@ static int mlx5_init_once(struct mlx5_core_dev *dev)
>  	}
>  
>  	dev->tracer = mlx5_fw_tracer_create(dev);
> +	dev->hv_vhca = mlx5_hv_vhca_create(dev);
>  
>  	return 0;
>  
> @@ -902,6 +904,7 @@ static int mlx5_init_once(struct mlx5_core_dev *dev)
>  
>  static void mlx5_cleanup_once(struct mlx5_core_dev *dev)
>  {
> +	mlx5_hv_vhca_destroy(dev->hv_vhca);
>  	mlx5_fw_tracer_destroy(dev->tracer);
>  	mlx5_fpga_cleanup(dev);
>  	mlx5_eswitch_cleanup(dev->priv.eswitch);
> @@ -1068,6 +1071,8 @@ static int mlx5_load(struct mlx5_core_dev *dev)
>  		goto err_fw_tracer;
>  	}
>  
> +	mlx5_hv_vhca_init(dev->hv_vhca);
> +
>  	err = mlx5_fpga_device_start(dev);
>  	if (err) {
>  		mlx5_core_err(dev, "fpga device start failed %d\n", err);
> @@ -1123,6 +1128,7 @@ static int mlx5_load(struct mlx5_core_dev *dev)
>  err_ipsec_start:
>  	mlx5_fpga_device_stop(dev);
>  err_fpga_start:
> +	mlx5_hv_vhca_cleanup(dev->hv_vhca);
>  	mlx5_fw_tracer_cleanup(dev->tracer);
>  err_fw_tracer:
>  	mlx5_eq_table_destroy(dev);
> @@ -1143,6 +1149,7 @@ static void mlx5_unload(struct mlx5_core_dev *dev)
>  	mlx5_accel_ipsec_cleanup(dev);
>  	mlx5_accel_tls_cleanup(dev);
>  	mlx5_fpga_device_stop(dev);
> +	mlx5_hv_vhca_cleanup(dev->hv_vhca);
>  	mlx5_fw_tracer_cleanup(dev->tracer);
>  	mlx5_eq_table_destroy(dev);
>  	mlx5_irq_table_destroy(dev);
> diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
> index 2b84ee9..97bb98c 100644
> --- a/include/linux/mlx5/driver.h
> +++ b/include/linux/mlx5/driver.h
> @@ -646,6 +646,7 @@ struct mlx5_clock {
>  struct mlx5_fw_tracer;
>  struct mlx5_vxlan;
>  struct mlx5_geneve;
> +struct mlx5_hv_vhca;
>  
>  struct mlx5_core_dev {
>  	struct device *device;
> @@ -693,6 +694,7 @@ struct mlx5_core_dev {
>  	struct mlx5_ib_clock_info  *clock_info;
>  	struct mlx5_fw_tracer   *tracer;
>  	u32                      vsc_addr;
> +	struct mlx5_hv_vhca	*hv_vhca;
>  };
>  
>  struct mlx5_db {
> 

^ permalink raw reply

* [PATCH v5 00/18] compat_ioctl.c removal, part 2/3
From: Arnd Bergmann @ 2019-08-14 20:42 UTC (permalink / raw)
  To: linux-kernel, viro, linux-fsdevel
  Cc: Arnd Bergmann, davem, axboe, linux-block, minyard, gregkh, linux,
	alexandre.belloni, jejb, martin.petersen, dgilbert, jslaby, wim,
	tytso, adilger.kernel, jaegeuk, rpeterso, agruenba, mikulas,
	konishi.ryusuke, jlbec, joseph.qi, darrick.wong, linux-xfs,
	netdev, openipmi-developer, linux-hwmon, linux-ppp, linux-rtc,
	linux-scsi, linux-watchdog, ecryptfs, linux-ext4,
	linux-f2fs-devel, cluster-devel, linux-nilfs, ocfs2-devel

This is a follow-up to part 1/3 that I posted after -rc2.
I hope these are still largely uncontroversial changes, and
I would like to get them into linux-5.4.

Part 1 was in

https://lore.kernel.org/lkml/CAPcyv4i_nHzV155RcgnAQ189aq2Lfd2g8pA1D5NbZqo9E_u+Dw@mail.gmail.com/

Part 3 will be one kernel release after part 2 is merged,
as that still needs a little extra work.

The entire series is available at

git://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git compat_ioctl

      Arnd

Al Viro (2):
  compat_ioctl: unify copy-in of ppp filters
  compat_ioctl: move PPPIOCSCOMPRESS to ppp_generic

Arnd Bergmann (16):
  xfs: compat_ioctl: use compat_ptr()
  xfs: compat_ioctl: add missing conversions
  gfs2: add compat_ioctl support
  fs: compat_ioctl: move FITRIM emulation into file systems
  watchdog: cpwd: use generic compat_ptr_ioctl
  compat_ioctl: move WDIOC handling into wdt drivers
  compat_ioctl: reimplement SG_IO handling
  af_unix: add compat_ioctl support
  compat_ioctl: handle SIOCOUTQNSD
  compat_ioctl: move SIOCOUTQ out of compat_ioctl.c
  tty: handle compat PPP ioctls
  compat_ioctl: handle PPPIOCGIDLE for 64-bit time_t
  compat_ioctl: ppp: move simple commands into ppp_generic.c
  compat_ioctl: move SG_GET_REQUEST_TABLE handling
  pktcdvd: add compat_ioctl handler
  scsi: sd: enable compat ioctls for sed-opal

 Documentation/networking/ppp_generic.txt  |   2 +
 arch/powerpc/platforms/52xx/mpc52xx_gpt.c |   1 +
 arch/um/drivers/harddog_kern.c            |   1 +
 block/scsi_ioctl.c                        | 132 ++++++++-
 drivers/block/pktcdvd.c                   |  25 ++
 drivers/char/ipmi/ipmi_watchdog.c         |   1 +
 drivers/hwmon/fschmd.c                    |   1 +
 drivers/net/ppp/ppp_generic.c             | 245 ++++++++++-----
 drivers/rtc/rtc-ds1374.c                  |   1 +
 drivers/scsi/sd.c                         |  14 +-
 drivers/scsi/sg.c                         |  59 +++-
 drivers/tty/tty_io.c                      |   5 +
 drivers/watchdog/acquirewdt.c             |   1 +
 drivers/watchdog/advantechwdt.c           |   1 +
 drivers/watchdog/alim1535_wdt.c           |   1 +
 drivers/watchdog/alim7101_wdt.c           |   1 +
 drivers/watchdog/ar7_wdt.c                |   1 +
 drivers/watchdog/at91rm9200_wdt.c         |   1 +
 drivers/watchdog/ath79_wdt.c              |   1 +
 drivers/watchdog/bcm63xx_wdt.c            |   1 +
 drivers/watchdog/cpu5wdt.c                |   1 +
 drivers/watchdog/cpwd.c                   |  25 +-
 drivers/watchdog/eurotechwdt.c            |   1 +
 drivers/watchdog/f71808e_wdt.c            |   1 +
 drivers/watchdog/gef_wdt.c                |   1 +
 drivers/watchdog/geodewdt.c               |   1 +
 drivers/watchdog/ib700wdt.c               |   1 +
 drivers/watchdog/ibmasr.c                 |   1 +
 drivers/watchdog/indydog.c                |   1 +
 drivers/watchdog/intel_scu_watchdog.c     |   1 +
 drivers/watchdog/iop_wdt.c                |   1 +
 drivers/watchdog/it8712f_wdt.c            |   1 +
 drivers/watchdog/ixp4xx_wdt.c             |   1 +
 drivers/watchdog/ks8695_wdt.c             |   1 +
 drivers/watchdog/m54xx_wdt.c              |   1 +
 drivers/watchdog/machzwd.c                |   1 +
 drivers/watchdog/mixcomwd.c               |   1 +
 drivers/watchdog/mtx-1_wdt.c              |   1 +
 drivers/watchdog/mv64x60_wdt.c            |   1 +
 drivers/watchdog/nuc900_wdt.c             |   1 +
 drivers/watchdog/nv_tco.c                 |   1 +
 drivers/watchdog/pc87413_wdt.c            |   1 +
 drivers/watchdog/pcwd.c                   |   1 +
 drivers/watchdog/pcwd_pci.c               |   1 +
 drivers/watchdog/pcwd_usb.c               |   1 +
 drivers/watchdog/pika_wdt.c               |   1 +
 drivers/watchdog/pnx833x_wdt.c            |   1 +
 drivers/watchdog/rc32434_wdt.c            |   1 +
 drivers/watchdog/rdc321x_wdt.c            |   1 +
 drivers/watchdog/riowd.c                  |   1 +
 drivers/watchdog/sa1100_wdt.c             |   1 +
 drivers/watchdog/sb_wdog.c                |   1 +
 drivers/watchdog/sbc60xxwdt.c             |   1 +
 drivers/watchdog/sbc7240_wdt.c            |   1 +
 drivers/watchdog/sbc_epx_c3.c             |   1 +
 drivers/watchdog/sbc_fitpc2_wdt.c         |   1 +
 drivers/watchdog/sc1200wdt.c              |   1 +
 drivers/watchdog/sc520_wdt.c              |   1 +
 drivers/watchdog/sch311x_wdt.c            |   1 +
 drivers/watchdog/scx200_wdt.c             |   1 +
 drivers/watchdog/smsc37b787_wdt.c         |   1 +
 drivers/watchdog/w83877f_wdt.c            |   1 +
 drivers/watchdog/w83977f_wdt.c            |   1 +
 drivers/watchdog/wafer5823wdt.c           |   1 +
 drivers/watchdog/watchdog_dev.c           |   1 +
 drivers/watchdog/wdrtas.c                 |   1 +
 drivers/watchdog/wdt.c                    |   1 +
 drivers/watchdog/wdt285.c                 |   1 +
 drivers/watchdog/wdt977.c                 |   1 +
 drivers/watchdog/wdt_pci.c                |   1 +
 fs/compat_ioctl.c                         | 346 +---------------------
 fs/ecryptfs/file.c                        |   1 +
 fs/ext4/ioctl.c                           |   1 +
 fs/f2fs/file.c                            |   1 +
 fs/gfs2/file.c                            |  24 ++
 fs/hpfs/dir.c                             |   1 +
 fs/hpfs/file.c                            |   1 +
 fs/nilfs2/ioctl.c                         |   1 +
 fs/ocfs2/ioctl.c                          |   1 +
 fs/xfs/xfs_ioctl32.c                      |  11 +-
 include/linux/blkdev.h                    |   2 +
 include/uapi/linux/ppp-ioctl.h            |   2 +
 include/uapi/linux/ppp_defs.h             |  14 +
 lib/iov_iter.c                            |   1 +
 net/socket.c                              |   3 +
 net/unix/af_unix.c                        |  19 ++
 86 files changed, 526 insertions(+), 472 deletions(-)

-- 
2.20.0

Cc: davem@davemloft.net
Cc: axboe@kernel.dk
Cc: linux-block@vger.kernel.org
Cc: minyard@acm.org
Cc: gregkh@linuxfoundation.org
Cc: linux@roeck-us.net
Cc: alexandre.belloni@bootlin.com
Cc: jejb@linux.ibm.com
Cc: martin.petersen@oracle.com
Cc: dgilbert@interlog.com
Cc: jslaby@suse.com
Cc: wim@linux-watchdog.org
Cc: viro@zeniv.linux.org.uk
Cc: tytso@mit.edu
Cc: adilger.kernel@dilger.ca
Cc: jaegeuk@kernel.org
Cc: rpeterso@redhat.com
Cc: agruenba@redhat.com
Cc: mikulas@artax.karlin.mff.cuni.cz
Cc: konishi.ryusuke@gmail.com
Cc: jlbec@evilplan.org
Cc: joseph.qi@linux.alibaba.com
Cc: darrick.wong@oracle.com
Cc: linux-xfs@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: openipmi-developer@lists.sourceforge.net
Cc: linux-hwmon@vger.kernel.org
Cc: linux-ppp@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-watchdog@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: ecryptfs@vger.kernel.org
Cc: linux-ext4@vger.kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: cluster-devel@redhat.com
Cc: linux-nilfs@vger.kernel.org
Cc: ocfs2-devel@oss.oracle.com

^ permalink raw reply

* Re: tc - mirred ingress not supported at the moment
From: Cong Wang @ 2019-08-14 20:51 UTC (permalink / raw)
  To: Martin Olsson; +Cc: netdev
In-Reply-To: <CAAT+qEYG5=5ny+t0VcqiYjDUQLrcj9sBR=2w-fdsE7Jjf4xOkQ@mail.gmail.com>

On Wed, Aug 14, 2019 at 2:02 AM Martin Olsson
<martin.olsson+netdev@sentorsecurity.com> wrote:
>
> Hi Cong!
>
> Ah sorry.
> Already implemented. Great!
>
> Hmmm. Then why don't the manual at https://www.linux.org/docs/man8/tc-mirred.html to reflect the changes?
> That was the place I checked to see if ingress was still not implemented.
> In the commit you point at, the sentence "Currently only egress is implemented" has been removed.


This means that website is out-of-date, not sync'ed with latest man pages.


>
>
> Question:
> Is there any form of performance penalty if I send the mirrored traffic to the ingress queue of the destination interface rather than to the egress queue?
> I mean, in the kernel there is the possibility to perform far more actions on the ingress queue than on the egress, but if I leave both queues at their defaults, will mirrored packets to ingress use more CPU cycles than to the egress destination, or are they more or less identical?
>

Hard to say without measurement. There is no queue on ingress
side, by the way, so it could be faster than egress, regarding to
lock contentions on queues.

>
> Question 2:
> Given the commit https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=5eca0a3701223619a513c7209f7d9335ca1b4cfa, how can I see in what kernel version it was added?
>

The kernel commit is:

commit 53592b3640019f2834701093e38272fdfd367ad8
Author: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Date:   Thu Oct 13 09:06:44 2016 +0300

    net/sched: act_mirred: Implement ingress actions

which is merged in 4.10.

Thanks.

^ permalink raw reply

* [PATCH v5 08/18] af_unix: add compat_ioctl support
From: Arnd Bergmann @ 2019-08-14 20:49 UTC (permalink / raw)
  To: linux-kernel, viro, linux-fsdevel, David S. Miller
  Cc: Arnd Bergmann, Andrei Vagin, Paolo Abeni, Karsten Graul, netdev
In-Reply-To: <20190814204259.120942-1-arnd@arndb.de>

The af_unix protocol family has a custom ioctl command (inexplicibly
based on SIOCPROTOPRIVATE), but never had a compat_ioctl handler for
32-bit applications.

Since all commands are compatible here, add a trivial wrapper that
performs the compat_ptr() conversion for SIOCOUTQ/SIOCINQ.  SIOCUNIXFILE
does not use the argument, but it doesn't hurt to also use compat_ptr()
here.

Fixes: ba94f3088b79 ("unix: add ioctl to open a unix socket file with O_PATH")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/unix/af_unix.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 67e87db5877f..e18ca6d9f3d4 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -646,6 +646,9 @@ static __poll_t unix_poll(struct file *, struct socket *, poll_table *);
 static __poll_t unix_dgram_poll(struct file *, struct socket *,
 				    poll_table *);
 static int unix_ioctl(struct socket *, unsigned int, unsigned long);
+#ifdef CONFIG_COMPAT
+static int unix_compat_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
+#endif
 static int unix_shutdown(struct socket *, int);
 static int unix_stream_sendmsg(struct socket *, struct msghdr *, size_t);
 static int unix_stream_recvmsg(struct socket *, struct msghdr *, size_t, int);
@@ -687,6 +690,9 @@ static const struct proto_ops unix_stream_ops = {
 	.getname =	unix_getname,
 	.poll =		unix_poll,
 	.ioctl =	unix_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl =	unix_compat_ioctl,
+#endif
 	.listen =	unix_listen,
 	.shutdown =	unix_shutdown,
 	.setsockopt =	sock_no_setsockopt,
@@ -710,6 +716,9 @@ static const struct proto_ops unix_dgram_ops = {
 	.getname =	unix_getname,
 	.poll =		unix_dgram_poll,
 	.ioctl =	unix_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl =	unix_compat_ioctl,
+#endif
 	.listen =	sock_no_listen,
 	.shutdown =	unix_shutdown,
 	.setsockopt =	sock_no_setsockopt,
@@ -732,6 +741,9 @@ static const struct proto_ops unix_seqpacket_ops = {
 	.getname =	unix_getname,
 	.poll =		unix_dgram_poll,
 	.ioctl =	unix_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl =	unix_compat_ioctl,
+#endif
 	.listen =	unix_listen,
 	.shutdown =	unix_shutdown,
 	.setsockopt =	sock_no_setsockopt,
@@ -2582,6 +2594,13 @@ static int unix_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 	return err;
 }
 
+#ifdef CONFIG_COMPAT
+static int unix_compat_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
+{
+	return unix_ioctl(sock, cmd, (unsigned long)compat_ptr(arg));
+}
+#endif
+
 static __poll_t unix_poll(struct file *file, struct socket *sock, poll_table *wait)
 {
 	struct sock *sk = sock->sk;
-- 
2.20.0


^ permalink raw reply related

* [PATCH v5 09/18] compat_ioctl: handle SIOCOUTQNSD
From: Arnd Bergmann @ 2019-08-14 20:49 UTC (permalink / raw)
  To: linux-kernel, viro, linux-fsdevel, David S. Miller
  Cc: Arnd Bergmann, Mario Schuknecht, Steffen Sledz, Willem de Bruijn,
	Johannes Berg, Deepa Dinamani, netdev
In-Reply-To: <20190814204259.120942-1-arnd@arndb.de>

Unlike the normal SIOCOUTQ, SIOCOUTQNSD was never handled in compat
mode. Add it to the common socket compat handler along with similar
ones.

Fixes: 2f4e1b397097 ("tcp: ioctl type SIOCOUTQNSD returns amount of data not sent")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/socket.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/socket.c b/net/socket.c
index 6a9ab7a8b1d2..a60f48ab2130 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -3452,6 +3452,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
 	case SIOCSARP:
 	case SIOCGARP:
 	case SIOCDARP:
+	case SIOCOUTQNSD:
 	case SIOCATMARK:
 		return sock_do_ioctl(net, sock, cmd, arg);
 	}
-- 
2.20.0


^ permalink raw reply related

* [PATCH v5 10/18] compat_ioctl: move SIOCOUTQ out of compat_ioctl.c
From: Arnd Bergmann @ 2019-08-14 20:54 UTC (permalink / raw)
  To: linux-kernel, viro, linux-fsdevel, Greg Kroah-Hartman, Jiri Slaby,
	David S. Miller
  Cc: Arnd Bergmann, netdev
In-Reply-To: <20190814204259.120942-1-arnd@arndb.de>

All users of this call are in socket or tty code, so handling
it there means we can avoid the table entry in fs/compat_ioctl.c.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/tty/tty_io.c | 1 +
 fs/compat_ioctl.c    | 2 --
 net/socket.c         | 2 ++
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 566728fbaf3c..cee8b69c6f72 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2755,6 +2755,7 @@ static long tty_compat_ioctl(struct file *file, unsigned int cmd,
 	int retval = -ENOIOCTLCMD;
 
 	switch (cmd) {
+	case TIOCOUTQ:
 	case TIOCSTI:
 	case TIOCGWINSZ:
 	case TIOCSWINSZ:
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index f279e77df256..d537888f3660 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -198,8 +198,6 @@ static int ppp_scompress(struct file *file, unsigned int cmd,
 
 #define COMPATIBLE_IOCTL(cmd) XFORM((u32)cmd),
 static unsigned int ioctl_pointer[] = {
-/* Little t */
-COMPATIBLE_IOCTL(TIOCOUTQ)
 #ifdef CONFIG_BLOCK
 /* Big S */
 COMPATIBLE_IOCTL(SCSI_IOCTL_GET_IDLUN)
diff --git a/net/socket.c b/net/socket.c
index a60f48ab2130..371999a024fa 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -100,6 +100,7 @@
 #include <linux/if_tun.h>
 #include <linux/ipv6_route.h>
 #include <linux/route.h>
+#include <linux/termios.h>
 #include <linux/sockios.h>
 #include <net/busy_poll.h>
 #include <linux/errqueue.h>
@@ -3452,6 +3453,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
 	case SIOCSARP:
 	case SIOCGARP:
 	case SIOCDARP:
+	case SIOCOUTQ:
 	case SIOCOUTQNSD:
 	case SIOCATMARK:
 		return sock_do_ioctl(net, sock, cmd, arg);
-- 
2.20.0


^ permalink raw reply related

* [PATCH v5 12/18] compat_ioctl: unify copy-in of ppp filters
From: Arnd Bergmann @ 2019-08-14 20:54 UTC (permalink / raw)
  To: linux-kernel, viro, linux-fsdevel, Paul Mackerras,
	David S. Miller
  Cc: Arnd Bergmann, linux-ppp, netdev, bpf
In-Reply-To: <20190814204259.120942-1-arnd@arndb.de>

From: Al Viro <viro@zeniv.linux.org.uk>

Now that isdn4linux is gone, the is only one implementation of PPPIOCSPASS
and PPPIOCSACTIVE in ppp_generic.c, so this is where the compat_ioctl
support should be implemented.

The two commands are implemented in very similar ways, so introduce
new helpers to allow sharing between the two and between native and
compat mode.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
[arnd: rebased, and added changelog text]
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ppp/ppp_generic.c | 169 ++++++++++++++++++++++------------
 fs/compat_ioctl.c             |  37 --------
 2 files changed, 108 insertions(+), 98 deletions(-)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index a30e41a56085..e3f207767589 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -554,29 +554,58 @@ static __poll_t ppp_poll(struct file *file, poll_table *wait)
 }
 
 #ifdef CONFIG_PPP_FILTER
-static int get_filter(void __user *arg, struct sock_filter **p)
+static struct bpf_prog *get_filter(struct sock_fprog *uprog)
+{
+	struct sock_fprog_kern fprog;
+	struct bpf_prog *res = NULL;
+	int err;
+
+	if (!uprog->len)
+		return NULL;
+
+	/* uprog->len is unsigned short, so no overflow here */
+	fprog.len = uprog->len * sizeof(struct sock_filter);
+	fprog.filter = memdup_user(uprog->filter, fprog.len);
+	if (IS_ERR(fprog.filter))
+		return ERR_CAST(fprog.filter);
+
+	err = bpf_prog_create(&res, &fprog);
+	kfree(fprog.filter);
+
+	return err ? ERR_PTR(err) : res;
+}
+
+static struct bpf_prog *ppp_get_filter(struct sock_fprog __user *p)
 {
 	struct sock_fprog uprog;
-	struct sock_filter *code = NULL;
-	int len;
 
-	if (copy_from_user(&uprog, arg, sizeof(uprog)))
-		return -EFAULT;
+	if (copy_from_user(&uprog, p, sizeof(struct sock_fprog)))
+		return ERR_PTR(-EFAULT);
+	return get_filter(&uprog);
+}
 
-	if (!uprog.len) {
-		*p = NULL;
-		return 0;
-	}
+#ifdef CONFIG_COMPAT
+struct sock_fprog32 {
+	unsigned short len;
+	compat_caddr_t filter;
+};
 
-	len = uprog.len * sizeof(struct sock_filter);
-	code = memdup_user(uprog.filter, len);
-	if (IS_ERR(code))
-		return PTR_ERR(code);
+#define PPPIOCSPASS32		_IOW('t', 71, struct sock_fprog32)
+#define PPPIOCSACTIVE32		_IOW('t', 70, struct sock_fprog32)
 
-	*p = code;
-	return uprog.len;
+static struct bpf_prog *compat_ppp_get_filter(struct sock_fprog32 __user *p)
+{
+	struct sock_fprog32 uprog32;
+	struct sock_fprog uprog;
+
+	if (copy_from_user(&uprog32, p, sizeof(struct sock_fprog32)))
+		return ERR_PTR(-EFAULT);
+	uprog.len = uprog32.len;
+	uprog.filter = compat_ptr(uprog32.filter);
+	return get_filter(&uprog);
 }
-#endif /* CONFIG_PPP_FILTER */
+#endif
+#endif
 
 static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
@@ -753,55 +782,25 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 #ifdef CONFIG_PPP_FILTER
 	case PPPIOCSPASS:
-	{
-		struct sock_filter *code;
-
-		err = get_filter(argp, &code);
-		if (err >= 0) {
-			struct bpf_prog *pass_filter = NULL;
-			struct sock_fprog_kern fprog = {
-				.len = err,
-				.filter = code,
-			};
-
-			err = 0;
-			if (fprog.filter)
-				err = bpf_prog_create(&pass_filter, &fprog);
-			if (!err) {
-				ppp_lock(ppp);
-				if (ppp->pass_filter)
-					bpf_prog_destroy(ppp->pass_filter);
-				ppp->pass_filter = pass_filter;
-				ppp_unlock(ppp);
-			}
-			kfree(code);
-		}
-		break;
-	}
 	case PPPIOCSACTIVE:
 	{
-		struct sock_filter *code;
+		struct bpf_prog *filter = ppp_get_filter(argp);
+		struct bpf_prog **which;
 
-		err = get_filter(argp, &code);
-		if (err >= 0) {
-			struct bpf_prog *active_filter = NULL;
-			struct sock_fprog_kern fprog = {
-				.len = err,
-				.filter = code,
-			};
-
-			err = 0;
-			if (fprog.filter)
-				err = bpf_prog_create(&active_filter, &fprog);
-			if (!err) {
-				ppp_lock(ppp);
-				if (ppp->active_filter)
-					bpf_prog_destroy(ppp->active_filter);
-				ppp->active_filter = active_filter;
-				ppp_unlock(ppp);
-			}
-			kfree(code);
+		if (IS_ERR(filter)) {
+			err = PTR_ERR(filter);
+			break;
 		}
+		if (cmd == PPPIOCSPASS)
+			which = &ppp->pass_filter;
+		else
+			which = &ppp->active_filter;
+		ppp_lock(ppp);
+		if (*which)
+			bpf_prog_destroy(*which);
+		*which = filter;
+		ppp_unlock(ppp);
+		err = 0;
 		break;
 	}
 #endif /* CONFIG_PPP_FILTER */
@@ -827,6 +826,51 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	return err;
 }
 
+#ifdef CONFIG_COMPAT
+static long ppp_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	struct ppp_file *pf;
+	int err = -ENOIOCTLCMD;
+	void __user *argp = (void __user *)arg;
+
+	mutex_lock(&ppp_mutex);
+
+	pf = file->private_data;
+	if (pf && pf->kind == INTERFACE) {
+		struct ppp *ppp = PF_TO_PPP(pf);
+		switch (cmd) {
+#ifdef CONFIG_PPP_FILTER
+		case PPPIOCSPASS32:
+		case PPPIOCSACTIVE32:
+		{
+			struct bpf_prog *filter = compat_ppp_get_filter(argp);
+			struct bpf_prog **which;
+
+			if (IS_ERR(filter)) {
+				err = PTR_ERR(filter);
+				break;
+			}
+			if (cmd == PPPIOCSPASS32)
+				which = &ppp->pass_filter;
+			else
+				which = &ppp->active_filter;
+			ppp_lock(ppp);
+			if (*which)
+				bpf_prog_destroy(*which);
+			*which = filter;
+			ppp_unlock(ppp);
+			err = 0;
+			break;
+		}
+#endif /* CONFIG_PPP_FILTER */
+		}
+	}
+	mutex_unlock(&ppp_mutex);
+
+	return err;
+}
+#endif
+
 static int ppp_unattached_ioctl(struct net *net, struct ppp_file *pf,
 			struct file *file, unsigned int cmd, unsigned long arg)
 {
@@ -895,6 +939,9 @@ static const struct file_operations ppp_device_fops = {
 	.write		= ppp_write,
 	.poll		= ppp_poll,
 	.unlocked_ioctl	= ppp_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= ppp_compat_ioctl,
+#endif
 	.open		= ppp_open,
 	.release	= ppp_release,
 	.llseek		= noop_llseek,
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index d537888f3660..eda41b2537f0 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -99,40 +99,6 @@ static int sg_grt_trans(struct file *file,
 }
 #endif /* CONFIG_BLOCK */
 
-struct sock_fprog32 {
-	unsigned short	len;
-	compat_caddr_t	filter;
-};
-
-#define PPPIOCSPASS32	_IOW('t', 71, struct sock_fprog32)
-#define PPPIOCSACTIVE32	_IOW('t', 70, struct sock_fprog32)
-
-static int ppp_sock_fprog_ioctl_trans(struct file *file,
-		unsigned int cmd, struct sock_fprog32 __user *u_fprog32)
-{
-	struct sock_fprog __user *u_fprog64 = compat_alloc_user_space(sizeof(struct sock_fprog));
-	void __user *fptr64;
-	u32 fptr32;
-	u16 flen;
-
-	if (get_user(flen, &u_fprog32->len) ||
-	    get_user(fptr32, &u_fprog32->filter))
-		return -EFAULT;
-
-	fptr64 = compat_ptr(fptr32);
-
-	if (put_user(flen, &u_fprog64->len) ||
-	    put_user(fptr64, &u_fprog64->filter))
-		return -EFAULT;
-
-	if (cmd == PPPIOCSPASS32)
-		cmd = PPPIOCSPASS;
-	else
-		cmd = PPPIOCSACTIVE;
-
-	return do_ioctl(file, cmd, (unsigned long) u_fprog64);
-}
-
 struct ppp_option_data32 {
 	compat_caddr_t	ptr;
 	u32			length;
@@ -285,9 +251,6 @@ static long do_ioctl_trans(unsigned int cmd,
 		return ppp_gidle(file, cmd, argp);
 	case PPPIOCSCOMPRESS32:
 		return ppp_scompress(file, cmd, argp);
-	case PPPIOCSPASS32:
-	case PPPIOCSACTIVE32:
-		return ppp_sock_fprog_ioctl_trans(file, cmd, argp);
 #ifdef CONFIG_BLOCK
 	case SG_GET_REQUEST_TABLE:
 		return sg_grt_trans(file, cmd, argp);
-- 
2.20.0


^ permalink raw reply related

* [PATCH v5 13/18] compat_ioctl: move PPPIOCSCOMPRESS to ppp_generic
From: Arnd Bergmann @ 2019-08-14 20:54 UTC (permalink / raw)
  To: linux-kernel, viro, linux-fsdevel, Paul Mackerras,
	David S. Miller
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-ppp, netdev
In-Reply-To: <20190814204259.120942-1-arnd@arndb.de>

From: Al Viro <viro@zeniv.linux.org.uk>

Rather than using a compat_alloc_user_space() buffer, moving
this next to the native handler allows sharing most of
the code, leaving only the user copy portion distinct.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ppp/ppp_generic.c | 53 +++++++++++++++++++++++++----------
 fs/compat_ioctl.c             | 32 ---------------------
 2 files changed, 38 insertions(+), 47 deletions(-)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index e3f207767589..2ab67bad6224 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -270,7 +270,7 @@ static void ppp_mp_insert(struct ppp *ppp, struct sk_buff *skb);
 static struct sk_buff *ppp_mp_reconstruct(struct ppp *ppp);
 static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb);
 #endif /* CONFIG_PPP_MULTILINK */
-static int ppp_set_compress(struct ppp *ppp, unsigned long arg);
+static int ppp_set_compress(struct ppp *ppp, struct ppp_option_data *data);
 static void ppp_ccp_peek(struct ppp *ppp, struct sk_buff *skb, int inbound);
 static void ppp_ccp_closed(struct ppp *ppp);
 static struct compressor *find_compressor(int type);
@@ -708,9 +708,14 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		break;
 
 	case PPPIOCSCOMPRESS:
-		err = ppp_set_compress(ppp, arg);
+	{
+		struct ppp_option_data data;
+		if (copy_from_user(&data, argp, sizeof(data)))
+			err = -EFAULT;
+		else
+			err = ppp_set_compress(ppp, &data);
 		break;
-
+	}
 	case PPPIOCGUNIT:
 		if (put_user(ppp->file.index, p))
 			break;
@@ -827,6 +832,13 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 }
 
 #ifdef CONFIG_COMPAT
+struct ppp_option_data32 {
+	compat_uptr_t		ptr;
+	u32			length;
+	compat_int_t		transmit;
+};
+#define PPPIOCSCOMPRESS32	_IOW('t', 77, struct ppp_option_data32)
+
 static long ppp_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	struct ppp_file *pf;
@@ -863,6 +875,21 @@ static long ppp_compat_ioctl(struct file *file, unsigned int cmd, unsigned long
 			break;
 		}
 #endif /* CONFIG_PPP_FILTER */
+		case PPPIOCSCOMPRESS32:
+		{
+			struct ppp_option_data32 data32;
+			if (copy_from_user(&data32, argp, sizeof(data32))) {
+				err = -EFAULT;
+			} else {
+				struct ppp_option_data data = {
+					.ptr = compat_ptr(data32.ptr),
+					.length = data32.length,
+					.transmit = data32.transmit
+				};
+				err = ppp_set_compress(ppp, &data);
+			}
+			break;
+		}
 		}
 	}
 	mutex_unlock(&ppp_mutex);
@@ -2781,24 +2808,20 @@ ppp_output_wakeup(struct ppp_channel *chan)
 
 /* Process the PPPIOCSCOMPRESS ioctl. */
 static int
-ppp_set_compress(struct ppp *ppp, unsigned long arg)
+ppp_set_compress(struct ppp *ppp, struct ppp_option_data *data)
 {
-	int err;
+	int err = -EFAULT;
 	struct compressor *cp, *ocomp;
-	struct ppp_option_data data;
 	void *state, *ostate;
 	unsigned char ccp_option[CCP_MAX_OPTION_LENGTH];
 
-	err = -EFAULT;
-	if (copy_from_user(&data, (void __user *) arg, sizeof(data)))
-		goto out;
-	if (data.length > CCP_MAX_OPTION_LENGTH)
+	if (data->length > CCP_MAX_OPTION_LENGTH)
 		goto out;
-	if (copy_from_user(ccp_option, (void __user *) data.ptr, data.length))
+	if (copy_from_user(ccp_option, data->ptr, data->length))
 		goto out;
 
 	err = -EINVAL;
-	if (data.length < 2 || ccp_option[1] < 2 || ccp_option[1] > data.length)
+	if (data->length < 2 || ccp_option[1] < 2 || ccp_option[1] > data->length)
 		goto out;
 
 	cp = try_then_request_module(
@@ -2808,8 +2831,8 @@ ppp_set_compress(struct ppp *ppp, unsigned long arg)
 		goto out;
 
 	err = -ENOBUFS;
-	if (data.transmit) {
-		state = cp->comp_alloc(ccp_option, data.length);
+	if (data->transmit) {
+		state = cp->comp_alloc(ccp_option, data->length);
 		if (state) {
 			ppp_xmit_lock(ppp);
 			ppp->xstate &= ~SC_COMP_RUN;
@@ -2827,7 +2850,7 @@ ppp_set_compress(struct ppp *ppp, unsigned long arg)
 			module_put(cp->owner);
 
 	} else {
-		state = cp->decomp_alloc(ccp_option, data.length);
+		state = cp->decomp_alloc(ccp_option, data->length);
 		if (state) {
 			ppp_recv_lock(ppp);
 			ppp->rstate &= ~SC_DECOMP_RUN;
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index eda41b2537f0..0b5a732d7afd 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -99,13 +99,6 @@ static int sg_grt_trans(struct file *file,
 }
 #endif /* CONFIG_BLOCK */
 
-struct ppp_option_data32 {
-	compat_caddr_t	ptr;
-	u32			length;
-	compat_int_t		transmit;
-};
-#define PPPIOCSCOMPRESS32	_IOW('t', 77, struct ppp_option_data32)
-
 struct ppp_idle32 {
 	compat_time_t xmit_idle;
 	compat_time_t recv_idle;
@@ -133,29 +126,6 @@ static int ppp_gidle(struct file *file, unsigned int cmd,
 	return err;
 }
 
-static int ppp_scompress(struct file *file, unsigned int cmd,
-	struct ppp_option_data32 __user *odata32)
-{
-	struct ppp_option_data __user *odata;
-	__u32 data;
-	void __user *datap;
-
-	odata = compat_alloc_user_space(sizeof(*odata));
-
-	if (get_user(data, &odata32->ptr))
-		return -EFAULT;
-
-	datap = compat_ptr(data);
-	if (put_user(datap, &odata->ptr))
-		return -EFAULT;
-
-	if (copy_in_user(&odata->length, &odata32->length,
-			 sizeof(__u32) + sizeof(int)))
-		return -EFAULT;
-
-	return do_ioctl(file, PPPIOCSCOMPRESS, (unsigned long) odata);
-}
-
 /*
  * simple reversible transform to make our table more evenly
  * distributed after sorting.
@@ -249,8 +219,6 @@ static long do_ioctl_trans(unsigned int cmd,
 	switch (cmd) {
 	case PPPIOCGIDLE32:
 		return ppp_gidle(file, cmd, argp);
-	case PPPIOCSCOMPRESS32:
-		return ppp_scompress(file, cmd, argp);
 #ifdef CONFIG_BLOCK
 	case SG_GET_REQUEST_TABLE:
 		return sg_grt_trans(file, cmd, argp);
-- 
2.20.0


^ permalink raw reply related

* [PATCH v5 14/18] compat_ioctl: handle PPPIOCGIDLE for 64-bit time_t
From: Arnd Bergmann @ 2019-08-14 20:54 UTC (permalink / raw)
  To: linux-kernel, viro, linux-fsdevel, David S. Miller,
	Jonathan Corbet, Paul Mackerras
  Cc: Arnd Bergmann, netdev, linux-doc, linux-ppp
In-Reply-To: <20190814204259.120942-1-arnd@arndb.de>

The ppp_idle structure is defined in terms of __kernel_time_t, which is
defined as 'long' on all architectures, and this usage is not affected
by the y2038 problem since it transports a time interval rather than an
absolute time.

However, the ppp user space defines the same structure as time_t, which
may be 64-bit wide on new libc versions even on 32-bit architectures.

It's easy enough to just handle both possible structure layouts on
all architectures, to deal with the possibility that a user space ppp
implementation comes with its own ppp_idle structure definition, as well
as to document the fact that the driver is y2038-safe.

Doing this also avoids the need for a special compat mode translation,
since 32-bit and 64-bit kernels now support the same interfaces.  The old
32-bit structure is also available on native 64-bit architectures now,
but this is harmless.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 Documentation/networking/ppp_generic.txt |  2 ++
 drivers/net/ppp/ppp_generic.c            | 19 ++++++++++----
 fs/compat_ioctl.c                        | 32 ++----------------------
 include/uapi/linux/ppp-ioctl.h           |  2 ++
 include/uapi/linux/ppp_defs.h            | 14 +++++++++++
 5 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/Documentation/networking/ppp_generic.txt b/Documentation/networking/ppp_generic.txt
index 61daf4b39600..fd563aff5fc9 100644
--- a/Documentation/networking/ppp_generic.txt
+++ b/Documentation/networking/ppp_generic.txt
@@ -378,6 +378,8 @@ an interface unit are:
   CONFIG_PPP_FILTER option is enabled, the set of packets which reset
   the transmit and receive idle timers is restricted to those which
   pass the `active' packet filter.
+  Two versions of this command exist, to deal with user space
+  expecting times as either 32-bit or 64-bit time_t seconds.
 
 * PPPIOCSMAXCID sets the maximum connection-ID parameter (and thus the
   number of connection slots) for the TCP header compressor and
diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 2ab67bad6224..6b4e227cb002 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -612,7 +612,8 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	struct ppp_file *pf;
 	struct ppp *ppp;
 	int err = -EFAULT, val, val2, i;
-	struct ppp_idle idle;
+	struct ppp_idle32 idle32;
+	struct ppp_idle64 idle64;
 	struct npioctl npi;
 	int unit, cflags;
 	struct slcompress *vj;
@@ -735,10 +736,18 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		err = 0;
 		break;
 
-	case PPPIOCGIDLE:
-		idle.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
-		idle.recv_idle = (jiffies - ppp->last_recv) / HZ;
-		if (copy_to_user(argp, &idle, sizeof(idle)))
+	case PPPIOCGIDLE32:
+                idle32.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
+                idle32.recv_idle = (jiffies - ppp->last_recv) / HZ;
+                if (copy_to_user(argp, &idle32, sizeof(idle32)))
+			break;
+		err = 0;
+		break;
+
+	case PPPIOCGIDLE64:
+		idle64.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
+		idle64.recv_idle = (jiffies - ppp->last_recv) / HZ;
+		if (copy_to_user(argp, &idle64, sizeof(idle64)))
 			break;
 		err = 0;
 		break;
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 0b5a732d7afd..f97cf698cfdd 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -99,33 +99,6 @@ static int sg_grt_trans(struct file *file,
 }
 #endif /* CONFIG_BLOCK */
 
-struct ppp_idle32 {
-	compat_time_t xmit_idle;
-	compat_time_t recv_idle;
-};
-#define PPPIOCGIDLE32		_IOR('t', 63, struct ppp_idle32)
-
-static int ppp_gidle(struct file *file, unsigned int cmd,
-		struct ppp_idle32 __user *idle32)
-{
-	struct ppp_idle __user *idle;
-	__kernel_time_t xmit, recv;
-	int err;
-
-	idle = compat_alloc_user_space(sizeof(*idle));
-
-	err = do_ioctl(file, PPPIOCGIDLE, (unsigned long) idle);
-
-	if (!err) {
-		if (get_user(xmit, &idle->xmit_idle) ||
-		    get_user(recv, &idle->recv_idle) ||
-		    put_user(xmit, &idle32->xmit_idle) ||
-		    put_user(recv, &idle32->recv_idle))
-			err = -EFAULT;
-	}
-	return err;
-}
-
 /*
  * simple reversible transform to make our table more evenly
  * distributed after sorting.
@@ -192,7 +165,8 @@ COMPATIBLE_IOCTL(PPPIOCGDEBUG)
 COMPATIBLE_IOCTL(PPPIOCSDEBUG)
 /* PPPIOCSPASS is translated */
 /* PPPIOCSACTIVE is translated */
-/* PPPIOCGIDLE is translated */
+COMPATIBLE_IOCTL(PPPIOCGIDLE32)
+COMPATIBLE_IOCTL(PPPIOCGIDLE64)
 COMPATIBLE_IOCTL(PPPIOCNEWUNIT)
 COMPATIBLE_IOCTL(PPPIOCATTACH)
 COMPATIBLE_IOCTL(PPPIOCDETACH)
@@ -217,8 +191,6 @@ static long do_ioctl_trans(unsigned int cmd,
 	void __user *argp = compat_ptr(arg);
 
 	switch (cmd) {
-	case PPPIOCGIDLE32:
-		return ppp_gidle(file, cmd, argp);
 #ifdef CONFIG_BLOCK
 	case SG_GET_REQUEST_TABLE:
 		return sg_grt_trans(file, cmd, argp);
diff --git a/include/uapi/linux/ppp-ioctl.h b/include/uapi/linux/ppp-ioctl.h
index 88b5f9990320..7bd2a5a75348 100644
--- a/include/uapi/linux/ppp-ioctl.h
+++ b/include/uapi/linux/ppp-ioctl.h
@@ -104,6 +104,8 @@ struct pppol2tp_ioc_stats {
 #define PPPIOCGDEBUG	_IOR('t', 65, int)	/* Read debug level */
 #define PPPIOCSDEBUG	_IOW('t', 64, int)	/* Set debug level */
 #define PPPIOCGIDLE	_IOR('t', 63, struct ppp_idle) /* get idle time */
+#define PPPIOCGIDLE32	_IOR('t', 63, struct ppp_idle32) /* 32-bit times */
+#define PPPIOCGIDLE64	_IOR('t', 63, struct ppp_idle64) /* 64-bit times */
 #define PPPIOCNEWUNIT	_IOWR('t', 62, int)	/* create new ppp unit */
 #define PPPIOCATTACH	_IOW('t', 61, int)	/* attach to ppp unit */
 #define PPPIOCDETACH	_IOW('t', 60, int)	/* obsolete, do not use */
diff --git a/include/uapi/linux/ppp_defs.h b/include/uapi/linux/ppp_defs.h
index fff51b91b409..0039fa39a358 100644
--- a/include/uapi/linux/ppp_defs.h
+++ b/include/uapi/linux/ppp_defs.h
@@ -142,10 +142,24 @@ struct ppp_comp_stats {
 /*
  * The following structure records the time in seconds since
  * the last NP packet was sent or received.
+ *
+ * Linux implements both 32-bit and 64-bit time_t versions
+ * for compatibility with user space that defines ppp_idle
+ * based on the libc time_t.
  */
 struct ppp_idle {
     __kernel_time_t xmit_idle;	/* time since last NP packet sent */
     __kernel_time_t recv_idle;	/* time since last NP packet received */
 };
 
+struct ppp_idle32 {
+    __s32 xmit_idle;		/* time since last NP packet sent */
+    __s32 recv_idle;		/* time since last NP packet received */
+};
+
+struct ppp_idle64 {
+    __s64 xmit_idle;		/* time since last NP packet sent */
+    __s64 recv_idle;		/* time since last NP packet received */
+};
+
 #endif /* _UAPI_PPP_DEFS_H_ */
-- 
2.20.0


^ permalink raw reply related

* [PATCH v5 15/18] compat_ioctl: ppp: move simple commands into ppp_generic.c
From: Arnd Bergmann @ 2019-08-14 20:54 UTC (permalink / raw)
  To: linux-kernel, viro, linux-fsdevel, Paul Mackerras,
	David S. Miller
  Cc: Arnd Bergmann, linux-ppp, netdev
In-Reply-To: <20190814204259.120942-1-arnd@arndb.de>

All ppp commands that are not already handled in ppp_compat_ioctl()
are compatible, so they can now handled by calling the native
ppp_ioctl() directly.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ppp/ppp_generic.c |  4 ++++
 fs/compat_ioctl.c             | 32 --------------------------------
 2 files changed, 4 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 6b4e227cb002..ea1507c7c40e 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -903,6 +903,10 @@ static long ppp_compat_ioctl(struct file *file, unsigned int cmd, unsigned long
 	}
 	mutex_unlock(&ppp_mutex);
 
+	/* all other commands have compatible arguments */
+	if (err == -ENOIOCTLCMD)
+		err = ppp_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
+
 	return err;
 }
 #endif
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index f97cf698cfdd..3d127bb6357a 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -144,38 +144,6 @@ COMPATIBLE_IOCTL(SG_GET_REQUEST_TABLE)
 COMPATIBLE_IOCTL(SG_SET_KEEP_ORPHAN)
 COMPATIBLE_IOCTL(SG_GET_KEEP_ORPHAN)
 #endif
-/* PPP stuff */
-COMPATIBLE_IOCTL(PPPIOCGFLAGS)
-COMPATIBLE_IOCTL(PPPIOCSFLAGS)
-COMPATIBLE_IOCTL(PPPIOCGASYNCMAP)
-COMPATIBLE_IOCTL(PPPIOCSASYNCMAP)
-COMPATIBLE_IOCTL(PPPIOCGUNIT)
-COMPATIBLE_IOCTL(PPPIOCGRASYNCMAP)
-COMPATIBLE_IOCTL(PPPIOCSRASYNCMAP)
-COMPATIBLE_IOCTL(PPPIOCGMRU)
-COMPATIBLE_IOCTL(PPPIOCSMRU)
-COMPATIBLE_IOCTL(PPPIOCSMAXCID)
-COMPATIBLE_IOCTL(PPPIOCGXASYNCMAP)
-COMPATIBLE_IOCTL(PPPIOCSXASYNCMAP)
-COMPATIBLE_IOCTL(PPPIOCXFERUNIT)
-/* PPPIOCSCOMPRESS is translated */
-COMPATIBLE_IOCTL(PPPIOCGNPMODE)
-COMPATIBLE_IOCTL(PPPIOCSNPMODE)
-COMPATIBLE_IOCTL(PPPIOCGDEBUG)
-COMPATIBLE_IOCTL(PPPIOCSDEBUG)
-/* PPPIOCSPASS is translated */
-/* PPPIOCSACTIVE is translated */
-COMPATIBLE_IOCTL(PPPIOCGIDLE32)
-COMPATIBLE_IOCTL(PPPIOCGIDLE64)
-COMPATIBLE_IOCTL(PPPIOCNEWUNIT)
-COMPATIBLE_IOCTL(PPPIOCATTACH)
-COMPATIBLE_IOCTL(PPPIOCDETACH)
-COMPATIBLE_IOCTL(PPPIOCSMRRU)
-COMPATIBLE_IOCTL(PPPIOCCONNECT)
-COMPATIBLE_IOCTL(PPPIOCDISCONN)
-COMPATIBLE_IOCTL(PPPIOCATTCHAN)
-COMPATIBLE_IOCTL(PPPIOCGCHAN)
-COMPATIBLE_IOCTL(PPPIOCGL2TPSTATS)
 };
 
 /*
-- 
2.20.0


^ permalink raw reply related

* Re: [PATCH bpf-next] tools: bpftool: compile with $(EXTRA_WARNINGS)
From: Daniel Borkmann @ 2019-08-14 20:58 UTC (permalink / raw)
  To: Quentin Monnet, Alexei Starovoitov; +Cc: bpf, netdev, oss-drivers
In-Reply-To: <20190814113724.20884-1-quentin.monnet@netronome.com>

On 8/14/19 1:37 PM, Quentin Monnet wrote:
> Compile bpftool with $(EXTRA_WARNINGS), as defined in
> scripts/Makefile.include, and fix the new warnings produced.
> 
> Simply leave -Wswitch-enum out of the warning list, as we have several
> switch-case structures where it is not desirable to process all values
> of an enum.
> 
> Remove -Wshadow from the warnings we manually add to CFLAGS, as it is
> handled in $(EXTRA_WARNINGS).
> 
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Applied, thanks!

^ permalink raw reply

* [PATCH v3 bpf-next] libbpf: make libbpf.map source of truth for libbpf version
From: Andrii Nakryiko @ 2019-08-14 20:05 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel, rdna
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Currently libbpf version is specified in 2 places: libbpf.map and
Makefile. They easily get out of sync and it's very easy to update one,
but forget to update another one. In addition, Github projection of
libbpf has to maintain its own version which has to be remembered to be
kept in sync manually, which is very error-prone approach.

This patch makes libbpf.map a source of truth for libbpf version and
uses shell invocation to parse out correct full and major libbpf version
to use during build. Now we need to make sure that once new release
cycle starts, we need to add (initially) empty section to libbpf.map
with correct latest version.

This also will make it possible to keep Github projection consistent
with kernel sources version of libbpf by adopting similar parsing of
version from libbpf.map.

v2->v3:
- grep -o + sort -rV (Andrey);

v1->v2:
- eager version vars evaluation (Jakub);
- simplified version regex (Andrey);

Cc: Andrey Ignatov <rdna@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/Makefile   | 20 ++++++++------------
 tools/lib/bpf/libbpf.map |  3 +++
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 9312066a1ae3..148a27164189 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -1,9 +1,10 @@
 # SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
 # Most of this file is copied from tools/lib/traceevent/Makefile
 
-BPF_VERSION = 0
-BPF_PATCHLEVEL = 0
-BPF_EXTRAVERSION = 4
+LIBBPF_VERSION := $(shell \
+	grep -oE '^LIBBPF_([0-9.]+)' libbpf.map | \
+	sort -rV | head -n1 | cut -d'_' -f2)
+LIBBPF_MAJOR_VERSION := $(firstword $(subst ., ,$(LIBBPF_VERSION)))
 
 MAKEFLAGS += --no-print-directory
 
@@ -79,15 +80,9 @@ export prefix libdir src obj
 libdir_SQ = $(subst ','\'',$(libdir))
 libdir_relative_SQ = $(subst ','\'',$(libdir_relative))
 
-VERSION		= $(BPF_VERSION)
-PATCHLEVEL	= $(BPF_PATCHLEVEL)
-EXTRAVERSION	= $(BPF_EXTRAVERSION)
-
 OBJ		= $@
 N		=
 
-LIBBPF_VERSION	= $(BPF_VERSION).$(BPF_PATCHLEVEL).$(BPF_EXTRAVERSION)
-
 LIB_TARGET	= libbpf.a libbpf.so.$(LIBBPF_VERSION)
 LIB_FILE	= libbpf.a libbpf.so*
 PC_FILE		= libbpf.pc
@@ -178,10 +173,10 @@ $(BPF_IN): force elfdep bpfdep
 $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
 
 $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
-	$(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(VERSION) \
+	$(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
 				    -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
 	@ln -sf $(@F) $(OUTPUT)libbpf.so
-	@ln -sf $(@F) $(OUTPUT)libbpf.so.$(VERSION)
+	@ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
 
 $(OUTPUT)libbpf.a: $(BPF_IN)
 	$(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
@@ -257,7 +252,8 @@ config-clean:
 
 clean:
 	$(call QUIET_CLEAN, libbpf) $(RM) $(TARGETS) $(CXX_TEST_TARGET) \
-		*.o *~ *.a *.so *.so.$(VERSION) .*.d .*.cmd *.pc LIBBPF-CFLAGS
+		*.o *~ *.a *.so *.so.$(LIBBPF_MAJOR_VERSION) .*.d .*.cmd \
+		*.pc LIBBPF-CFLAGS
 	$(call QUIET_CLEAN, core-gen) $(RM) $(OUTPUT)FEATURE-DUMP.libbpf
 
 
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index f9d316e873d8..4e72df8e98ba 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -184,3 +184,6 @@ LIBBPF_0.0.4 {
 		perf_buffer__new_raw;
 		perf_buffer__poll;
 } LIBBPF_0.0.3;
+
+LIBBPF_0.0.5 {
+} LIBBPF_0.0.4;
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH net-next 1/5] RDS: Re-add pf/sol access via sysctl
From: David Miller @ 2019-08-14 21:31 UTC (permalink / raw)
  To: santosh.shilimkar; +Cc: dledford, gerd.rausch, netdev, linux-rdma, rds-devel
In-Reply-To: <53a6feaf-b48e-aadf-1bd7-4c82ddc36d1e@oracle.com>

From: santosh.shilimkar@oracle.com
Date: Wed, 14 Aug 2019 11:36:19 -0700

> On 8/14/19 11:21 AM, David Miller wrote:
>> From: santosh.shilimkar@oracle.com
>> Date: Wed, 14 Aug 2019 11:01:36 -0700
>> 
>>> Some of the application software was released before 2009 and ended up
>>> using these proc entries from downstream kernel. The newer lib/app
>>> using RDS don't use these. Unfortunately lot of customer still use
>>> Oracle 9, 10, 11 which were released before 2007 and run these apps
>>> on modern kernels.
>> So those apps are using proc entries that were never upstream...
>>
>> Sorry, this is completely and utterly inappropriate.
>> 
> Agree. Unfortunately one of the legacy application library didn't
> get upgraded even after the ports were registered with IANA.
> Oracle 11 is still very active release and hence this patch.
> 
> It is fine to drop $subject patch from this series.

The appropriate procedure is to resubmit the series with the patch
removed.

Thank you.

^ permalink raw reply

* Re: [PATCH] net: usbnet: fix a memory leak bug
From: Jack Pham @ 2019-08-14 21:32 UTC (permalink / raw)
  To: Wenwen Wang
  Cc: Oliver Neukum, David S. Miller,
	open list:USB "USBNET" DRIVER FRAMEWORK,
	open list:USB NETWORKING DRIVERS, open list
In-Reply-To: <1565804493-7758-1-git-send-email-wenwen@cs.uga.edu>

On Wed, Aug 14, 2019 at 12:41:33PM -0500, Wenwen Wang wrote:
> In usbnet_start_xmit(), 'urb->sg' is allocated through kmalloc_array() by
> invoking build_dma_sg(). Later on, if 'CONFIG_PM' is defined and the if
> branch is taken, the execution will go to the label 'deferred'. However,
> 'urb->sg' is not deallocated on this execution path, leading to a memory
> leak bug.
> 
> Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
> ---
>  drivers/net/usb/usbnet.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 72514c4..f17fafa 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -1433,6 +1433,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
>  		usb_anchor_urb(urb, &dev->deferred);
>  		/* no use to process more packets */
>  		netif_stop_queue(net);
> +		kfree(urb->sg);
>  		usb_put_urb(urb);

The URB itself is not getting freed here; it is merely added to the
anchor list and will be submitted later upon usbnet_resume(). Therefore
freeing the SG list is premature and incorrect, as it will get freed in
either the tx_complete/tx_done path or upon URB submission failure.

>  		spin_unlock_irqrestore(&dev->txq.lock, flags);
>  		netdev_dbg(dev->net, "Delaying transmission for resumption\n");

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH v3 bpf-next] libbpf: make libbpf.map source of truth for libbpf version
From: Andrey Ignatov @ 2019-08-14 21:33 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf@vger.kernel.org, netdev@vger.kernel.org, Alexei Starovoitov,
	daniel@iogearbox.net, andrii.nakryiko@gmail.com, Kernel Team
In-Reply-To: <20190814200548.623033-1-andriin@fb.com>

Andrii Nakryiko <andriin@fb.com> [Wed, 2019-08-14 13:57 -0700]:
> Currently libbpf version is specified in 2 places: libbpf.map and
> Makefile. They easily get out of sync and it's very easy to update one,
> but forget to update another one. In addition, Github projection of
> libbpf has to maintain its own version which has to be remembered to be
> kept in sync manually, which is very error-prone approach.
> 
> This patch makes libbpf.map a source of truth for libbpf version and
> uses shell invocation to parse out correct full and major libbpf version
> to use during build. Now we need to make sure that once new release
> cycle starts, we need to add (initially) empty section to libbpf.map
> with correct latest version.
> 
> This also will make it possible to keep Github projection consistent
> with kernel sources version of libbpf by adopting similar parsing of
> version from libbpf.map.
> 
> v2->v3:
> - grep -o + sort -rV (Andrey);
> 
> v1->v2:
> - eager version vars evaluation (Jakub);
> - simplified version regex (Andrey);

Acked-by: Andrey Ignatov <rdna@fb.com>


> Cc: Andrey Ignatov <rdna@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  tools/lib/bpf/Makefile   | 20 ++++++++------------
>  tools/lib/bpf/libbpf.map |  3 +++
>  2 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 9312066a1ae3..148a27164189 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -1,9 +1,10 @@
>  # SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
>  # Most of this file is copied from tools/lib/traceevent/Makefile
>  
> -BPF_VERSION = 0
> -BPF_PATCHLEVEL = 0
> -BPF_EXTRAVERSION = 4
> +LIBBPF_VERSION := $(shell \
> +	grep -oE '^LIBBPF_([0-9.]+)' libbpf.map | \
> +	sort -rV | head -n1 | cut -d'_' -f2)
> +LIBBPF_MAJOR_VERSION := $(firstword $(subst ., ,$(LIBBPF_VERSION)))
>  
>  MAKEFLAGS += --no-print-directory
>  
> @@ -79,15 +80,9 @@ export prefix libdir src obj
>  libdir_SQ = $(subst ','\'',$(libdir))
>  libdir_relative_SQ = $(subst ','\'',$(libdir_relative))
>  
> -VERSION		= $(BPF_VERSION)
> -PATCHLEVEL	= $(BPF_PATCHLEVEL)
> -EXTRAVERSION	= $(BPF_EXTRAVERSION)
> -
>  OBJ		= $@
>  N		=
>  
> -LIBBPF_VERSION	= $(BPF_VERSION).$(BPF_PATCHLEVEL).$(BPF_EXTRAVERSION)
> -
>  LIB_TARGET	= libbpf.a libbpf.so.$(LIBBPF_VERSION)
>  LIB_FILE	= libbpf.a libbpf.so*
>  PC_FILE		= libbpf.pc
> @@ -178,10 +173,10 @@ $(BPF_IN): force elfdep bpfdep
>  $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
>  
>  $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
> -	$(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(VERSION) \
> +	$(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
>  				    -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
>  	@ln -sf $(@F) $(OUTPUT)libbpf.so
> -	@ln -sf $(@F) $(OUTPUT)libbpf.so.$(VERSION)
> +	@ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
>  
>  $(OUTPUT)libbpf.a: $(BPF_IN)
>  	$(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
> @@ -257,7 +252,8 @@ config-clean:
>  
>  clean:
>  	$(call QUIET_CLEAN, libbpf) $(RM) $(TARGETS) $(CXX_TEST_TARGET) \
> -		*.o *~ *.a *.so *.so.$(VERSION) .*.d .*.cmd *.pc LIBBPF-CFLAGS
> +		*.o *~ *.a *.so *.so.$(LIBBPF_MAJOR_VERSION) .*.d .*.cmd \
> +		*.pc LIBBPF-CFLAGS
>  	$(call QUIET_CLEAN, core-gen) $(RM) $(OUTPUT)FEATURE-DUMP.libbpf
>  
>  
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index f9d316e873d8..4e72df8e98ba 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -184,3 +184,6 @@ LIBBPF_0.0.4 {
>  		perf_buffer__new_raw;
>  		perf_buffer__poll;
>  } LIBBPF_0.0.3;
> +
> +LIBBPF_0.0.5 {
> +} LIBBPF_0.0.4;
> -- 
> 2.17.1
> 

-- 
Andrey Ignatov

^ permalink raw reply

* Re: [PATCH] netfilter: nft_bitwise: Adjust parentheses to fix memcmp size argument
From: Pablo Neira Ayuso @ 2019-08-14 21:37 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Jozsef Kadlecsik, Florian Westphal, David S. Miller,
	netfilter-devel, coreteam, netdev, linux-kernel,
	clang-built-linux, kbuild test robot
In-Reply-To: <20190814165809.46421-1-natechancellor@gmail.com>

On Wed, Aug 14, 2019 at 09:58:09AM -0700, Nathan Chancellor wrote:
> clang warns:
> 
> net/netfilter/nft_bitwise.c:138:50: error: size argument in 'memcmp'
> call is a comparison [-Werror,-Wmemsize-comparison]
>         if (memcmp(&priv->xor, &zero, sizeof(priv->xor) ||
>                                       ~~~~~~~~~~~~~~~~~~^~

Applied, thanks.

^ permalink raw reply

* [PATCH 0/2] Netfilter updates for net-next
From: Pablo Neira Ayuso @ 2019-08-14 21:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Hi,

The following patchset contains Netfilter updates for net-next.
This round addresses fallout from previous pull request:

1) Remove #warning from ipt_LOG.h and ip6t_LOG.h headers,
   from Jeremy Sowden.

2) Incorrect parens in memcmp() in nft_bitwise, from Nathan Chancellor.

You can pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git

Thanks.

----------------------------------------------------------------

The following changes since commit 5181b473d64ee278f24035ce335b89ddc4520fc0:

  net: phy: realtek: add NBase-T PHY auto-detection (2019-08-14 13:26:08 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git HEAD

for you to fetch changes up to 83c156d3ecc0121d27dc2b7f34e829b265c70c4f:

  netfilter: nft_bitwise: Adjust parentheses to fix memcmp size argument (2019-08-14 23:36:45 +0200)

----------------------------------------------------------------
Jeremy Sowden (1):
      netfilter: remove deprecation warnings from uapi headers.

Nathan Chancellor (1):
      netfilter: nft_bitwise: Adjust parentheses to fix memcmp size argument

 include/uapi/linux/netfilter_ipv4/ipt_LOG.h  | 2 --
 include/uapi/linux/netfilter_ipv6/ip6t_LOG.h | 2 --
 net/netfilter/nft_bitwise.c                  | 4 ++--
 3 files changed, 2 insertions(+), 6 deletions(-)

^ permalink raw reply

* [PATCH 1/2] netfilter: remove deprecation warnings from uapi headers.
From: Pablo Neira Ayuso @ 2019-08-14 21:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190814214347.4940-1-pablo@netfilter.org>

From: Jeremy Sowden <jeremy@azazel.net>

There are two netfilter userspace headers which contain deprecation
warnings.  While these headers are not used within the kernel, they are
compiled stand-alone for header-testing.

Pablo informs me that userspace iptables still refer to these headers,
and the intention was to use xt_LOG.h instead and remove these, but
userspace was never updated.

Remove the warnings.

Fixes: 2a475c409fe8 ("kbuild: remove all netfilter headers from header-test blacklist.")
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/uapi/linux/netfilter_ipv4/ipt_LOG.h  | 2 --
 include/uapi/linux/netfilter_ipv6/ip6t_LOG.h | 2 --
 2 files changed, 4 deletions(-)

diff --git a/include/uapi/linux/netfilter_ipv4/ipt_LOG.h b/include/uapi/linux/netfilter_ipv4/ipt_LOG.h
index 6dec14ba851b..b7cf2c669f40 100644
--- a/include/uapi/linux/netfilter_ipv4/ipt_LOG.h
+++ b/include/uapi/linux/netfilter_ipv4/ipt_LOG.h
@@ -2,8 +2,6 @@
 #ifndef _IPT_LOG_H
 #define _IPT_LOG_H
 
-#warning "Please update iptables, this file will be removed soon!"
-
 /* make sure not to change this without changing netfilter.h:NF_LOG_* (!) */
 #define IPT_LOG_TCPSEQ		0x01	/* Log TCP sequence numbers */
 #define IPT_LOG_TCPOPT		0x02	/* Log TCP options */
diff --git a/include/uapi/linux/netfilter_ipv6/ip6t_LOG.h b/include/uapi/linux/netfilter_ipv6/ip6t_LOG.h
index 7553a434e4da..23e91a9c2583 100644
--- a/include/uapi/linux/netfilter_ipv6/ip6t_LOG.h
+++ b/include/uapi/linux/netfilter_ipv6/ip6t_LOG.h
@@ -2,8 +2,6 @@
 #ifndef _IP6T_LOG_H
 #define _IP6T_LOG_H
 
-#warning "Please update iptables, this file will be removed soon!"
-
 /* make sure not to change this without changing netfilter.h:NF_LOG_* (!) */
 #define IP6T_LOG_TCPSEQ		0x01	/* Log TCP sequence numbers */
 #define IP6T_LOG_TCPOPT		0x02	/* Log TCP options */
-- 
2.11.0


^ permalink raw reply related

* [PATCH 2/2] netfilter: nft_bitwise: Adjust parentheses to fix memcmp size argument
From: Pablo Neira Ayuso @ 2019-08-14 21:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190814214347.4940-1-pablo@netfilter.org>

From: Nathan Chancellor <natechancellor@gmail.com>

clang warns:

net/netfilter/nft_bitwise.c:138:50: error: size argument in 'memcmp'
call is a comparison [-Werror,-Wmemsize-comparison]
        if (memcmp(&priv->xor, &zero, sizeof(priv->xor) ||
                                      ~~~~~~~~~~~~~~~~~~^~
net/netfilter/nft_bitwise.c:138:6: note: did you mean to compare the
result of 'memcmp' instead?
        if (memcmp(&priv->xor, &zero, sizeof(priv->xor) ||
            ^
                                                       )
net/netfilter/nft_bitwise.c:138:32: note: explicitly cast the argument
to size_t to silence this warning
        if (memcmp(&priv->xor, &zero, sizeof(priv->xor) ||
                                      ^
                                      (size_t)(
1 error generated.

Adjust the parentheses so that the result of the sizeof is used for the
size argument in memcmp, rather than the result of the comparison (which
would always be true because sizeof is a non-zero number).

Fixes: bd8699e9e292 ("netfilter: nft_bitwise: add offload support")
Link: https://github.com/ClangBuiltLinux/linux/issues/638
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_bitwise.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nft_bitwise.c b/net/netfilter/nft_bitwise.c
index 1f04ed5c518c..974300178fa9 100644
--- a/net/netfilter/nft_bitwise.c
+++ b/net/netfilter/nft_bitwise.c
@@ -135,8 +135,8 @@ static int nft_bitwise_offload(struct nft_offload_ctx *ctx,
 {
 	const struct nft_bitwise *priv = nft_expr_priv(expr);
 
-	if (memcmp(&priv->xor, &zero, sizeof(priv->xor) ||
-	    priv->sreg != priv->dreg))
+	if (memcmp(&priv->xor, &zero, sizeof(priv->xor)) ||
+	    priv->sreg != priv->dreg)
 		return -EOPNOTSUPP;
 
 	memcpy(&ctx->regs[priv->dreg].mask, &priv->mask, sizeof(priv->mask));
-- 
2.11.0


^ permalink raw reply related


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