* [PATCH v5 bpf-next 2/2] bpf: adding tests for map_in_map helpber in libbpf
From: Nikita V. Shirokov @ 2018-11-21 4:55 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski
Cc: netdev, Nikita V. Shirokov
In-Reply-To: <20181121045557.17924-1-tehnerd@tehnerd.com>
adding test/example of bpf_map__set_inner_map_fd usage
Signed-off-by: Nikita V. Shirokov <tehnerd@tehnerd.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
tools/testing/selftests/bpf/Makefile | 3 +-
tools/testing/selftests/bpf/test_map_in_map.c | 49 +++++++++++++++
tools/testing/selftests/bpf/test_maps.c | 90 +++++++++++++++++++++++++++
3 files changed, 141 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/test_map_in_map.c
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 1dde03ea1484..43157bd89165 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -38,7 +38,8 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
test_lwt_seg6local.o sendmsg4_prog.o sendmsg6_prog.o test_lirc_mode2_kern.o \
get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o \
test_skb_cgroup_id_kern.o bpf_flow.o netcnt_prog.o \
- test_sk_lookup_kern.o test_xdp_vlan.o test_queue_map.o test_stack_map.o
+ test_sk_lookup_kern.o test_xdp_vlan.o test_queue_map.o test_stack_map.o \
+ test_map_in_map.o
# Order correspond to 'make run_tests' order
TEST_PROGS := test_kmod.sh \
diff --git a/tools/testing/selftests/bpf/test_map_in_map.c b/tools/testing/selftests/bpf/test_map_in_map.c
new file mode 100644
index 000000000000..ce923e67e08e
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_map_in_map.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018 Facebook */
+#include <stddef.h>
+#include <linux/bpf.h>
+#include <linux/types.h>
+#include "bpf_helpers.h"
+
+struct bpf_map_def SEC("maps") mim_array = {
+ .type = BPF_MAP_TYPE_ARRAY_OF_MAPS,
+ .key_size = sizeof(int),
+ /* must be sizeof(__u32) for map in map */
+ .value_size = sizeof(__u32),
+ .max_entries = 1,
+ .map_flags = 0,
+};
+
+struct bpf_map_def SEC("maps") mim_hash = {
+ .type = BPF_MAP_TYPE_HASH_OF_MAPS,
+ .key_size = sizeof(int),
+ /* must be sizeof(__u32) for map in map */
+ .value_size = sizeof(__u32),
+ .max_entries = 1,
+ .map_flags = 0,
+};
+
+SEC("xdp_mimtest")
+int xdp_mimtest0(struct xdp_md *ctx)
+{
+ int value = 123;
+ int key = 0;
+ void *map;
+
+ map = bpf_map_lookup_elem(&mim_array, &key);
+ if (!map)
+ return XDP_DROP;
+
+ bpf_map_update_elem(map, &key, &value, 0);
+
+ map = bpf_map_lookup_elem(&mim_hash, &key);
+ if (!map)
+ return XDP_DROP;
+
+ bpf_map_update_elem(map, &key, &value, 0);
+
+ return XDP_PASS;
+}
+
+int _version SEC("version") = 1;
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 9f0a5b16a246..9c79ee017df3 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -1125,6 +1125,94 @@ static void test_sockmap(int tasks, void *data)
exit(1);
}
+#define MAPINMAP_PROG "./test_map_in_map.o"
+static void test_map_in_map(void)
+{
+ struct bpf_program *prog;
+ struct bpf_object *obj;
+ struct bpf_map *map;
+ int mim_fd, fd, err;
+ int pos = 0;
+
+ obj = bpf_object__open(MAPINMAP_PROG);
+
+ fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(int), sizeof(int),
+ 2, 0);
+ if (fd < 0) {
+ printf("Failed to create hashmap '%s'!\n", strerror(errno));
+ exit(1);
+ }
+
+ map = bpf_object__find_map_by_name(obj, "mim_array");
+ if (IS_ERR(map)) {
+ printf("Failed to load array of maps from test prog\n");
+ goto out_map_in_map;
+ }
+ err = bpf_map__set_inner_map_fd(map, fd);
+ if (err) {
+ printf("Failed to set inner_map_fd for array of maps\n");
+ goto out_map_in_map;
+ }
+
+ map = bpf_object__find_map_by_name(obj, "mim_hash");
+ if (IS_ERR(map)) {
+ printf("Failed to load hash of maps from test prog\n");
+ goto out_map_in_map;
+ }
+ err = bpf_map__set_inner_map_fd(map, fd);
+ if (err) {
+ printf("Failed to set inner_map_fd for hash of maps\n");
+ goto out_map_in_map;
+ }
+
+ bpf_object__for_each_program(prog, obj) {
+ bpf_program__set_xdp(prog);
+ }
+ bpf_object__load(obj);
+
+ map = bpf_object__find_map_by_name(obj, "mim_array");
+ if (IS_ERR(map)) {
+ printf("Failed to load array of maps from test prog\n");
+ goto out_map_in_map;
+ }
+ mim_fd = bpf_map__fd(map);
+ if (mim_fd < 0) {
+ printf("Failed to get descriptor for array of maps\n");
+ goto out_map_in_map;
+ }
+
+ err = bpf_map_update_elem(mim_fd, &pos, &fd, 0);
+ if (err) {
+ printf("Failed to update array of maps\n");
+ goto out_map_in_map;
+ }
+
+ map = bpf_object__find_map_by_name(obj, "mim_hash");
+ if (IS_ERR(map)) {
+ printf("Failed to load hash of maps from test prog\n");
+ goto out_map_in_map;
+ }
+ mim_fd = bpf_map__fd(map);
+ if (mim_fd < 0) {
+ printf("Failed to get descriptor for hash of maps\n");
+ goto out_map_in_map;
+ }
+
+ err = bpf_map_update_elem(mim_fd, &pos, &fd, 0);
+ if (err) {
+ printf("Failed to update hash of maps\n");
+ goto out_map_in_map;
+ }
+
+ close(fd);
+ bpf_object__close(obj);
+ return;
+
+out_map_in_map:
+ close(fd);
+ exit(1);
+}
+
#define MAP_SIZE (32 * 1024)
static void test_map_large(void)
@@ -1600,6 +1688,8 @@ static void run_all_tests(void)
test_queuemap(0, NULL);
test_stackmap(0, NULL);
+
+ test_map_in_map();
}
int main(void)
--
2.15.1
^ permalink raw reply related
* Re: [PATCH v4 net-next 0/6] net: dsa: microchip: Modify KSZ9477 DSA driver in preparation to add other KSZ switch drivers
From: David Miller @ 2018-11-21 4:57 UTC (permalink / raw)
To: Tristram.Ha; +Cc: andrew, f.fainelli, pavel, UNGLinuxDriver, netdev
In-Reply-To: <1542758110-1037-1-git-send-email-Tristram.Ha@microchip.com>
From: <Tristram.Ha@microchip.com>
Date: Tue, 20 Nov 2018 15:55:04 -0800
> This series of patches is to modify the original KSZ9477 DSA driver so
> that other KSZ switch drivers can be added and use the common code.
Series applied.
^ permalink raw reply
* Re: [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option
From: Nambiar, Amritha @ 2018-11-21 4:59 UTC (permalink / raw)
To: David Ahern, stephen, netdev
Cc: jakub.kicinski, sridhar.samudrala, jhs, xiyou.wangcong, jiri
In-Reply-To: <2192286d-01ff-dea2-1699-defe72f64a95@gmail.com>
On 11/20/2018 8:46 PM, David Ahern wrote:
> On 11/20/18 9:44 PM, Nambiar, Amritha wrote:
>> On 11/20/2018 2:56 PM, David Ahern wrote:
>>> On 11/15/18 5:55 PM, Amritha Nambiar wrote:
>>>> Add details explaining filtering based on port ranges.
>>>>
>>>> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
>>>> ---
>>>> man/man8/tc-flower.8 | 12 ++++++++++--
>>>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
>>>> index 8be8882..768bfa1 100644
>>>> --- a/man/man8/tc-flower.8
>>>> +++ b/man/man8/tc-flower.8
>>>> @@ -56,8 +56,10 @@ flower \- flow based traffic control filter
>>>> .IR MASKED_IP_TTL " | { "
>>>> .BR dst_ip " | " src_ip " } "
>>>> .IR PREFIX " | { "
>>>> -.BR dst_port " | " src_port " } "
>>>> -.IR port_number " } | "
>>>> +.BR dst_port " | " src_port " } { "
>>>> +.IR port_number " | "
>>>> +.B range
>>>> +.IR min_port_number-max_port_number " } | "
>>>> .B tcp_flags
>>>> .IR MASKED_TCP_FLAGS " | "
>>>> .B type
>>>> @@ -227,6 +229,12 @@ Match on layer 4 protocol source or destination port number. Only available for
>>>> .BR ip_proto " values " udp ", " tcp " and " sctp
>>>> which have to be specified in beforehand.
>>>> .TP
>>>> +.BI range " MIN_VALUE-MAX_VALUE"
>>>> +Match on a range of layer 4 protocol source or destination port number. Only
>>>> +available for
>>>> +.BR ip_proto " values " udp ", " tcp " and " sctp
>>>> +which have to be specified in beforehand.
>>>> +.TP
>>>> .BI tcp_flags " MASKED_TCP_FLAGS"
>>>> Match on TCP flags represented as 12bit bitfield in in hexadecimal format.
>>>> A mask may be optionally provided to limit the bits which are matched. A mask
>>>>
>>>
>>> This prints as:
>>>
>>> dst_port NUMBER
>>> src_port NUMBER
>>> Match on layer 4 protocol source or destination port number.
>>> Only available for ip_proto values udp, tcp and sctp which have
>>> to be specified in beforehand.
>>>
>>> range MIN_VALUE-MAX_VALUE
>>> Match on a range of layer 4 protocol source or destination port
>>> number. Only available for ip_proto values udp, tcp and sctp
>>> which have to be specified in beforehand.
>>>
>>> ###
>>>
>>> That makes it look like range is a standalone option - independent of
>>> dst_port/src_port.
>>>
>>> It seems to me the dst_port / src_port should be updated to:
>>>
>>> dst_port {NUMBER | range MIN_VALUE-MAX_VALUE}
>>>
>>> with the description updated for both options and indented under
>>> dst_port / src_port
>>>
>>
>> Okay, will do.
>>
>
> Thinking about this perhaps the 'range' keyword can just be dropped. We
> do not use it in other places -- e.g., ip rule.
>
Oops, submitted the v2 patch for man changes too soon, without seeing
this. So, in this case, should I re-submit the iproute2-flower patch
that was accepted removing the 'range' keyword?
^ permalink raw reply
* Re: [net-next 00/11][pull request] 100GbE Intel Wired LAN Driver Updates 2018-11-20
From: David Miller @ 2018-11-21 4:59 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann
In-Reply-To: <20181120202247.10743-1-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 20 Nov 2018 12:22:36 -0800
> This series contains updates to the ice driver only.
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option
From: David Ahern @ 2018-11-21 4:59 UTC (permalink / raw)
To: Nambiar, Amritha, stephen, netdev
Cc: jakub.kicinski, sridhar.samudrala, jhs, xiyou.wangcong, jiri
In-Reply-To: <f4dd15da-3787-1351-9dcc-e7dbf06fb4b0@intel.com>
On 11/20/18 9:59 PM, Nambiar, Amritha wrote:
> Oops, submitted the v2 patch for man changes too soon, without seeing
> this. So, in this case, should I re-submit the iproute2-flower patch
> that was accepted removing the 'range' keyword?
I think so. Consistency across commands is a good thing.
^ permalink raw reply
* Re: netns_id in bpf_sk_lookup_{tcp,udp}
From: David Ahern @ 2018-11-21 5:12 UTC (permalink / raw)
To: nicolas.dichtel, Joe Stringer; +Cc: netdev, daniel
In-Reply-To: <1754196d-67a8-6632-878f-72e0e6c2d917@6wind.com>
On 11/20/18 2:05 AM, Nicolas Dichtel wrote:
> Le 20/11/2018 à 00:46, David Ahern a écrit :
> [snip]
>> That revelation shows another hole:
>> $ ip netns add foo
>> $ ip netns set foo 0xffffffff
> It also works with 0xf0000000 ...
>
>> $ ip netns list
>> foo (id: 0)
>>
>> Seems like alloc_netid() should error out if reqid < -1 (-1 being the
>> NETNSA_NSID_NOT_ASSIGNED flag) as opposed to blindly ignoring it.
> alloc_netid() tries to allocate the specified nsid if this nsid is valid, ie >=
> 0, else it allocates a new nsid (actually the lower available).
> This is the expected behavior.
>
> For me, it's more an iproute2 problem, which parses an unsigned and silently
> cast it to a signed value.
>
> -----8<--------------------
>
> From 79bac98bfd0acbf2526a3427d5aba96564844209 Mon Sep 17 00:00:00 2001
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Tue, 20 Nov 2018 09:59:46 +0100
> Subject: ipnetns: parse nsid as a signed integer
>
> Don't confuse the user, nsid is a signed interger, this kind of command
> should return an error: 'ip netns set foo 0xffffffff'.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
> ip/ipnetns.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/ip/ipnetns.c b/ip/ipnetns.c
> index 0eac18cf2682..54346ac987cf 100644
> --- a/ip/ipnetns.c
> +++ b/ip/ipnetns.c
> @@ -739,8 +739,7 @@ static int netns_set(int argc, char **argv)
> {
> char netns_path[PATH_MAX];
> const char *name;
> - unsigned int nsid;
> - int netns;
> + int netns, nsid;
>
> if (argc < 1) {
> fprintf(stderr, "No netns name specified\n");
> @@ -754,7 +753,7 @@ static int netns_set(int argc, char **argv)
> /* If a negative nsid is specified the kernel will select the nsid. */
> if (strcmp(argv[1], "auto") == 0)
> nsid = -1;
> - else if (get_unsigned(&nsid, argv[1], 0))
> + else if (get_integer(&nsid, argv[1], 0))
> invarg("Invalid \"netnsid\" value\n", argv[1]);
>
> snprintf(netns_path, sizeof(netns_path), "%s/%s", NETNS_RUN_DIR, name);
>
Nicolas: Can you send this formally and cc Stephen so it goes into the
master branch? Thanks
^ permalink raw reply
* Re: [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option
From: Nambiar, Amritha @ 2018-11-21 5:16 UTC (permalink / raw)
To: David Ahern, stephen, netdev
Cc: jakub.kicinski, sridhar.samudrala, jhs, xiyou.wangcong, jiri
In-Reply-To: <8426219f-7024-1030-01ce-8daecbaa097b@gmail.com>
On 11/20/2018 8:59 PM, David Ahern wrote:
> On 11/20/18 9:59 PM, Nambiar, Amritha wrote:
>> Oops, submitted the v2 patch for man changes too soon, without seeing
>> this. So, in this case, should I re-submit the iproute2-flower patch
>> that was accepted removing the 'range' keyword?
>
> I think so. Consistency across commands is a good thing.
>
Okay, will do. I'll also combine the 'man patch' into 'flower patch' and
make a single patch as Jiri recommended.
^ permalink raw reply
* Re: [PATCH v2 14/14] nvme-tcp: add NVMe over TCP host driver
From: Sagi Grimberg @ 2018-11-21 5:43 UTC (permalink / raw)
To: Ethan Weidman, linux-nvme@lists.infradead.org
Cc: linux-block@vger.kernel.org, netdev@vger.kernel.org,
David S. Miller, Keith Busch, Christoph Hellwig
In-Reply-To: <CY4PR08MB258265FB085A9C26B6F27D93CFDA0@CY4PR08MB2582.namprd08.prod.outlook.com>
> NVME_TCP_DISC_PORT, 8009, should not be the default for nvme fabric connections when trsvcid is not specified.
Its allowed, the statement simply does not recommend it as a possible
misuse in an environment where iWARP and TCP are deployed on the same
port spaces...
And, it recommends that subsystems don't listen on the 8009 by default,
not anything about a host. Furthermore, usually controllers that are not
passed with a default port are discovery controllers anyways.
> NVME_RDMA_IP_PORT, 4420, should be used, or renamed to something more generic.
Not at all... in fact, this is really not what the spec is intending.
^ permalink raw reply
* RE: [PATCH net-next 3/8] net: hns3: Add "FD flow table" info query function
From: Salil Mehta @ 2018-11-21 16:25 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem@davemloft.net, Zhuangyuzeng (Yisen), lipeng (Y),
mehta.salil@opnsrc.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Linuxarm, Liuzhongzhu
In-Reply-To: <20181119143045.5a147396@cakuba.netronome.com>
Hi Jacub,
> From: Jakub Kicinski [mailto:jakub.kicinski@netronome.com]
> Sent: Monday, November 19, 2018 10:31 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: davem@davemloft.net; Zhuangyuzeng (Yisen) <yisen.zhuang@huawei.com>;
> lipeng (Y) <lipeng321@huawei.com>; mehta.salil@opnsrc.net;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
> <linuxarm@huawei.com>; Liuzhongzhu <liuzhongzhu@huawei.com>
> Subject: Re: [PATCH net-next 3/8] net: hns3: Add "FD flow table" info
> query function
>
> On Mon, 19 Nov 2018 21:18:40 +0000, Salil Mehta wrote:
> > diff --git
> a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h.rej
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h.rej
> > new file mode 100644
> > index 000000000000..be7d0dea06e7
> > --- /dev/null
> > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h.rej
> > @@ -0,0 +1,12 @@
> > +*************** int hclge_set_vlan_filter(struct hnae3_handle
> *handle, __be16 proto,
> > +*** 800,803 ****
> > + void hclge_reset_vf_queue(struct hclge_vport *vport, u16 queue_id);
> > + int hclge_cfg_flowctrl(struct hclge_dev *hdev);
> > + int hclge_func_reset_cmd(struct hclge_dev *hdev, int func_id);
> > + #endif
> > +--- 800,804 ----
> > + void hclge_reset_vf_queue(struct hclge_vport *vport, u16 queue_id);
> > + int hclge_cfg_flowctrl(struct hclge_dev *hdev);
> > + int hclge_func_reset_cmd(struct hclge_dev *hdev, int func_id);
> > ++ int hclge_dbg_run_cmd(struct hnae3_handle *handle, char *cmd_buf);
> > + #endif
>
> You probably don't need this.
Goodness! I was on a transit while I floated this patch-set, somehow
missed my quick review. Sorry for this and thanks for catching!
Best regards
Salil
^ permalink raw reply
* [iproute2-next PATCH v4] tc: flower: Classify packets based port ranges
From: Amritha Nambiar @ 2018-11-21 6:17 UTC (permalink / raw)
To: stephen, netdev, dsahern
Cc: jakub.kicinski, amritha.nambiar, sridhar.samudrala, jhs,
xiyou.wangcong, jiri
Added support for filtering based on port ranges.
UAPI changes have been accepted into net-next.
Example:
1. Match on a port range:
-------------------------
$ tc filter add dev enp4s0 protocol ip parent ffff:\
prio 1 flower ip_proto tcp dst_port range 20-30 skip_hw\
action drop
$ tc -s filter show dev enp4s0 parent ffff:
filter protocol ip pref 1 flower chain 0
filter protocol ip pref 1 flower chain 0 handle 0x1
eth_type ipv4
ip_proto tcp
dst_port range 20-30
skip_hw
not_in_hw
action order 1: gact action drop
random type none pass val 0
index 1 ref 1 bind 1 installed 85 sec used 3 sec
Action statistics:
Sent 460 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
2. Match on IP address and port range:
--------------------------------------
$ tc filter add dev enp4s0 protocol ip parent ffff:\
prio 1 flower dst_ip 192.168.1.1 ip_proto tcp dst_port range 100-200\
skip_hw action drop
$ tc -s filter show dev enp4s0 parent ffff:
filter protocol ip pref 1 flower chain 0 handle 0x2
eth_type ipv4
ip_proto tcp
dst_ip 192.168.1.1
dst_port range 100-200
skip_hw
not_in_hw
action order 1: gact action drop
random type none pass val 0
index 2 ref 1 bind 1 installed 58 sec used 2 sec
Action statistics:
Sent 920 bytes 20 pkt (dropped 20, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
v4:
Added man updates explaining filtering based on port ranges.
Removed 'range' keyword.
v3:
Modified flower_port_range_attr_type calls.
v2:
Addressed Jiri's comment to sync output format with input
Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
---
man/man8/tc-flower.8 | 13 +++--
tc/f_flower.c | 136 ++++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 134 insertions(+), 15 deletions(-)
diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 8be8882..adff41e 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -56,8 +56,9 @@ flower \- flow based traffic control filter
.IR MASKED_IP_TTL " | { "
.BR dst_ip " | " src_ip " } "
.IR PREFIX " | { "
-.BR dst_port " | " src_port " } "
-.IR port_number " } | "
+.BR dst_port " | " src_port " } { "
+.IR port_number " | "
+.IR min_port_number-max_port_number " } | "
.B tcp_flags
.IR MASKED_TCP_FLAGS " | "
.B type
@@ -220,10 +221,12 @@ must be a valid IPv4 or IPv6 address, depending on the \fBprotocol\fR
option to tc filter, optionally followed by a slash and the prefix length.
If the prefix is missing, \fBtc\fR assumes a full-length host match.
.TP
-.BI dst_port " NUMBER"
+.IR \fBdst_port " { " NUMBER " | " " MIN_VALUE-MAX_VALUE " }
.TQ
-.BI src_port " NUMBER"
-Match on layer 4 protocol source or destination port number. Only available for
+.IR \fBsrc_port " { " NUMBER " | " " MIN_VALUE-MAX_VALUE " }
+Match on layer 4 protocol source or destination port number. Alternatively, the
+mininum and maximum values can be specified to match on a range of layer 4
+protocol source or destination port numbers. Only available for
.BR ip_proto " values " udp ", " tcp " and " sctp
which have to be specified in beforehand.
.TP
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 65fca04..722647d 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -494,6 +494,68 @@ static int flower_parse_port(char *str, __u8 ip_proto,
return 0;
}
+static int flower_port_range_attr_type(__u8 ip_proto, enum flower_endpoint type,
+ __be16 *min_port_type,
+ __be16 *max_port_type)
+{
+ if (ip_proto == IPPROTO_TCP || ip_proto == IPPROTO_UDP ||
+ ip_proto == IPPROTO_SCTP) {
+ if (type == FLOWER_ENDPOINT_SRC) {
+ *min_port_type = TCA_FLOWER_KEY_PORT_SRC_MIN;
+ *max_port_type = TCA_FLOWER_KEY_PORT_SRC_MAX;
+ } else {
+ *min_port_type = TCA_FLOWER_KEY_PORT_DST_MIN;
+ *max_port_type = TCA_FLOWER_KEY_PORT_DST_MAX;
+ }
+ } else {
+ return -1;
+ }
+
+ return 0;
+}
+
+static int flower_parse_port_range(__be16 *min, __be16 *max, __u8 ip_proto,
+ enum flower_endpoint endpoint,
+ struct nlmsghdr *n)
+{
+ __be16 min_port_type, max_port_type;
+
+ if (htons(*max) <= htons(*min)) {
+ fprintf(stderr, "max value should be greater than min value\n");
+ return -1;
+ }
+
+ if (flower_port_range_attr_type(ip_proto, endpoint, &min_port_type,
+ &max_port_type))
+ return -1;
+
+ addattr16(n, MAX_MSG, min_port_type, *min);
+ addattr16(n, MAX_MSG, max_port_type, *max);
+
+ return 0;
+}
+
+static int get_range(__be16 *min, __be16 *max, char *argv)
+{
+ char *r;
+
+ r = strchr(argv, '-');
+ if (r) {
+ *r = '\0';
+ if (get_be16(min, argv, 10)) {
+ fprintf(stderr, "invalid min range\n");
+ return -1;
+ }
+ if (get_be16(max, r + 1, 10)) {
+ fprintf(stderr, "invalid max range\n");
+ return -1;
+ }
+ } else {
+ return -1;
+ }
+ return 0;
+}
+
#define TCP_FLAGS_MAX_MASK 0xfff
static int flower_parse_tcp_flags(char *str, int flags_type, int mask_type,
@@ -1061,20 +1123,47 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
return -1;
}
} else if (matches(*argv, "dst_port") == 0) {
+ __be16 min, max;
+
NEXT_ARG();
- ret = flower_parse_port(*argv, ip_proto,
- FLOWER_ENDPOINT_DST, n);
- if (ret < 0) {
- fprintf(stderr, "Illegal \"dst_port\"\n");
- return -1;
+
+ if (!get_range(&min, &max, *argv)) {
+ ret = flower_parse_port_range(&min, &max,
+ ip_proto,
+ FLOWER_ENDPOINT_DST,
+ n);
+ if (ret < 0) {
+ fprintf(stderr, "Illegal \"dst_port range\"\n");
+ return -1;
+ }
+ } else {
+ ret = flower_parse_port(*argv, ip_proto,
+ FLOWER_ENDPOINT_DST, n);
+ if (ret < 0) {
+ fprintf(stderr, "Illegal \"dst_port\"\n");
+ return -1;
+ }
}
} else if (matches(*argv, "src_port") == 0) {
+ __be16 min, max;
+
NEXT_ARG();
- ret = flower_parse_port(*argv, ip_proto,
- FLOWER_ENDPOINT_SRC, n);
- if (ret < 0) {
- fprintf(stderr, "Illegal \"src_port\"\n");
- return -1;
+ if (!get_range(&min, &max, *argv)) {
+ ret = flower_parse_port_range(&min, &max,
+ ip_proto,
+ FLOWER_ENDPOINT_SRC,
+ n);
+ if (ret < 0) {
+ fprintf(stderr, "Illegal \"src_port range\"\n");
+ return -1;
+ }
+ } else {
+ ret = flower_parse_port(*argv, ip_proto,
+ FLOWER_ENDPOINT_SRC, n);
+ if (ret < 0) {
+ fprintf(stderr, "Illegal \"src_port\"\n");
+ return -1;
+ }
}
} else if (matches(*argv, "tcp_flags") == 0) {
NEXT_ARG();
@@ -1490,6 +1579,22 @@ static void flower_print_port(char *name, struct rtattr *attr)
print_hu(PRINT_ANY, name, namefrm, rta_getattr_be16(attr));
}
+static void flower_print_port_range(char *name, struct rtattr *min_attr,
+ struct rtattr *max_attr)
+{
+ SPRINT_BUF(namefrm);
+ SPRINT_BUF(out);
+ size_t done;
+
+ if (!min_attr || !max_attr)
+ return;
+
+ done = sprintf(out, "%u", rta_getattr_be16(min_attr));
+ sprintf(out + done, "-%u", rta_getattr_be16(max_attr));
+ sprintf(namefrm, "\n %s %%s", name);
+ print_string(PRINT_ANY, name, namefrm, out);
+}
+
static void flower_print_tcp_flags(const char *name, struct rtattr *flags_attr,
struct rtattr *mask_attr)
{
@@ -1678,6 +1783,7 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
struct rtattr *opt, __u32 handle)
{
struct rtattr *tb[TCA_FLOWER_MAX + 1];
+ __be16 min_port_type, max_port_type;
int nl_type, nl_mask_type;
__be16 eth_type = 0;
__u8 ip_proto = 0xff;
@@ -1796,6 +1902,16 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
if (nl_type >= 0)
flower_print_port("src_port", tb[nl_type]);
+ if (!flower_port_range_attr_type(ip_proto, FLOWER_ENDPOINT_DST,
+ &min_port_type, &max_port_type))
+ flower_print_port_range("dst_port range",
+ tb[min_port_type], tb[max_port_type]);
+
+ if (!flower_port_range_attr_type(ip_proto, FLOWER_ENDPOINT_SRC,
+ &min_port_type, &max_port_type))
+ flower_print_port_range("src_port range",
+ tb[min_port_type], tb[max_port_type]);
+
flower_print_tcp_flags("tcp_flags", tb[TCA_FLOWER_KEY_TCP_FLAGS],
tb[TCA_FLOWER_KEY_TCP_FLAGS_MASK]);
^ permalink raw reply related
* Re: [PATCH 3/4] tools: bpftool: fix potential NULL pointer dereference in do_load
From: Jakub Kicinski @ 2018-11-21 16:58 UTC (permalink / raw)
To: Wen Yang
Cc: ast, daniel, quentin.monnet, jiong.wang, guro, sandipan,
john.fastabend, netdev, linux-kernel, zhong.weidong, wang.yi59,
Julia Lawall
In-Reply-To: <1542786192-19164-1-git-send-email-wen.yang99@zte.com.cn>
On Wed, 21 Nov 2018 15:43:12 +0800, Wen Yang wrote:
> This patch fixes a possible null pointer dereference in
> do_load, detected by the semantic patch
> deref_null.cocci, with the following warning:
>
> ./tools/bpf/bpftool/prog.c:1021:23-25: ERROR: map_replace is NULL but dereferenced.
>
> The following code has potential null pointer references:
> 881 map_replace = reallocarray(map_replace, old_map_fds + 1,
> 882 sizeof(*map_replace));
> 883 if (!map_replace) {
> 884 p_err("mem alloc failed");
> 885 goto err_free_reuse_maps;
> 886 }
>
> ...
> 1019 err_free_reuse_maps:
> 1020 for (i = 0; i < old_map_fds; i++)
> 1021 close(map_replace[i].fd);
> 1022 free(map_replace);
Ugh, good catch and very nice commit message! However, I think the
resolution is wrong. We still want to free the old maps. Note that
reallocarray() does not free the old array when reallocation fails, so
we just shouldn't overwrite the map_replace with the return value.
Like this, maybe:
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 5302ee282409..be319c0eb94d 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -846,6 +846,7 @@ static int do_load(int argc, char **argv)
NEXT_ARG();
} else if (is_prefix(*argv, "map")) {
char *endptr, *name;
+ void *new_map_replace;
int fd;
NEXT_ARG();
@@ -878,12 +879,15 @@ static int do_load(int argc, char **argv)
if (fd < 0)
goto err_free_reuse_maps;
- map_replace = reallocarray(map_replace, old_map_fds + 1,
- sizeof(*map_replace));
- if (!map_replace) {
+ new_map_replace = reallocarray(map_replace,
+ old_map_fds + 1,
+ sizeof(*map_replace));
+ if (!new_map_replace) {
p_err("mem alloc failed");
goto err_free_reuse_maps;
}
+ map_replace = new_map_replace;
+
map_replace[old_map_fds].idx = idx;
map_replace[old_map_fds].name = name;
map_replace[old_map_fds].fd = fd;
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Reviewed-by: Tan Hu <tan.hu@zte.com.cn>
> CC: Julia Lawall <julia.lawall@lip6.fr>
> ---
> tools/bpf/bpftool/prog.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 5302ee2..de42187 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -1017,8 +1017,9 @@ static int do_load(int argc, char **argv)
> err_close_obj:
> bpf_object__close(obj);
> err_free_reuse_maps:
> - for (i = 0; i < old_map_fds; i++)
> - close(map_replace[i].fd);
> + if (map_replace)
> + for (i = 0; i < old_map_fds; i++)
> + close(map_replace[i].fd);
> free(map_replace);
> return -1;
> }
^ permalink raw reply related
* Re: [PATCH 3/4] tools: bpftool: fix potential NULL pointer dereference in do_load
From: Y Song @ 2018-11-21 17:18 UTC (permalink / raw)
To: wen.yang99
Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
Quentin Monnet, Jiong Wang, guro, Sandipan Das, John Fastabend,
netdev, LKML, zhong.weidong, wang.yi59, julia.lawall
In-Reply-To: <1542786192-19164-1-git-send-email-wen.yang99@zte.com.cn>
On Tue, Nov 20, 2018 at 11:42 PM Wen Yang <wen.yang99@zte.com.cn> wrote:
>
> This patch fixes a possible null pointer dereference in
> do_load, detected by the semantic patch
> deref_null.cocci, with the following warning:
>
> ./tools/bpf/bpftool/prog.c:1021:23-25: ERROR: map_replace is NULL but dereferenced.
>
> The following code has potential null pointer references:
> 881 map_replace = reallocarray(map_replace, old_map_fds + 1,
> 882 sizeof(*map_replace));
> 883 if (!map_replace) {
> 884 p_err("mem alloc failed");
> 885 goto err_free_reuse_maps;
> 886 }
>
> ...
> 1019 err_free_reuse_maps:
> 1020 for (i = 0; i < old_map_fds; i++)
> 1021 close(map_replace[i].fd);
> 1022 free(map_replace);
I did not see any issues here.
We have code looks like:
867 struct map_replace *map_replace = NULL;
868 struct bpf_program *prog = NULL, *pos;
869 unsigned int old_map_fds = 0;
...
948 map_replace = reallocarray(map_replace,
old_map_fds + 1,
949 sizeof(*map_replace));
950 if (!map_replace) {
951 p_err("mem alloc failed");
952 goto err_free_reuse_maps;
953 }
954 map_replace[old_map_fds].idx = idx;
955 map_replace[old_map_fds].name = name;
956 map_replace[old_map_fds].fd = fd;
957 old_map_fds++;
...
old_map_fds becomes non zero if and only if map_replace is not NULL.
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Reviewed-by: Tan Hu <tan.hu@zte.com.cn>
> CC: Julia Lawall <julia.lawall@lip6.fr>
> ---
> tools/bpf/bpftool/prog.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 5302ee2..de42187 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -1017,8 +1017,9 @@ static int do_load(int argc, char **argv)
> err_close_obj:
> bpf_object__close(obj);
> err_free_reuse_maps:
> - for (i = 0; i < old_map_fds; i++)
> - close(map_replace[i].fd);
> + if (map_replace)
> + for (i = 0; i < old_map_fds; i++)
> + close(map_replace[i].fd);
> free(map_replace);
> return -1;
> }
> --
> 2.9.5
>
^ permalink raw reply
* Re: [PATCH net] sctp: hold transport before accessing its asoc in sctp_hash_transport
From: Xin Long @ 2018-11-21 6:47 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: Neil Horman, network dev, linux-sctp, davem
In-Reply-To: <20181121004625.GC3601@localhost.localdomain>
On Wed, Nov 21, 2018 at 9:46 AM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Tue, Nov 20, 2018 at 07:52:48AM -0500, Neil Horman wrote:
> > On Tue, Nov 20, 2018 at 07:09:16PM +0800, Xin Long wrote:
> > > In sctp_hash_transport, it dereferences a transport's asoc only under
> > > rcu_read_lock. Without holding the transport, its asoc could be freed
> > > already, which leads to a use-after-free panic.
> > >
> > > A similar fix as Commit bab1be79a516 ("sctp: hold transport before
> > > accessing its asoc in sctp_transport_get_next") is needed to hold
> > > the transport before accessing its asoc in sctp_hash_transport.
> > >
> > > Fixes: cd2b70875058 ("sctp: check duplicate node before inserting a new transport")
> > > Reported-by: syzbot+0b05d8aa7cb185107483@syzkaller.appspotmail.com
> > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > ---
> > > net/sctp/input.c | 7 ++++++-
> > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/net/sctp/input.c b/net/sctp/input.c
> > > index 5c36a99..69584e9 100644
> > > --- a/net/sctp/input.c
> > > +++ b/net/sctp/input.c
> > > @@ -896,11 +896,16 @@ int sctp_hash_transport(struct sctp_transport *t)
> > > list = rhltable_lookup(&sctp_transport_hashtable, &arg,
> > > sctp_hash_params);
> > >
> > > - rhl_for_each_entry_rcu(transport, tmp, list, node)
> > > + rhl_for_each_entry_rcu(transport, tmp, list, node) {
> > > + if (!sctp_transport_hold(transport))
> > > + continue;
> > > if (transport->asoc->ep == t->asoc->ep) {
> > > + sctp_transport_put(transport);
> > > rcu_read_unlock();
> > > return -EEXIST;
> > > }
> > > + sctp_transport_put(transport);
> > > + }
> > > rcu_read_unlock();
> > >
> > > err = rhltable_insert_key(&sctp_transport_hashtable, &arg,
> > > --
> > > 2.1.0
> > >
> > >
> >
> > something doesn't feel at all right about this. If we are inserting a transport
> > to an association, it would seem to me that we should have at least one user of
> > the association (i.e. non-zero refcount). As such it seems something is wrong
> > with the association refcount here. At the very least, if there is a case where
> > an association is being removed while a transport is being added, the better
> > solution would be to ensure that sctp_association_destroy goes through a
> > quiescent point prior to unhashing transports from the list, to ensure that
> > there is no conflict with the add operation above.
Changing to do call_rcu(&transport->rcu, sctp_association_destroy) can
work for this case.
But it means asoc and socket (taking the port) will have to wait for a
grace period, which is not expected. We seemed to have talked about
this before, Marcelo?
>
> Consider that the rhl_for_each_entry_rcu() is traversing the global
> rhashtable, and that it may operate on unrelated transports/asocs.
> E.g., transport->asoc in the for() is potentially different from the
> asoc under socket lock.
>
> The core of the fix is at:
> + if (!sctp_transport_hold(transport))
> + continue;
> If we can get a hold, the asoc will be available for dereferencing in
> subsequent lines. Otherwise, move on.
>
> With that, the patch makes sense to me.
>
> Although I would prefer if we come up with a better way to do this
> jump, or even avoid the jump. We are only comparing pointers here and,
> if we had asoc->ep cached on sctp_transport itself, we could avoid the
> atomics here.
Right, but it's another u64.
>
> This change, in the next patch on sctp_epaddr_lookup_transport, will
> hurt performance as that is called in datapath. Rhashtable will help
> on keeping entry lists to a size, but still.
This loop is not long normally, will only a few atomic operations hurt
a noticeable performance?
^ permalink raw reply
* Re: [PATCH 3/4] tools: bpftool: fix potential NULL pointer dereference in do_load
From: Jakub Kicinski @ 2018-11-21 17:30 UTC (permalink / raw)
To: Y Song
Cc: wen.yang99, Alexei Starovoitov, Daniel Borkmann, Quentin Monnet,
Jiong Wang, guro, Sandipan Das, John Fastabend, netdev, LKML,
zhong.weidong, wang.yi59, julia.lawall
In-Reply-To: <CAH3MdRVEoJXh1FRJKPiUvvphRLV9=-a8UV4pFUa=kq4qaWYjww@mail.gmail.com>
On Wed, 21 Nov 2018 09:18:06 -0800, Y Song wrote:
> On Tue, Nov 20, 2018 at 11:42 PM Wen Yang <wen.yang99@zte.com.cn> wrote:
> >
> > This patch fixes a possible null pointer dereference in
> > do_load, detected by the semantic patch
> > deref_null.cocci, with the following warning:
> >
> > ./tools/bpf/bpftool/prog.c:1021:23-25: ERROR: map_replace is NULL but dereferenced.
> >
> > The following code has potential null pointer references:
> > 881 map_replace = reallocarray(map_replace, old_map_fds + 1,
> > 882 sizeof(*map_replace));
> > 883 if (!map_replace) {
> > 884 p_err("mem alloc failed");
> > 885 goto err_free_reuse_maps;
> > 886 }
> >
> > ...
> > 1019 err_free_reuse_maps:
> > 1020 for (i = 0; i < old_map_fds; i++)
> > 1021 close(map_replace[i].fd);
> > 1022 free(map_replace);
>
> I did not see any issues here.
> We have code looks like:
> 867 struct map_replace *map_replace = NULL;
> 868 struct bpf_program *prog = NULL, *pos;
> 869 unsigned int old_map_fds = 0;
> ...
> 948 map_replace = reallocarray(map_replace,
> old_map_fds + 1,
> 949 sizeof(*map_replace));
> 950 if (!map_replace) {
> 951 p_err("mem alloc failed");
> 952 goto err_free_reuse_maps;
> 953 }
> 954 map_replace[old_map_fds].idx = idx;
> 955 map_replace[old_map_fds].name = name;
> 956 map_replace[old_map_fds].fd = fd;
> 957 old_map_fds++;
> ...
>
> old_map_fds becomes non zero if and only if map_replace is not NULL.
Note that this is a realloc in a loop, and map_replace will become NULL
again if realloc fails. We should check the return value of realloc()
without loosing the pointer to the old value. No?
> > Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> > Reviewed-by: Tan Hu <tan.hu@zte.com.cn>
> > CC: Julia Lawall <julia.lawall@lip6.fr>
> > ---
> > tools/bpf/bpftool/prog.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> > index 5302ee2..de42187 100644
> > --- a/tools/bpf/bpftool/prog.c
> > +++ b/tools/bpf/bpftool/prog.c
> > @@ -1017,8 +1017,9 @@ static int do_load(int argc, char **argv)
> > err_close_obj:
> > bpf_object__close(obj);
> > err_free_reuse_maps:
> > - for (i = 0; i < old_map_fds; i++)
> > - close(map_replace[i].fd);
> > + if (map_replace)
> > + for (i = 0; i < old_map_fds; i++)
> > + close(map_replace[i].fd);
> > free(map_replace);
> > return -1;
> > }
> > --
> > 2.9.5
> >
^ permalink raw reply
* Re: [PATCH net-next v3 00/13] virtio: support packed ring
From: David Miller @ 2018-11-21 17:37 UTC (permalink / raw)
To: mst
Cc: tiwei.bie, jasowang, virtualization, linux-kernel, netdev,
virtio-dev, wexu, jfreimann, maxime.coquelin
In-Reply-To: <20181121071308-mutt-send-email-mst@kernel.org>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Wed, 21 Nov 2018 07:20:27 -0500
> Dave, given the holiday, attempts to wrap up the 1.1 spec and the
> patchset size I would very much appreciate a bit more time for
> review. Say until Nov 28?
Ok.
^ permalink raw reply
* Re: [PATCH 3/4] tools: bpftool: fix potential NULL pointer dereference in do_load
From: Y Song @ 2018-11-21 17:45 UTC (permalink / raw)
To: Jakub Kicinski
Cc: wen.yang99, Alexei Starovoitov, Daniel Borkmann, Quentin Monnet,
Jiong Wang, guro, Sandipan Das, John Fastabend, netdev, LKML,
zhong.weidong, wang.yi59, julia.lawall
In-Reply-To: <20181121093044.3c34b66b@cakuba.netronome.com>
On Wed, Nov 21, 2018 at 9:30 AM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Wed, 21 Nov 2018 09:18:06 -0800, Y Song wrote:
> > On Tue, Nov 20, 2018 at 11:42 PM Wen Yang <wen.yang99@zte.com.cn> wrote:
> > >
> > > This patch fixes a possible null pointer dereference in
> > > do_load, detected by the semantic patch
> > > deref_null.cocci, with the following warning:
> > >
> > > ./tools/bpf/bpftool/prog.c:1021:23-25: ERROR: map_replace is NULL but dereferenced.
> > >
> > > The following code has potential null pointer references:
> > > 881 map_replace = reallocarray(map_replace, old_map_fds + 1,
> > > 882 sizeof(*map_replace));
> > > 883 if (!map_replace) {
> > > 884 p_err("mem alloc failed");
> > > 885 goto err_free_reuse_maps;
> > > 886 }
> > >
> > > ...
> > > 1019 err_free_reuse_maps:
> > > 1020 for (i = 0; i < old_map_fds; i++)
> > > 1021 close(map_replace[i].fd);
> > > 1022 free(map_replace);
> >
> > I did not see any issues here.
> > We have code looks like:
> > 867 struct map_replace *map_replace = NULL;
> > 868 struct bpf_program *prog = NULL, *pos;
> > 869 unsigned int old_map_fds = 0;
> > ...
> > 948 map_replace = reallocarray(map_replace,
> > old_map_fds + 1,
> > 949 sizeof(*map_replace));
> > 950 if (!map_replace) {
> > 951 p_err("mem alloc failed");
> > 952 goto err_free_reuse_maps;
> > 953 }
> > 954 map_replace[old_map_fds].idx = idx;
> > 955 map_replace[old_map_fds].name = name;
> > 956 map_replace[old_map_fds].fd = fd;
> > 957 old_map_fds++;
> > ...
> >
> > old_map_fds becomes non zero if and only if map_replace is not NULL.
>
> Note that this is a realloc in a loop, and map_replace will become NULL
> again if realloc fails. We should check the return value of realloc()
> without loosing the pointer to the old value. No?
Or right. Agreed. The right fix seems to restore the old map_replace
in the error path
and jump to err_free_reuse_maps if reallocarray fails.
>
> > > Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> > > Reviewed-by: Tan Hu <tan.hu@zte.com.cn>
> > > CC: Julia Lawall <julia.lawall@lip6.fr>
> > > ---
> > > tools/bpf/bpftool/prog.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> > > index 5302ee2..de42187 100644
> > > --- a/tools/bpf/bpftool/prog.c
> > > +++ b/tools/bpf/bpftool/prog.c
> > > @@ -1017,8 +1017,9 @@ static int do_load(int argc, char **argv)
> > > err_close_obj:
> > > bpf_object__close(obj);
> > > err_free_reuse_maps:
> > > - for (i = 0; i < old_map_fds; i++)
> > > - close(map_replace[i].fd);
> > > + if (map_replace)
> > > + for (i = 0; i < old_map_fds; i++)
> > > + close(map_replace[i].fd);
> > > free(map_replace);
> > > return -1;
> > > }
> > > --
> > > 2.9.5
> > >
>
^ permalink raw reply
* Re: general protection fault in sctp_sched_dequeue_common
From: Xin Long @ 2018-11-21 17:51 UTC (permalink / raw)
To: syzbot+e33a3a138267ca119c7d
Cc: davem, LKML, linux-sctp, Marcelo Ricardo Leitner, network dev,
Neil Horman, syzkaller-bugs, Vlad Yasevich
In-Reply-To: <00000000000014c430057b252d70@google.com>
On Wed, Nov 21, 2018 at 1:30 PM syzbot
<syzbot+e33a3a138267ca119c7d@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: cfc6731d2f79 Merge branch 'sctp-add-subscribe-per-asoc-and..
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=14fc234d400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=73e2bc0cb6463446
> dashboard link: https://syzkaller.appspot.com/bug?extid=e33a3a138267ca119c7d
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+e33a3a138267ca119c7d@syzkaller.appspotmail.com
>
> netlink: 'syz-executor1': attribute type 39 has an invalid length.
> IPv6: ADDRCONF(NETDEV_CHANGE): lo: link becomes ready
> A link change request failed with some changes committed already. Interface
> lo may have been left with an inconsistent configuration, please check.
> kasan: CONFIG_KASAN_INLINE enabled
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: 0000 [#1] PREEMPT SMP KASAN
> CPU: 1 PID: 4030 Comm: syz-executor5 Not tainted 4.20.0-rc3+ #304
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:__list_del_entry_valid+0x84/0x100 lib/list_debug.c:51
> Code: 0f 84 60 01 00 00 48 b8 00 02 00 00 00 00 ad de 49 39 c4 0f 84 39 01
> 00 00 48 b8 00 00 00 00 00 fc ff df 4c 89 e2 48 c1 ea 03 <80> 3c 02 00 75
> 5f 49 8b 14 24 48 39 da 0f 85 4e 01 00 00 49 8d 7d
> RSP: 0018:ffff8881c1bfea48 EFLAGS: 00010246
> RAX: dffffc0000000000 RBX: ffff8881bb175618 RCX: ffffc9001006b000
> RDX: 0000000000000000 RSI: ffffffff8752c823 RDI: ffff8881bb175620
> RBP: ffff8881c1bfea60 R08: ffff88818dc04200 R09: ffff8881c1bff488
> R10: ffffed103837feb6 R11: 0000000000000003 R12: 0000000000000000
> R13: 0000000000000000 R14: ffff8881cb4d8e20 R15: dffffc0000000000
> FS: 00007f34b0944700(0000) GS:ffff8881daf00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000ae0ed0 CR3: 00000001cb898000 CR4: 00000000001406e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> __list_del_entry include/linux/list.h:117 [inline]
> list_del_init include/linux/list.h:159 [inline]
> sctp_sched_dequeue_common+0xab/0x610 net/sctp/stream_sched.c:267
> sctp_sched_fcfs_dequeue+0x18d/0x280 net/sctp/stream_sched.c:90
This probably is also caused by stream->out_curr not being updated
when adding stream_out. we need a fix like:
+static void sctp_stream_out_copy(struct flex_array *fa,
+ struct sctp_stream *stream, __u16 count)
+{
+ size_t index = 0;
+ void *elem;
+
+ count = min(count, stream->outcnt);
+ while (count--) {
+ elem = flex_array_get(stream->out, index);
+ flex_array_put(fa, index, elem, 0);
+ if (stream->out_curr == elem)
+ stream->out_curr = flex_array_get(fa, index);
+ index++;
+ }
+}
+
static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt,
gfp_t gfp)
{
@@ -146,7 +164,7 @@ static int sctp_stream_alloc_out(struct
sctp_stream *stream, __u16 outcnt,
return -ENOMEM;
if (stream->out) {
- fa_copy(out, stream->out, 0, min(outcnt, stream->outcnt));
+ sctp_stream_out_copy(out, stream, outcnt);
fa_free(stream->out);
}
> sctp_outq_dequeue_data net/sctp/outqueue.c:90 [inline]
> sctp_outq_flush_data net/sctp/outqueue.c:1079 [inline]
> sctp_outq_flush+0x13e2/0x34f0 net/sctp/outqueue.c:1205
> sctp_outq_uncork+0x6a/0x80 net/sctp/outqueue.c:772
> sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1820 [inline]
> sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline]
> sctp_do_sm+0x5ff/0x7180 net/sctp/sm_sideeffect.c:1191
> sctp_primitive_SEND+0xa0/0xd0 net/sctp/primitive.c:178
> sctp_sendmsg_to_asoc+0xa0b/0x1a10 net/sctp/socket.c:1955
> sctp_sendmsg+0x13c2/0x1da0 net/sctp/socket.c:2113
> inet_sendmsg+0x1a1/0x690 net/ipv4/af_inet.c:798
> sock_sendmsg_nosec net/socket.c:621 [inline]
> sock_sendmsg+0xd5/0x120 net/socket.c:631
> sock_write_iter+0x35e/0x5c0 net/socket.c:900
> call_write_iter include/linux/fs.h:1857 [inline]
> new_sync_write fs/read_write.c:474 [inline]
> __vfs_write+0x6b8/0x9f0 fs/read_write.c:487
> vfs_write+0x1fc/0x560 fs/read_write.c:549
> ksys_write+0x101/0x260 fs/read_write.c:598
> __do_sys_write fs/read_write.c:610 [inline]
> __se_sys_write fs/read_write.c:607 [inline]
> __x64_sys_write+0x73/0xb0 fs/read_write.c:607
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x457569
> Code: fd b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> ff 0f 83 cb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007f34b0943c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000457569
> RDX: 0000000000034000 RSI: 0000000020000040 RDI: 0000000000000005
> RBP: 000000000072bfa0 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00007f34b09446d4
> R13: 00000000004c5cde R14: 00000000004da090 R15: 00000000ffffffff
> Modules linked in:
> ---[ end trace 23757f22491d2e93 ]---
> RIP: 0010:__list_del_entry_valid+0x84/0x100 lib/list_debug.c:51
> Code: 0f 84 60 01 00 00 48 b8 00 02 00 00 00 00 ad de 49 39 c4 0f 84 39 01
> 00 00 48 b8 00 00 00 00 00 fc ff df 4c 89 e2 48 c1 ea 03 <80> 3c 02 00 75
> 5f 49 8b 14 24 48 39 da 0f 85 4e 01 00 00 49 8d 7d
> kobject: 'loop0' (00000000e65c44e1): kobject_uevent_env
> kobject: 'loop0' (00000000e65c44e1): fill_kobj_path: path
> = '/devices/virtual/block/loop0'
> RSP: 0018:ffff8881c1bfea48 EFLAGS: 00010246
> RAX: dffffc0000000000 RBX: ffff8881bb175618 RCX: ffffc9001006b000
> kobject: 'rx-0' (000000005994716d): kobject_cleanup, parent 00000000ba7aa131
> RDX: 0000000000000000 RSI: ffffffff8752c823 RDI: ffff8881bb175620
> kobject: 'loop4' (000000004d250bad): kobject_uevent_env
> kobject: 'rx-0' (000000005994716d): auto cleanup 'remove' event
> kobject: 'loop4' (000000004d250bad): fill_kobj_path: path
> = '/devices/virtual/block/loop4'
> kobject: 'rx-0' (000000005994716d): kobject_uevent_env
> RBP: ffff8881c1bfea60 R08: ffff88818dc04200 R09: ffff8881c1bff488
> R10: ffffed103837feb6 R11: 0000000000000003 R12: 0000000000000000
> kobject: 'rx-0' (000000005994716d): kobject_uevent_env: uevent_suppress
> caused the event to drop!
> R13: 0000000000000000 R14: ffff8881cb4d8e20 R15: dffffc0000000000
> kobject: 'rx-0' (000000005994716d): auto cleanup kobject_del
> FS: 00007f34b0944700(0000) GS:ffff8881daf00000(0000) knlGS:0000000000000000
> kobject: 'rx-0' (000000005994716d): calling ktype release
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 000000000072c000 CR3: 00000001cb898000 CR4: 00000000001406e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> kobject: 'rx-0': free name
> kobject: 'loop0' (00000000e65c44e1): kobject_uevent_env
> kobject: 'tx-3' (00000000a24965cd): kobject_cleanup, parent 00000000ba7aa131
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> kobject: 'tx-3' (00000000a24965cd): auto cleanup 'remove' event
> kobject: 'loop0' (00000000e65c44e1): fill_kobj_path: path
> = '/devices/virtual/block/loop0'
> kobject: 'tx-3' (00000000a24965cd): kobject_uevent_env
> kobject: 'tx-3' (00000000a24965cd): kobject_uevent_env: uevent_suppress
> caused the event to drop!
> kobject: 'tx-3' (00000000a24965cd): auto cleanup kobject_del
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
^ permalink raw reply
* WARNING in remove_proc_entry (2)
From: syzbot @ 2018-11-21 17:52 UTC (permalink / raw)
To: andy, davem, j.vosburgh, linux-kernel, netdev, syzkaller-bugs,
vfalico
Hello,
syzbot found the following crash on:
HEAD commit: c8ce94b8fe53 Merge tag 'mips_fixes_4.20_3' of git://git.ke..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1325f225400000
kernel config: https://syzkaller.appspot.com/x/.config?x=73e2bc0cb6463446
dashboard link: https://syzkaller.appspot.com/bug?extid=46d1fec9e51890edb1a6
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+46d1fec9e51890edb1a6@syzkaller.appspotmail.com
IPv6: ADDRCONF(NETDEV_UP): bridge0: link is not ready
IPv6: ADDRCONF(NETDEV_CHANGE): bridge0: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): bridge0: link becomes ready
------------[ cut here ]------------
remove_proc_entry: removing non-empty directory 'net/bonding', leaking at
least '�\x0f'
WARNING: CPU: 0 PID: 9001 at fs/proc/generic.c:681
remove_proc_entry+0x3e1/0x4a0 fs/proc/generic.c:679
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 9001 Comm: kworker/u4:6 Not tainted 4.20.0-rc3+ #343
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: netns cleanup_net
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x244/0x39d lib/dump_stack.c:113
panic+0x2ad/0x55c kernel/panic.c:188
__warn.cold.8+0x20/0x45 kernel/panic.c:540
report_bug+0x254/0x2d0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:178 [inline]
do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
do_invalid_op+0x36/0x40 arch/x86/kernel/traps.c:290
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:969
RIP: 0010:remove_proc_entry+0x3e1/0x4a0 fs/proc/generic.c:679
Code: fa 48 c1 ea 03 80 3c 02 00 0f 85 86 00 00 00 49 8b 94 24 c8 00 00 00
48 c7 c6 80 2f 37 88 48 c7 c7 00 2f 37 88 e8 1f c9 52 ff <0f> 0b e9 19 fe
ff ff e8 93 b7 cc ff e9 88 fd ff ff e8 49 b8 cc ff
RSP: 0000:ffff88817e3172f0 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff8881c0602180 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff8165eaf5 RDI: 0000000000000005
RBP: ffff88817e3173c8 R08: ffff8881c0e88400 R09: ffffed103b5c5020
R10: ffffed103b5c5020 R11: ffff8881dae28107 R12: ffff8881ced1e340
R13: ffff88817e317320 R14: 1ffff1102fc62e60 R15: ffff8881c0602228
bond_destroy_proc_dir+0x87/0xdd drivers/net/bonding/bond_procfs.c:307
bond_net_exit+0x33f/0x4d0 drivers/net/bonding/bond_main.c:4837
ops_exit_list.isra.5+0xb0/0x160 net/core/net_namespace.c:153
cleanup_net+0x555/0xb10 net/core/net_namespace.c:551
process_one_work+0xc90/0x1c40 kernel/workqueue.c:2153
worker_thread+0x17f/0x1390 kernel/workqueue.c:2296
kthread+0x35a/0x440 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
Kernel Offset: disabled
Rebooting in 86400 seconds..
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* RE: [PATCH net-next 0/8] net: hns3: Adds support of debugfs to HNS3 driver
From: Salil Mehta @ 2018-11-21 18:06 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem@davemloft.net, Zhuangyuzeng (Yisen), lipeng (Y),
mehta.salil@opnsrc.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Linuxarm
In-Reply-To: <20181119142955.56b6baa6@cakuba.netronome.com>
> From: Jakub Kicinski [mailto:jakub.kicinski@netronome.com]
> Sent: Monday, November 19, 2018 10:30 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: davem@davemloft.net; Zhuangyuzeng (Yisen) <yisen.zhuang@huawei.com>;
> lipeng (Y) <lipeng321@huawei.com>; mehta.salil@opnsrc.net;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
> <linuxarm@huawei.com>
> Subject: Re: [PATCH net-next 0/8] net: hns3: Adds support of debugfs to
> HNS3 driver
>
> On Mon, 19 Nov 2018 21:18:37 +0000, Salil Mehta wrote:
> > This patchset adds support of debugfs to the HNS3 driver.
> >
> > Support has been added to query info related to below items:
> > 1. Queue related
> > 2. Flow Director
> > 3. TC config
> > 4. Transmit Module/Scheduler
> > 5. QoS pause
> > 6. QoS buffer
> > 7. QoS prio map
>
> Please provide a more informative cover letter.
>
> From a short glance it looks like you add a single debugfs file, which
> can be written to, and upon that write the driver will dump stuff into
> kernel logs.
That is correct. Do you foresee any problem with it or are we doing something
which is objectionable?
Salil.
^ permalink raw reply
* Re: Application of f8b39039cbf2a15f ("net: fs_enet: do not call phy_stop() in interrupts") to 4.9 and 4.14 stable
From: Greg KH @ 2018-11-21 18:17 UTC (permalink / raw)
To: Christophe LEROY
Cc: David Miller, netdev@vger.kernel.org, stable@vger.kernel.org
In-Reply-To: <f22dfae5-df25-ca45-7991-0a5b3b31eb23@c-s.fr>
On Wed, Oct 17, 2018 at 02:08:41PM +0200, Christophe LEROY wrote:
> Hi,
>
> Could you please apply f8b39039cbf2a15f2b8c9f081e1cbd5dee00aaf5 to 4.9 and
> 4.14 ?
>
> It fixes an Oops when Ethernet transmission times out.
>
> As you can observe in the commit log, the Oops what initially observed in
> 4.9.
>
> The fix has been in mainline since 4.15
And it was released in 4.9.136 and 4.14.80 already :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode
From: Grygorii Strashko @ 2018-11-21 18:23 UTC (permalink / raw)
To: Kishon Vijay Abraham I, David S. Miller, Antoine Tenart,
Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
Cc: Alexandre Belloni, Manu Gautam, Tony Lindgren, netdev,
Sekhar Nori, linux-kernel, Maxime Ripard, Chen-Yu Tsai,
Chunfeng Yun, linux-mediatek, Vivek Gautam, Carlo Caione,
linux-amlogic, linux-arm-kernel, Matthias Brugger
In-Reply-To: <6418b7c3-7345-76e8-90c0-9a77769f6f87@ti.com>
On 11/21/18 2:55 AM, Kishon Vijay Abraham I wrote:
>
>
> On 20/11/18 6:54 AM, Grygorii Strashko wrote:
>> Hi Kishon, All,
>>
>> Thank you for your review.
>> I've not added "Tested-by"/"Acked-by" tags due to code changes in v3.
>
> merged after adding the required ACKs.
>
Thank you all. I'll send TI specific patches soon.
--
regards,
-grygorii
^ permalink raw reply
* Re: [PATCH net-next 0/8] net: hns3: Adds support of debugfs to HNS3 driver
From: Jakub Kicinski @ 2018-11-21 18:25 UTC (permalink / raw)
To: Salil Mehta
Cc: davem@davemloft.net, Zhuangyuzeng (Yisen), lipeng (Y),
mehta.salil@opnsrc.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Linuxarm
In-Reply-To: <F4CC6FACFEB3C54C9141D49AD221F7F93BFC8A64@FRAEML521-MBX.china.huawei.com>
On Wed, 21 Nov 2018 18:06:19 +0000, Salil Mehta wrote:
> > From: Jakub Kicinski [mailto:jakub.kicinski@netronome.com]
> > Sent: Monday, November 19, 2018 10:30 PM
> > To: Salil Mehta <salil.mehta@huawei.com>
> > Cc: davem@davemloft.net; Zhuangyuzeng (Yisen) <yisen.zhuang@huawei.com>;
> > lipeng (Y) <lipeng321@huawei.com>; mehta.salil@opnsrc.net;
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
> > <linuxarm@huawei.com>
> > Subject: Re: [PATCH net-next 0/8] net: hns3: Adds support of debugfs to
> > HNS3 driver
> >
> > On Mon, 19 Nov 2018 21:18:37 +0000, Salil Mehta wrote:
> > > This patchset adds support of debugfs to the HNS3 driver.
> > >
> > > Support has been added to query info related to below items:
> > > 1. Queue related
> > > 2. Flow Director
> > > 3. TC config
> > > 4. Transmit Module/Scheduler
> > > 5. QoS pause
> > > 6. QoS buffer
> > > 7. QoS prio map
> >
> > Please provide a more informative cover letter.
> >
> > From a short glance it looks like you add a single debugfs file, which
> > can be written to, and upon that write the driver will dump stuff into
> > kernel logs.
>
> That is correct. Do you foresee any problem with it or are we doing something
> which is objectionable?
Its fine with me, but I think not everyone reading the list will read
the code so a few extra words and adding one of the examples from the
patches to the cover letter could help people understand the changes.
^ permalink raw reply
* Re: [PATCH 00/12 net-next,v2] add flow_rule infrastructure
From: Jiri Pirko @ 2018-11-21 7:46 UTC (permalink / raw)
To: David Miller
Cc: pablo, netdev, thomas.lendacky, f.fainelli, ariel.elior,
michael.chan, santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz
In-Reply-To: <20181120.091640.1759508282843765424.davem@davemloft.net>
Tue, Nov 20, 2018 at 06:16:40PM CET, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Tue, 20 Nov 2018 08:39:12 +0100
>
>> If later on the netfilter code will use it, through another
>> ndo/notifier/whatever, that is side a nice side-effect in my
>> opinion.
>
>Netfilter HW offloading is the main motivation of these changes.
>
>You can try to spin it any way you like, but I think this is pretty
>clear.
>
>Would the author of these changes be even be remotely interested in
>this "cleanup" in areas of code he has never been involved in if that
>were not the case?
No, but of course. I'm just saying that the cleanup is nice and handy
even if the code would never be used by netfilter. Therefore I think
the info is irrelevant for the review. Anyway, I get your point.
>
>I think it is very dishonest to portray the situation differently.
>
>Thank you.
^ permalink raw reply
* Re: [PATCH v3 4/5] phy: mvebu-cp110-comphy: convert to use eth phy mode and submode
From: Antoine Tenart @ 2018-11-21 8:02 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier,
netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
Chunfeng Yun, Matthias Brugger <m
In-Reply-To: <20181120012424.11802-5-grygorii.strashko@ti.com>
Hi Grygorii,
On Mon, Nov 19, 2018 at 07:24:23PM -0600, Grygorii Strashko wrote:
> Convert mvebu-cp110-comphy PHY driver to use recently introduced
> PHY_MODE_ETHERNET and phy_set_mode_ext().
>
> Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
> Cc: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Cc: Antoine Tenart <antoine.tenart@free-electrons.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Thanks for the changes, this looks good. I tested this on a
MacchiatoBin.
Acked-by: Antoine Tenart <antoine.tenart@bootlin.com>
> ---
> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 19 +-----
> drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 90 ++++++++++++++-----------
> 2 files changed, 53 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 7a37a37..731793a 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -1165,28 +1165,13 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
> */
> static int mvpp22_comphy_init(struct mvpp2_port *port)
> {
> - enum phy_mode mode;
> int ret;
>
> if (!port->comphy)
> return 0;
>
> - switch (port->phy_interface) {
> - case PHY_INTERFACE_MODE_SGMII:
> - case PHY_INTERFACE_MODE_1000BASEX:
> - mode = PHY_MODE_SGMII;
> - break;
> - case PHY_INTERFACE_MODE_2500BASEX:
> - mode = PHY_MODE_2500SGMII;
> - break;
> - case PHY_INTERFACE_MODE_10GKR:
> - mode = PHY_MODE_10GKR;
> - break;
> - default:
> - return -EINVAL;
> - }
> -
> - ret = phy_set_mode(port->comphy, mode);
> + ret = phy_set_mode_ext(port->comphy, PHY_MODE_ETHERNET,
> + port->phy_interface);
> if (ret)
> return ret;
>
> diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> index 79b52c3..2b4462a 100644
> --- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> +++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> @@ -9,6 +9,7 @@
> #include <linux/iopoll.h>
> #include <linux/mfd/syscon.h>
> #include <linux/module.h>
> +#include <linux/phy.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> @@ -116,41 +117,43 @@
>
> struct mvebu_comhy_conf {
> enum phy_mode mode;
> + int submode;
> unsigned lane;
> unsigned port;
> u32 mux;
> };
>
> -#define MVEBU_COMPHY_CONF(_lane, _port, _mode, _mux) \
> +#define MVEBU_COMPHY_CONF(_lane, _port, _submode, _mux) \
> { \
> .lane = _lane, \
> .port = _port, \
> - .mode = _mode, \
> + .mode = PHY_MODE_ETHERNET, \
> + .submode = _submode, \
> .mux = _mux, \
> }
>
> static const struct mvebu_comhy_conf mvebu_comphy_cp110_modes[] = {
> /* lane 0 */
> - MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
> - MVEBU_COMPHY_CONF(0, 1, PHY_MODE_2500SGMII, 0x1),
> + MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
> + MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> /* lane 1 */
> - MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII, 0x1),
> - MVEBU_COMPHY_CONF(1, 2, PHY_MODE_2500SGMII, 0x1),
> + MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
> + MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> /* lane 2 */
> - MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII, 0x1),
> - MVEBU_COMPHY_CONF(2, 0, PHY_MODE_2500SGMII, 0x1),
> - MVEBU_COMPHY_CONF(2, 0, PHY_MODE_10GKR, 0x1),
> + MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_SGMII, 0x1),
> + MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> + MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_10GKR, 0x1),
> /* lane 3 */
> - MVEBU_COMPHY_CONF(3, 1, PHY_MODE_SGMII, 0x2),
> - MVEBU_COMPHY_CONF(3, 1, PHY_MODE_2500SGMII, 0x2),
> + MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_SGMII, 0x2),
> + MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_2500BASEX, 0x2),
> /* lane 4 */
> - MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII, 0x2),
> - MVEBU_COMPHY_CONF(4, 0, PHY_MODE_2500SGMII, 0x2),
> - MVEBU_COMPHY_CONF(4, 0, PHY_MODE_10GKR, 0x2),
> - MVEBU_COMPHY_CONF(4, 1, PHY_MODE_SGMII, 0x1),
> + MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_SGMII, 0x2),
> + MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_2500BASEX, 0x2),
> + MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_10GKR, 0x2),
> + MVEBU_COMPHY_CONF(4, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
> /* lane 5 */
> - MVEBU_COMPHY_CONF(5, 2, PHY_MODE_SGMII, 0x1),
> - MVEBU_COMPHY_CONF(5, 2, PHY_MODE_2500SGMII, 0x1),
> + MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
> + MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> };
>
> struct mvebu_comphy_priv {
> @@ -163,10 +166,12 @@ struct mvebu_comphy_lane {
> struct mvebu_comphy_priv *priv;
> unsigned id;
> enum phy_mode mode;
> + int submode;
> int port;
> };
>
> -static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
> +static int mvebu_comphy_get_mux(int lane, int port,
> + enum phy_mode mode, int submode)
> {
> int i, n = ARRAY_SIZE(mvebu_comphy_cp110_modes);
>
> @@ -177,7 +182,8 @@ static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
> for (i = 0; i < n; i++) {
> if (mvebu_comphy_cp110_modes[i].lane == lane &&
> mvebu_comphy_cp110_modes[i].port == port &&
> - mvebu_comphy_cp110_modes[i].mode == mode)
> + mvebu_comphy_cp110_modes[i].mode == mode &&
> + mvebu_comphy_cp110_modes[i].submode == submode)
> break;
> }
>
> @@ -187,8 +193,7 @@ static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
> return mvebu_comphy_cp110_modes[i].mux;
> }
>
> -static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
> - enum phy_mode mode)
> +static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane)
> {
> struct mvebu_comphy_priv *priv = lane->priv;
> u32 val;
> @@ -206,14 +211,14 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
> MVEBU_COMPHY_SERDES_CFG0_HALF_BUS |
> MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xf) |
> MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xf));
> - if (mode == PHY_MODE_10GKR)
> + if (lane->submode == PHY_INTERFACE_MODE_10GKR)
> val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xe) |
> MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xe);
> - else if (mode == PHY_MODE_2500SGMII)
> + else if (lane->submode == PHY_INTERFACE_MODE_2500BASEX)
> val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x8) |
> MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x8) |
> MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
> - else if (mode == PHY_MODE_SGMII)
> + else if (lane->submode == PHY_INTERFACE_MODE_SGMII)
> val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x6) |
> MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x6) |
> MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
> @@ -243,7 +248,7 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
> /* refclk selection */
> val = readl(priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
> val &= ~MVEBU_COMPHY_MISC_CTRL0_REFCLK_SEL;
> - if (mode == PHY_MODE_10GKR)
> + if (lane->submode == PHY_INTERFACE_MODE_10GKR)
> val |= MVEBU_COMPHY_MISC_CTRL0_ICP_FORCE;
> writel(val, priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
>
> @@ -261,8 +266,7 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
> writel(val, priv->base + MVEBU_COMPHY_LOOPBACK(lane->id));
> }
>
> -static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
> - enum phy_mode mode)
> +static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane)
> {
> struct mvebu_comphy_priv *priv = lane->priv;
> u32 val;
> @@ -303,13 +307,13 @@ static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
> return 0;
> }
>
> -static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
> +static int mvebu_comphy_set_mode_sgmii(struct phy *phy)
> {
> struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
> struct mvebu_comphy_priv *priv = lane->priv;
> u32 val;
>
> - mvebu_comphy_ethernet_init_reset(lane, mode);
> + mvebu_comphy_ethernet_init_reset(lane);
>
> val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
> val &= ~MVEBU_COMPHY_RX_CTRL1_CLK8T_EN;
> @@ -330,7 +334,7 @@ static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
> val |= MVEBU_COMPHY_GEN1_S0_TX_EMPH(0x1);
> writel(val, priv->base + MVEBU_COMPHY_GEN1_S0(lane->id));
>
> - return mvebu_comphy_init_plls(lane, PHY_MODE_SGMII);
> + return mvebu_comphy_init_plls(lane);
> }
>
> static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
> @@ -339,7 +343,7 @@ static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
> struct mvebu_comphy_priv *priv = lane->priv;
> u32 val;
>
> - mvebu_comphy_ethernet_init_reset(lane, PHY_MODE_10GKR);
> + mvebu_comphy_ethernet_init_reset(lane);
>
> val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
> val |= MVEBU_COMPHY_RX_CTRL1_RXCLK2X_SEL |
> @@ -469,7 +473,7 @@ static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
> val |= MVEBU_COMPHY_EXT_SELV_RX_SAMPL(0x1a);
> writel(val, priv->base + MVEBU_COMPHY_EXT_SELV(lane->id));
>
> - return mvebu_comphy_init_plls(lane, PHY_MODE_10GKR);
> + return mvebu_comphy_init_plls(lane);
> }
>
> static int mvebu_comphy_power_on(struct phy *phy)
> @@ -479,7 +483,8 @@ static int mvebu_comphy_power_on(struct phy *phy)
> int ret, mux;
> u32 val;
>
> - mux = mvebu_comphy_get_mux(lane->id, lane->port, lane->mode);
> + mux = mvebu_comphy_get_mux(lane->id, lane->port,
> + lane->mode, lane->submode);
> if (mux < 0)
> return -ENOTSUPP;
>
> @@ -492,12 +497,12 @@ static int mvebu_comphy_power_on(struct phy *phy)
> val |= mux << MVEBU_COMPHY_SELECTOR_PHY(lane->id);
> regmap_write(priv->regmap, MVEBU_COMPHY_SELECTOR, val);
>
> - switch (lane->mode) {
> - case PHY_MODE_SGMII:
> - case PHY_MODE_2500SGMII:
> - ret = mvebu_comphy_set_mode_sgmii(phy, lane->mode);
> + switch (lane->submode) {
> + case PHY_INTERFACE_MODE_SGMII:
> + case PHY_INTERFACE_MODE_2500BASEX:
> + ret = mvebu_comphy_set_mode_sgmii(phy);
> break;
> - case PHY_MODE_10GKR:
> + case PHY_INTERFACE_MODE_10GKR:
> ret = mvebu_comphy_set_mode_10gkr(phy);
> break;
> default:
> @@ -517,10 +522,17 @@ static int mvebu_comphy_set_mode(struct phy *phy,
> {
> struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
>
> - if (mvebu_comphy_get_mux(lane->id, lane->port, mode) < 0)
> + if (mode != PHY_MODE_ETHERNET)
> + return -EINVAL;
> +
> + if (submode == PHY_INTERFACE_MODE_1000BASEX)
> + submode = PHY_INTERFACE_MODE_SGMII;
> +
> + if (mvebu_comphy_get_mux(lane->id, lane->port, mode, submode) < 0)
> return -EINVAL;
>
> lane->mode = mode;
> + lane->submode = submode;
> return 0;
> }
>
> --
> 2.10.5
>
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* [PATCH net-next 01/16] vxlan: __vxlan_fdb_delete(): Drop unused argument vid
From: Ido Schimmel @ 2018-11-21 8:02 UTC (permalink / raw)
To: netdev@vger.kernel.org, bridge@lists.linux-foundation.org
Cc: davem@davemloft.net, Jiri Pirko, Petr Machata,
roopa@cumulusnetworks.com, nikolay@cumulusnetworks.com,
stephen@networkplumber.org, ivecera@redhat.com, mlxsw,
Ido Schimmel
In-Reply-To: <20181121080141.16676-1-idosch@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
This argument is necessary for vxlan_fdb_delete(), the API of which is
prescribed by ndo_fdb_del, but __vxlan_fdb_delete() doesn't need it.
Therefore drop it.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/vxlan.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index c3e65e78f015..99ab7844476f 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -977,7 +977,7 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
static int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
const unsigned char *addr, union vxlan_addr ip,
__be16 port, __be32 src_vni, __be32 vni,
- u32 ifindex, u16 vid)
+ u32 ifindex)
{
struct vxlan_fdb *f;
struct vxlan_rdst *rd = NULL;
@@ -1024,8 +1024,7 @@ static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
return err;
spin_lock_bh(&vxlan->hash_lock);
- err = __vxlan_fdb_delete(vxlan, addr, ip, port, src_vni, vni, ifindex,
- vid);
+ err = __vxlan_fdb_delete(vxlan, addr, ip, port, src_vni, vni, ifindex);
spin_unlock_bh(&vxlan->hash_lock);
return err;
@@ -3611,7 +3610,7 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
vxlan->cfg.dst_port,
old_dst.remote_vni,
old_dst.remote_vni,
- old_dst.remote_ifindex, 0);
+ old_dst.remote_ifindex);
if (!vxlan_addr_any(&dst->remote_ip)) {
err = vxlan_fdb_create(vxlan, all_zeros_mac,
--
2.19.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox