* Re: [PATCH net] net: rtnetlink: fix info leak in RTM_GETSTATS call
From: Roopa Prabhu @ 2017-10-03 14:18 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev@vger.kernel.org, keescook, Dmitry Vyukov, Andrey Konovalov,
Kostya Serebryany, Alexander Potapenko, davem@davemloft.net,
Eric Dumazet
In-Reply-To: <1507026048-13734-1-git-send-email-nikolay@cumulusnetworks.com>
On Tue, Oct 3, 2017 at 3:20 AM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> When RTM_GETSTATS was added the fields of its header struct were not all
> initialized when returning the result thus leaking 4 bytes of information
> to user-space per rtnl_fill_statsinfo call, so initialize them now. Thanks
> to Alexander Potapenko for the detailed report and bisection.
>
> Reported-by: Alexander Potapenko <glider@google.com>
> Fixes: 10c9ead9f3c6 ("rtnetlink: add new RTM_GETSTATS message to dump link stats")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Thanks Nikolay!.
^ permalink raw reply
* Re: [PATCH net-next v2 3/3] tools: bpftool: add documentation
From: Daniel Borkmann @ 2017-10-03 14:15 UTC (permalink / raw)
To: Jakub Kicinski, Alexei Starovoitov; +Cc: netdev, oss-drivers, David Beckett
In-Reply-To: <20171002183509.76b2cc65@cakuba>
On 10/03/2017 03:35 AM, Jakub Kicinski wrote:
> On Mon, 2 Oct 2017 17:55:02 -0700, Alexei Starovoitov wrote:
>>> +EXAMPLES
>>> +========
>>> +**# bpftool prog show**
>>> +::
>>> +
>>> + 10: xdp name:some_prog tag 00:5a:3d:21:23:62:0c:8b
>>
>> could you please remove ':' in the output to match what
>> show_fdinfo and kallsyms do ?
Yep, iproute2 as well, so would be better indeed.
> Ack.
>
>>> + loaded_at:2024.771 uid:0
>>
>> may be translate that to something human readable?
>
> Oh yes, the code will print a proper date/time, I forgot to
> regenerate the doc :S
>
>>> + xlated:528B jited:370B memlock:4096B map_ids:10
>>> +
>>> +|
>>> +| **# bpftool prog dump xlated id 10 file /tmp/t**
>>> +| **# ls -l /tmp/t**
>>> +| -rw------- 1 root root 560 Jul 22 01:42 /tmp/t
>>> +
>>> +|
>>> +| **# mount -t bpf none /sys/fs/bpf/**
>>> +| **# bpftool prog pin id 10 /sys/fs/bpf/prog**
>>> +| **# bpftool prog dum jited pinned /sys/fs/bpf/prog**
>>> +
>>> +::
>>> +
>>> + push %rbp
>>> + mov %rsp,%rbp
>>> + sub $0x228,%rsp
>>> + sub $0x28,%rbp
>>> + mov %rbx,0x0(%rbp)
>>
>> imo too many steps to dump disasm output.
>> Can it print it if we just say:
>> bpftool prog dump jited id 10
>> and
>> dump xlated
>
> Yes those will work. This example kind of shows pinning and dumping at
> the some time. Perhaps that's ill advised.
>
>> will pretty print them as verifier output as well?
>
> We tried to use LLVM as a library for this but the interface is
> painfully unstable and it's a heavy dependency. The current thinking
> is to try to put the instruction printing code in some higher level
> library, but I would rather leave that as a follow up.
>
>> All that can be changed later. Thanks for the doc.
Fine with that as well, so:
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] libbpf: parse maps sections of varying size
From: Jesper Dangaard Brouer @ 2017-10-03 14:11 UTC (permalink / raw)
To: Craig Gallek
Cc: Alexei Starovoitov, Daniel Borkmann, David S . Miller,
Chonggang Li, netdev, brouer, Eric Leblond
In-Reply-To: <20171002164129.47986-2-kraigatgoog@gmail.com>
On Mon, 2 Oct 2017 12:41:28 -0400
Craig Gallek <kraigatgoog@gmail.com> wrote:
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 4f402dcdf372..28b300868ad7 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -580,7 +580,7 @@ bpf_object__init_kversion(struct bpf_object *obj,
> }
>
> static int
> -bpf_object__validate_maps(struct bpf_object *obj)
> +bpf_object__validate_maps(struct bpf_object *obj, int map_def_sz)
> {
> int i;
>
> @@ -595,9 +595,11 @@ bpf_object__validate_maps(struct bpf_object *obj)
> const struct bpf_map *a = &obj->maps[i - 1];
> const struct bpf_map *b = &obj->maps[i];
>
> - if (b->offset - a->offset < sizeof(struct bpf_map_def)) {
> - pr_warning("corrupted map section in %s: map \"%s\" too small\n",
> - obj->path, a->name);
> + if (b->offset - a->offset < map_def_sz) {
> + pr_warning("corrupted map section in %s: map \"%s\" too small "
> + "(%zd vs %d)\n",
> + obj->path, a->name, b->offset - a->offset,
> + map_def_sz);
> return -EINVAL;
Hmm... one more comment. You have just coded handling of ELF
map_def_sz which are smaller in a safe manor, but here this case will
get rejected (in bpf_object__validate_maps). That cannot be the right
intend?
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] libbpf: parse maps sections of varying size
From: Jesper Dangaard Brouer @ 2017-10-03 14:03 UTC (permalink / raw)
To: Craig Gallek
Cc: Alexei Starovoitov, Daniel Borkmann, David S . Miller,
Chonggang Li, netdev, brouer, Eric Leblond, Wangnan (F)
In-Reply-To: <20171002164129.47986-2-kraigatgoog@gmail.com>
First of all, thank you Craig for working on this. As Alexei says, we
need to improve tools/lib/bpf/libbpf and move towards converting users
of bpf_load.c to this lib instead.
Comments inlined below.
On Mon, 2 Oct 2017 12:41:28 -0400 Craig Gallek <kraigatgoog@gmail.com> wrote:
> From: Craig Gallek <kraig@google.com>
>
> This library previously assumed a fixed-size map options structure.
> Any new options were ignored. In order to allow the options structure
> to grow and to support parsing older programs, this patch updates
> the maps section parsing to handle varying sizes.
>
> Object files with maps sections smaller than expected will have the new
> fields initialized to zero. Object files which have larger than expected
> maps sections will be rejected unless all of the unrecognized data is zero.
>
> This change still assumes that each map definition in the maps section
> is the same size.
>
> Signed-off-by: Craig Gallek <kraig@google.com>
> ---
> tools/lib/bpf/libbpf.c | 54 ++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 46 insertions(+), 8 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 4f402dcdf372..28b300868ad7 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -580,7 +580,7 @@ bpf_object__init_kversion(struct bpf_object *obj,
> }
>
> static int
> -bpf_object__validate_maps(struct bpf_object *obj)
> +bpf_object__validate_maps(struct bpf_object *obj, int map_def_sz)
> {
> int i;
>
> @@ -595,9 +595,11 @@ bpf_object__validate_maps(struct bpf_object *obj)
> const struct bpf_map *a = &obj->maps[i - 1];
> const struct bpf_map *b = &obj->maps[i];
>
> - if (b->offset - a->offset < sizeof(struct bpf_map_def)) {
> - pr_warning("corrupted map section in %s: map \"%s\" too small\n",
> - obj->path, a->name);
> + if (b->offset - a->offset < map_def_sz) {
> + pr_warning("corrupted map section in %s: map \"%s\" too small "
> + "(%zd vs %d)\n",
> + obj->path, a->name, b->offset - a->offset,
> + map_def_sz);
> return -EINVAL;
> }
> }
> @@ -615,7 +617,7 @@ static int compare_bpf_map(const void *_a, const void *_b)
> static int
> bpf_object__init_maps(struct bpf_object *obj)
> {
> - int i, map_idx, nr_maps = 0;
> + int i, map_idx, map_def_sz, nr_maps = 0;
> Elf_Scn *scn;
> Elf_Data *data;
> Elf_Data *symbols = obj->efile.symbols;
> @@ -658,6 +660,15 @@ bpf_object__init_maps(struct bpf_object *obj)
> if (!nr_maps)
> return 0;
>
> + /* Assume equally sized map definitions */
> + map_def_sz = data->d_size / nr_maps;
> + if (!data->d_size || (data->d_size % nr_maps) != 0) {
> + pr_warning("unable to determine map definition size "
> + "section %s, %d maps in %zd bytes\n",
> + obj->path, nr_maps, data->d_size);
> + return -EINVAL;
> + }
> +
> obj->maps = calloc(nr_maps, sizeof(obj->maps[0]));
> if (!obj->maps) {
> pr_warning("alloc maps for object failed\n");
> @@ -690,7 +701,7 @@ bpf_object__init_maps(struct bpf_object *obj)
> obj->efile.strtabidx,
> sym.st_name);
> obj->maps[map_idx].offset = sym.st_value;
> - if (sym.st_value + sizeof(struct bpf_map_def) > data->d_size) {
> + if (sym.st_value + map_def_sz > data->d_size) {
> pr_warning("corrupted maps section in %s: last map \"%s\" too small\n",
> obj->path, map_name);
> return -EINVAL;
> @@ -704,12 +715,39 @@ bpf_object__init_maps(struct bpf_object *obj)
> pr_debug("map %d is \"%s\"\n", map_idx,
> obj->maps[map_idx].name);
> def = (struct bpf_map_def *)(data->d_buf + sym.st_value);
> - obj->maps[map_idx].def = *def;
> + /*
> + * If the definition of the map in the object file fits in
> + * bpf_map_def, copy it. Any extra fields in our version
> + * of bpf_map_def will default to zero as a result of the
> + * calloc above.
> + */
> + if (map_def_sz <= sizeof(struct bpf_map_def)) {
> + memcpy(&obj->maps[map_idx].def, def, map_def_sz);
> + } else {
> + /*
> + * Here the map structure being read is bigger than what
> + * we expect, truncate if the excess bits are all zero.
> + * If they are not zero, reject this map as
> + * incompatible.
> + */
> + char *b;
> + for (b = ((char *)def) + sizeof(struct bpf_map_def);
> + b < ((char *)def) + map_def_sz; b++) {
> + if (*b != 0) {
> + pr_warning("maps section in %s: \"%s\" "
> + "has unrecognized, non-zero "
> + "options\n",
> + obj->path, map_name);
> + return -EINVAL;
> + }
> + }
> + obj->maps[map_idx].def = *def;
I'm not too happy/comfortable with this way of copying the memory of
"def" (the type-cased struct bpf_map_def). I guess it works, and is
part of the C-standard(?).
> + }
> map_idx++;
> }
>
> qsort(obj->maps, obj->nr_maps, sizeof(obj->maps[0]), compare_bpf_map);
> - return bpf_object__validate_maps(obj);
> + return bpf_object__validate_maps(obj, map_def_sz);
> }
>
> static int bpf_object__elf_collect(struct bpf_object *obj)
Besides above comment, I think the patch is correct, based on what I
did in commit 156450d9d964 ("samples/bpf: make bpf_load.c code
compatible with ELF maps section changes").
https://git.kernel.org/torvalds/c/156450d9d964
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH] rndis_host: support Novatel Verizon USB730L
From: Bjørn Mork @ 2017-10-03 14:01 UTC (permalink / raw)
To: David Miller
Cc: aleksander-Dvg4H30XQSRVIjRurl1/8g, oliver-GvhC2dPhHPQdnm+yROfE0A,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171002.161750.1123671129875495210.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> writes:
> From: Aleksander Morgado <aleksander-Dvg4H30XQSRVIjRurl1/8g@public.gmane.org>
> Date: Wed, 27 Sep 2017 23:31:03 +0200
>
>> I'm not sure if binding this logic to a specific vid:pid (1410:9030)
>> would be more appropriate here, or if it's ok to just bind
>> class/subclass/protocol (as in the activesync case). Let me know
>> what you think.
>
> I don't have enough USB Networking knowledge to make a decision here.
>
> Some things seem confusing. For example, if we should be matching
> USB_CLASS_MISC, subclass=4, protocol=1 for RNDIS over Ethernet, then
> we why aren't we also matching USB_CLASS_MISC, subclass=4, protocol=2
> for RNDIS over wireless and instead are matching "Remote RNDIS" in
> the form of USB_CLASS_WIRELSS, subclass=1, protocol=3?
>
> I really don't understand any of this magic :-)
>
> So someone more knowledgable needs to review this.
Let me just say that I am not qualified. But since this needs a review,
I am going to offer my view anyway. FWIW, I don't think *anyone*
understand this magic... I certainly don't.
We can pretty much ignore the USB-IF and any specs, since that is what
the vendors appear to do. They provide device specific drivers for
Windows, so all they care about is that their device "works" with their
driver.
But in Linux we prefer to create drivers for device classes whenever we
can, to avoid having to add every single device by ID. So we try to
guess future patterns based on the devices we have observed, even when
there is no clear spec. This is what Aleksander does here. He has a
device with a 'Cls=ef(misc ) Sub=04 Prot=01' function. This device
works with the rndis_host driver. That is all we know.
We cannot prove that a class match is correct. But it does make sense to
try it. At least we know that this works for one device.
Adding anything else, e.g. based on the table at
http://www.usb.org/developers/defined_class/#BaseClassEFh , is a bit
more risky. We don't know if a driver will work with *any* such device
until we've actually seen one.
This is just my opinion, and probably full of bogus assumptions as
usual. I was sort of hoping that some expert would speak up so I didn't
have to :-)
But FWIW:
Reviewed-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] netfilter: SYNPROXY: fix process non tcp packet bug in {ipv4,ipv6}_synproxy_hook
From: Pablo Neira Ayuso @ 2017-10-03 13:32 UTC (permalink / raw)
To: Lin Zhang
Cc: kadlec, fw, davem, kuznet, yoshfuji, netfilter-devel, coreteam,
netdev
In-Reply-To: <20171003132825.GA11182@salvia>
On Tue, Oct 03, 2017 at 03:28:25PM +0200, Pablo Neira Ayuso wrote:
> On Sat, Sep 30, 2017 at 06:25:15PM +0800, Lin Zhang wrote:
> > In function {ipv4,ipv6}_synproxy_hook we expect a normal tcp packet,
> > but the real server maybe reply an icmp error packet related to the
> > exist tcp conntrack, so we will access wrong tcp data.
> >
> > For fix it, check for the protocol field and only process tcp traffic.
>
> Applied, thanks.
>
> I have made minor comestic changes to patch title:
>
> netfilter: SYNPROXY: skip non-TCP packets from {ipv4,ipv6}_synproxy_hook
>
> for the record.
I have to keep this back, sorry.
This has been not compiled tested.
net/ipv6/netfilter/ip6t_SYNPROXY.c: In function ‘ipv6_synproxy_hook’:
net/ipv6/netfilter/ip6t_SYNPROXY.c:351:19: error: ‘struct ipv6hdr’ has
no member named ‘protocol’
ipv6_hdr(skb)->protocol != IPPROTO_TCP)
^
^ permalink raw reply
* Re: [PATCH v2] netfilter: SYNPROXY: fix process non tcp packet bug in {ipv4,ipv6}_synproxy_hook
From: Pablo Neira Ayuso @ 2017-10-03 13:28 UTC (permalink / raw)
To: Lin Zhang
Cc: kadlec, fw, davem, kuznet, yoshfuji, netfilter-devel, coreteam,
netdev
In-Reply-To: <1506767115-10051-1-git-send-email-xiaolou4617@gmail.com>
On Sat, Sep 30, 2017 at 06:25:15PM +0800, Lin Zhang wrote:
> In function {ipv4,ipv6}_synproxy_hook we expect a normal tcp packet,
> but the real server maybe reply an icmp error packet related to the
> exist tcp conntrack, so we will access wrong tcp data.
>
> For fix it, check for the protocol field and only process tcp traffic.
Applied, thanks.
I have made minor comestic changes to patch title:
netfilter: SYNPROXY: skip non-TCP packets from {ipv4,ipv6}_synproxy_hook
for the record.
^ permalink raw reply
* Re: [PATCH v2 net-next 06/12] qed: Add LL2 slowpath handling
From: Leon Romanovsky @ 2017-10-03 13:26 UTC (permalink / raw)
To: Michal Kalderon
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dledford-H+wXaHxf7aLQT0dZR+AlfA, Ariel Elior
In-Reply-To: <1507020902-4952-7-git-send-email-Michal.Kalderon-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3933 bytes --]
On Tue, Oct 03, 2017 at 11:54:56AM +0300, Michal Kalderon wrote:
> For iWARP unaligned MPA flow, a slowpath event of flushing an
> MPA connection that entered an unaligned state is required.
> The flush ramrod is received on the ll2 queue, and a pre-registered
> callback function is called to handle the flush event.
>
> Signed-off-by: Michal Kalderon <Michal.Kalderon-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Ariel Elior <Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> ---
> drivers/net/ethernet/qlogic/qed/qed_ll2.c | 40 +++++++++++++++++++++++++++++--
> include/linux/qed/qed_ll2_if.h | 5 ++++
> 2 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
> index 8eb9645..047f556 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
> @@ -423,6 +423,41 @@ static void qed_ll2_rxq_parse_reg(struct qed_hwfn *p_hwfn,
> }
>
> static int
> +qed_ll2_handle_slowpath(struct qed_hwfn *p_hwfn,
> + struct qed_ll2_info *p_ll2_conn,
> + union core_rx_cqe_union *p_cqe,
> + unsigned long *p_lock_flags)
> +{
> + struct qed_ll2_rx_queue *p_rx = &p_ll2_conn->rx_queue;
> + struct core_rx_slow_path_cqe *sp_cqe;
> +
> + sp_cqe = &p_cqe->rx_cqe_sp;
> + if (sp_cqe->ramrod_cmd_id != CORE_RAMROD_RX_QUEUE_FLUSH) {
> + DP_NOTICE(p_hwfn,
> + "LL2 - unexpected Rx CQE slowpath ramrod_cmd_id:%d\n",
> + sp_cqe->ramrod_cmd_id);
> + return -EINVAL;
> + }
> +
> + if (!p_ll2_conn->cbs.slowpath_cb) {
> + DP_NOTICE(p_hwfn,
> + "LL2 - received RX_QUEUE_FLUSH but no callback was provided\n");
> + return -EINVAL;
> + }
> +
> + spin_unlock_irqrestore(&p_rx->lock, *p_lock_flags);
Interesting, you are unlock the lock which was taken in upper layer.
It is not actual error, but chances to have such error are pretty high
(for example, after refactoring).
> +
> + p_ll2_conn->cbs.slowpath_cb(p_ll2_conn->cbs.cookie,
> + p_ll2_conn->my_id,
> + le32_to_cpu(sp_cqe->opaque_data.data[0]),
> + le32_to_cpu(sp_cqe->opaque_data.data[1]));
> +
> + spin_lock_irqsave(&p_rx->lock, *p_lock_flags);
> +
> + return 0;
> +}
> +
> +static int
> qed_ll2_rxq_handle_completion(struct qed_hwfn *p_hwfn,
> struct qed_ll2_info *p_ll2_conn,
> union core_rx_cqe_union *p_cqe,
> @@ -495,8 +530,8 @@ static int qed_ll2_rxq_completion(struct qed_hwfn *p_hwfn, void *cookie)
>
> switch (cqe->rx_cqe_sp.type) {
> case CORE_RX_CQE_TYPE_SLOW_PATH:
> - DP_NOTICE(p_hwfn, "LL2 - unexpected Rx CQE slowpath\n");
> - rc = -EINVAL;
> + rc = qed_ll2_handle_slowpath(p_hwfn, p_ll2_conn,
> + cqe, &flags);
> break;
> case CORE_RX_CQE_TYPE_GSI_OFFLOAD:
> case CORE_RX_CQE_TYPE_REGULAR:
> @@ -1214,6 +1249,7 @@ static int qed_ll2_acquire_connection_tx(struct qed_hwfn *p_hwfn,
> p_ll2_info->cbs.rx_release_cb = cbs->rx_release_cb;
> p_ll2_info->cbs.tx_comp_cb = cbs->tx_comp_cb;
> p_ll2_info->cbs.tx_release_cb = cbs->tx_release_cb;
> + p_ll2_info->cbs.slowpath_cb = cbs->slowpath_cb;
> p_ll2_info->cbs.cookie = cbs->cookie;
>
> return 0;
> diff --git a/include/linux/qed/qed_ll2_if.h b/include/linux/qed/qed_ll2_if.h
> index 95fdf02..e755954 100644
> --- a/include/linux/qed/qed_ll2_if.h
> +++ b/include/linux/qed/qed_ll2_if.h
> @@ -151,11 +151,16 @@ struct qed_ll2_comp_rx_data {
> dma_addr_t first_frag_addr,
> bool b_last_fragment, bool b_last_packet);
>
> +typedef
> +void (*qed_ll2_slowpath_cb)(void *cxt, u8 connection_handle,
> + u32 opaque_data_0, u32 opaque_data_1);
> +
> struct qed_ll2_cbs {
> qed_ll2_complete_rx_packet_cb rx_comp_cb;
> qed_ll2_release_rx_packet_cb rx_release_cb;
> qed_ll2_complete_tx_packet_cb tx_comp_cb;
> qed_ll2_release_tx_packet_cb tx_release_cb;
> + qed_ll2_slowpath_cb slowpath_cb;
> void *cookie;
> };
>
> --
> 1.8.3.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] netfilter: nf_tables: Release memory obtained by kasprintf
From: Pablo Neira Ayuso @ 2017-10-03 13:21 UTC (permalink / raw)
To: Arvind Yadav
Cc: kadlec, fw, davem, netfilter-devel, coreteam, netdev,
linux-kernel
In-Reply-To: <385554261c080cd3fc4adc093e68366a6d3dff77.1505889128.git.arvind.yadav.cs@gmail.com>
On Wed, Sep 20, 2017 at 12:31:28PM +0530, Arvind Yadav wrote:
> Free memory region, if nf_tables_set_alloc_name is not successful.
Applied, thanks.
I have added this tag to this patch:
Fixes: 387454901bd6 ("netfilter: nf_tables: Allow set names of up to 255 chars")
^ permalink raw reply
* Re: [PATCH] fsl/fman: remove of_node
From: Andrew Lunn @ 2017-10-03 13:00 UTC (permalink / raw)
To: Madalin-cristian Bucur
Cc: David Miller, netdev@vger.kernel.org, f.fainelli@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <AM5PR0402MB26914CBD37E3F28C17562258EC720@AM5PR0402MB2691.eurprd04.prod.outlook.com>
On Tue, Oct 03, 2017 at 08:49:31AM +0000, Madalin-cristian Bucur wrote:
> > -----Original Message-----
> > From: David Miller [mailto:davem@davemloft.net]
> > Sent: Tuesday, October 03, 2017 2:05 AM
> > To: Madalin-cristian Bucur <madalin.bucur@nxp.com>
> > Subject: Re: [PATCH] fsl/fman: remove of_node
> >
> > From: Madalin Bucur <madalin.bucur@nxp.com>
> > Date: Mon, 2 Oct 2017 13:31:37 +0300
> >
> > > The FMan MAC driver allocates a platform device for the Ethernet
> > > driver to probe on. Setting pdev->dev.of_node with the MAC node
> > > triggers the MAC driver probing of the new platform device. While
> > > this fails quickly and does not affect the functionality of the
> > > drivers, it is incorrect and must be removed. This was added to
> > > address a report that DSA code using of_find_net_device_by_node()
> > > is unable to use the DPAA interfaces. Error message seen before
> > > this fix:
> > >
> > > fsl_mac dpaa-ethernet.0: __devm_request_mem_region(mac) failed
> > > fsl_mac: probe of dpaa-ethernet.0 failed with error -16
> > >
> > > Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
> >
> > Is the DSA issue no longer something we need to be concerned
> > about? If not, why? You have to explain this.
>
> My patch removes the of_node that was set to a device that was not an
> of_device, preventing duplicated probing of both the real of_device
> and the "fake" one created through this assignment.
>
> I understand that the DSA issue that triggered the initial change
> was related to DSA finding the network devices using
> of_find_net_device_by_node(), something that will not work for the
> DPAA case where the netdevice does not have an of_node. I do not know
> enough about DSA to come up with a solution for this problem now.
> Andrew, Florian, can you please comment on this?
>
> Thanks,
Hi Madalin
I guess the real fix is to throw away the platform device. But that is
a big change.
I've not looked at the code in detail. Why is the platform device
needed?
Andrew
^ permalink raw reply
* (unknown),
From: marketing @ 2017-10-03 12:43 UTC (permalink / raw)
To: netdev
[-- Attachment #1: 2303159403401.zip --]
[-- Type: application/zip, Size: 7286 bytes --]
^ permalink raw reply
* Re: [PATCH] net: dsa: mt7530: make functions mt7530_phy_write static
From: Andrew Lunn @ 2017-10-03 12:42 UTC (permalink / raw)
To: Colin King
Cc: Vivien Didelot, Florian Fainelli, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20171003104633.27151-1-colin.king@canonical.com>
On Tue, Oct 03, 2017 at 11:46:33AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The function mt7530_phy_write is local to the source and does not need to
> be in global scope, so make it static.
>
> Cleans up sparse warnings:
> symbol 'mt7530_phy_write' was not declared. Should it be static?
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH] net: dsa: lan9303: make functions lan9303_mdio_phy_{read|write} static
From: Andrew Lunn @ 2017-10-03 12:41 UTC (permalink / raw)
To: Colin King
Cc: Vivien Didelot, Florian Fainelli, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20171003103918.26934-1-colin.king@canonical.com>
On Tue, Oct 03, 2017 at 11:39:18AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The functions lan9303_mdio_phy_write and lan9303_mdio_phy_read are local
> to the source and do not need to be in global scope, so make them static.
>
> Cleans up sparse warnings:
> symbol 'lan9303_mdio_phy_write' was not declared. Should it be static?
> symbol 'lan9303_mdio_phy_read' was not declared. Should it be static?
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net] net: br: Fix igmp snooping offload with CONFIG_BRIDGE_VLAN_FILTERING
From: Andrew Lunn @ 2017-10-03 12:16 UTC (permalink / raw)
To: Toshiaki Makita; +Cc: David Miller, Vivien Didelot, netdev
In-Reply-To: <37af5488-a064-37dc-b1ce-373119ae7b05@lab.ntt.co.jp>
On Tue, Oct 03, 2017 at 12:29:56PM +0900, Toshiaki Makita wrote:
> On 2017/10/03 9:55, Andrew Lunn wrote:
> > With CONFIG_BRIDGE_VLAN_FILTERING enabled, but the feature not enabled
> > via /sys/class/net/brX/bridge/vlan_filtering, mdb offloaded to the
> > kernel have the wrong VID.
> >
> > When an interface is added to the bridge, switchdev is first used to
> > notify the hardware that a port has joined a bridge. This is
> > immediately followed by the default_pvid, 1, being added to the
> > interface via another switchdev call.
> >
> > The bridge will then perform IGMP snooping, and offload an mdb entries
> > to the switch as needed. With vlan filtering disabled, the vid is left
> > as 0. This causes the switch to put the static mdb into the wrong
> > vlan, and so frames are not forwarded by the mdb entry.
> >
> > If vlan filtering is disable, use the default_pvid, not 0.
> >
> > Fixes: f1fecb1d10ec ("bridge: Reflect MDB entries to hardware")
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> > net/bridge/br_vlan.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> > index 233a30040c91..aa3589891797 100644
> > --- a/net/bridge/br_vlan.c
> > +++ b/net/bridge/br_vlan.c
> > @@ -492,6 +492,7 @@ bool br_allowed_ingress(const struct net_bridge *br,
> > */
> > if (!br->vlan_enabled) {
> > BR_INPUT_SKB_CB(skb)->vlan_filtered = false;
> > + *vid = br_get_pvid(vg);
> > return true;
> > }
> >
>
> This does not look correct.
> This will update fdb with vid which is not 0.
> Pvid can be different between each port even when vlan_filtering is
> disabled so unicast forwarding (fdb learning) will break.
> Also, fdb is visible to userspace so this can break userspace which
> expects fdb entries with 0 as well.
>
> Why does the switch driver use pvid while vlan_filtering is disabled?
Hi Toshiaki
We get a vlan added to the port. I think it comes from a combination
of:
int br_vlan_init(struct net_bridge *br)
{
struct net_bridge_vlan_group *vg;
int ret = -ENOMEM;
vg = kzalloc(sizeof(*vg), GFP_KERNEL);
if (!vg)
goto out;
ret = rhashtable_init(&vg->vlan_hash, &br_vlan_rht_params);
if (ret)
goto err_rhtbl;
ret = vlan_tunnel_init(vg);
if (ret)
goto err_tunnel_init;
INIT_LIST_HEAD(&vg->vlan_list);
br->vlan_proto = htons(ETH_P_8021Q);
br->default_pvid = 1;
and
int nbp_vlan_init(struct net_bridge_port *p)
{
struct switchdev_attr attr = {
.orig_dev = p->br->dev,
.id = SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING,
.flags = SWITCHDEV_F_SKIP_EOPNOTSUPP,
.u.vlan_filtering = p->br->vlan_enabled,
};
struct net_bridge_vlan_group *vg;
int ret = -ENOMEM;
vg = kzalloc(sizeof(struct net_bridge_vlan_group), GFP_KERNEL);
if (!vg)
goto out;
ret = switchdev_port_attr_set(p->dev, &attr);
if (ret && ret != -EOPNOTSUPP)
goto err_vlan_enabled;
ret = rhashtable_init(&vg->vlan_hash, &br_vlan_rht_params);
if (ret)
goto err_rhtbl;
ret = vlan_tunnel_init(vg);
if (ret)
goto err_tunnel_init;
INIT_LIST_HEAD(&vg->vlan_list);
rcu_assign_pointer(p->vlgrp, vg);
if (p->br->default_pvid) {
ret = nbp_vlan_add(p, p->br->default_pvid,
BRIDGE_VLAN_INFO_PVID |
BRIDGE_VLAN_INFO_UNTAGGED);
Now, i just noticed the switchdev call above. I don't think the DSA
layer implements SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING. It probably
should. So what is it supposed to do with this VLAN when filtering is
disabled?
Andrew
^ permalink raw reply
* [PATCH net-next] dev: advertise the new nsid when the netns iface changes
From: Nicolas Dichtel @ 2017-10-03 11:53 UTC (permalink / raw)
To: netdev; +Cc: davem, Nicolas Dichtel, Jason A . Donenfeld
In-Reply-To: <52f84baf-8027-d01f-8ece-db4f39a2f76f@6wind.com>
x-netns interfaces are bound to two netns: the link netns and the upper
netns. Usually, this kind of interfaces is created in the link netns and
then moved to the upper netns. At the end, the interface is visible only
in the upper netns. The link nsid is advertised via netlink in the upper
netns, thus the user always knows where is the link part.
There is no such mechanism in the link netns. When the interface is moved
to another netns, the user cannot "follow" it.
This patch adds a new netlink attribute which helps to follow an interface
which moves to another netns. When the interface is unregistered, the new
nsid is advertised. If the interface is a x-netns interface (ie
rtnl_link_ops->get_link_net is defined), the nsid is allocated if needed.
CC: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
include/linux/rtnetlink.h | 4 +++-
include/uapi/linux/if_link.h | 1 +
net/core/dev.c | 11 ++++++++---
net/core/rtnetlink.c | 31 ++++++++++++++++++++++---------
4 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index dea59c8eec54..1251638e60d3 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -17,9 +17,11 @@ extern int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst,
u32 id, long expires, u32 error);
void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change, gfp_t flags);
+void rtmsg_ifinfo_newnet(int type, struct net_device *dev, unsigned int change,
+ gfp_t flags, int *new_nsid);
struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
unsigned change, u32 event,
- gfp_t flags);
+ gfp_t flags, int *new_nsid);
void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev,
gfp_t flags);
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index ea87bd708ee9..cd580fc0e58f 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -158,6 +158,7 @@ enum {
IFLA_PAD,
IFLA_XDP,
IFLA_EVENT,
+ IFLA_NEW_NETNSID,
__IFLA_MAX
};
diff --git a/net/core/dev.c b/net/core/dev.c
index e350c768d4b5..2341e9d64e02 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -145,6 +145,7 @@
#include <linux/crash_dump.h>
#include <linux/sctp.h>
#include <net/udp_tunnel.h>
+#include <linux/net_namespace.h>
#include "net-sysfs.h"
@@ -7178,7 +7179,7 @@ static void rollback_registered_many(struct list_head *head)
if (!dev->rtnl_link_ops ||
dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
skb = rtmsg_ifinfo_build_skb(RTM_DELLINK, dev, ~0U, 0,
- GFP_KERNEL);
+ GFP_KERNEL, NULL);
/*
* Flush the unicast and multicast chains
@@ -8265,7 +8266,7 @@ EXPORT_SYMBOL(unregister_netdev);
int dev_change_net_namespace(struct net_device *dev, struct net *net, const char *pat)
{
- int err;
+ int err, new_nsid;
ASSERT_RTNL();
@@ -8321,7 +8322,11 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
rcu_barrier();
call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL, dev);
- rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
+ if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net)
+ new_nsid = peernet2id_alloc(dev_net(dev), net);
+ else
+ new_nsid = peernet2id(dev_net(dev), net);
+ rtmsg_ifinfo_newnet(RTM_DELLINK, dev, ~0U, GFP_KERNEL, &new_nsid);
/*
* Flush the unicast and multicast chains
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index e6955da0d58d..5bec24c348bf 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -927,6 +927,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
+ nla_total_size(IFNAMSIZ) /* IFLA_PHYS_PORT_NAME */
+ rtnl_xdp_size() /* IFLA_XDP */
+ nla_total_size(4) /* IFLA_EVENT */
+ + nla_total_size(4) /* IFLA_NEW_NETNSID */
+ nla_total_size(1); /* IFLA_PROTO_DOWN */
}
@@ -1386,7 +1387,7 @@ static int rtnl_fill_link_netnsid(struct sk_buff *skb,
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask,
- u32 event)
+ u32 event, int *new_nsid)
{
struct ifinfomsg *ifm;
struct nlmsghdr *nlh;
@@ -1475,6 +1476,10 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
if (rtnl_fill_link_netnsid(skb, dev))
goto nla_put_failure;
+ if (new_nsid &&
+ nla_put_s32(skb, IFLA_NEW_NETNSID, *new_nsid) < 0)
+ goto nla_put_failure;
+
if (!(af_spec = nla_nest_start(skb, IFLA_AF_SPEC)))
goto nla_put_failure;
@@ -1704,7 +1709,7 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, 0,
flags,
- ext_filter_mask, 0);
+ ext_filter_mask, 0, NULL);
if (err < 0) {
if (likely(skb->len))
@@ -2817,7 +2822,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
return -ENOBUFS;
err = rtnl_fill_ifinfo(nskb, dev, RTM_NEWLINK, NETLINK_CB(skb).portid,
- nlh->nlmsg_seq, 0, 0, ext_filter_mask, 0);
+ nlh->nlmsg_seq, 0, 0, ext_filter_mask, 0, NULL);
if (err < 0) {
/* -EMSGSIZE implies BUG in if_nlmsg_size */
WARN_ON(err == -EMSGSIZE);
@@ -2902,7 +2907,7 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
unsigned int change,
- u32 event, gfp_t flags)
+ u32 event, gfp_t flags, int *new_nsid)
{
struct net *net = dev_net(dev);
struct sk_buff *skb;
@@ -2913,7 +2918,8 @@ struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
if (skb == NULL)
goto errout;
- err = rtnl_fill_ifinfo(skb, dev, type, 0, 0, change, 0, 0, event);
+ err = rtnl_fill_ifinfo(skb, dev, type, 0, 0, change, 0, 0, event,
+ new_nsid);
if (err < 0) {
/* -EMSGSIZE implies BUG in if_nlmsg_size() */
WARN_ON(err == -EMSGSIZE);
@@ -2936,14 +2942,14 @@ void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev, gfp_t flags)
static void rtmsg_ifinfo_event(int type, struct net_device *dev,
unsigned int change, u32 event,
- gfp_t flags)
+ gfp_t flags, int *new_nsid)
{
struct sk_buff *skb;
if (dev->reg_state != NETREG_REGISTERED)
return;
- skb = rtmsg_ifinfo_build_skb(type, dev, change, event, flags);
+ skb = rtmsg_ifinfo_build_skb(type, dev, change, event, flags, new_nsid);
if (skb)
rtmsg_ifinfo_send(skb, dev, flags);
}
@@ -2951,10 +2957,17 @@ static void rtmsg_ifinfo_event(int type, struct net_device *dev,
void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
gfp_t flags)
{
- rtmsg_ifinfo_event(type, dev, change, rtnl_get_event(0), flags);
+ rtmsg_ifinfo_event(type, dev, change, rtnl_get_event(0), flags, NULL);
}
EXPORT_SYMBOL(rtmsg_ifinfo);
+void rtmsg_ifinfo_newnet(int type, struct net_device *dev, unsigned int change,
+ gfp_t flags, int *new_nsid)
+{
+ rtmsg_ifinfo_event(type, dev, change, rtnl_get_event(0), flags,
+ new_nsid);
+}
+
static int nlmsg_populate_fdb_fill(struct sk_buff *skb,
struct net_device *dev,
u8 *addr, u16 vid, u32 pid, u32 seq,
@@ -4330,7 +4343,7 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
case NETDEV_RESEND_IGMP:
case NETDEV_CHANGEINFODATA:
rtmsg_ifinfo_event(RTM_NEWLINK, dev, 0, rtnl_get_event(event),
- GFP_KERNEL);
+ GFP_KERNEL, NULL);
break;
default:
break;
--
2.13.2
^ permalink raw reply related
* Re: [PATCH 1/2 net-next] mlxsw: spectrum: Fix check for IS_ERR() instead of NULL
From: Yotam Gigi @ 2017-10-03 10:58 UTC (permalink / raw)
To: Dan Carpenter, Jiri Pirko; +Cc: Ido Schimmel, netdev, kernel-janitors
In-Reply-To: <20171003105303.u7yrzxknddmmerol@mwanda>
On 10/03/2017 01:53 PM, Dan Carpenter wrote:
> mlxsw_afa_block_create() doesn't return error pointers, it returns NULL
> on error.
>
> Fixes: 0e14c7777acb ("mlxsw: spectrum: Add the multicast routing hardware logic")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Yotam Gigi <yotamg@mellanox.com>
Thanks!
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
> index cda9e9ad10e3..5e4ccbf17e3d 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
> @@ -239,8 +239,8 @@ mlxsw_sp_mr_tcam_afa_block_create(struct mlxsw_sp *mlxsw_sp,
> int err;
>
> afa_block = mlxsw_afa_block_create(mlxsw_sp->afa);
> - if (IS_ERR(afa_block))
> - return afa_block;
> + if (!afa_block)
> + return ERR_PTR(-ENOMEM);
>
> err = mlxsw_afa_block_append_counter(afa_block, counter_index);
> if (err)
^ permalink raw reply
* Re: [PATCH 2/2 net-next] mlxsw: spectrum: Add missing error code on allocation failure
From: Yotam Gigi @ 2017-10-03 10:56 UTC (permalink / raw)
To: Dan Carpenter, Jiri Pirko; +Cc: Ido Schimmel, netdev, kernel-janitors
In-Reply-To: <20171003105340.llwk5oajgrohbksu@mwanda>
On 10/03/2017 01:53 PM, Dan Carpenter wrote:
> We accidentally return success if the kmalloc_array() call fails.
>
> Fixes: 0e14c7777acb ("mlxsw: spectrum: Add the multicast routing hardware logic")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Yotam Gigi <yotamg@mellanox.com>
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
> index 5e4ccbf17e3d..839eadf0765b 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
> @@ -763,8 +763,10 @@ mlxsw_sp_mr_tcam_region_init(struct mlxsw_sp *mlxsw_sp,
>
> parman_prios = kmalloc_array(MLXSW_SP_MR_ROUTE_PRIO_MAX + 1,
> sizeof(*parman_prios), GFP_KERNEL);
> - if (!parman_prios)
> + if (!parman_prios) {
> + err = -ENOMEM;
> goto err_parman_prios_alloc;
> + }
> mr_tcam_region->parman_prios = parman_prios;
>
> for (i = 0; i < MLXSW_SP_MR_ROUTE_PRIO_MAX + 1; i++)
^ permalink raw reply
* [PATCH 2/2 net-next] mlxsw: spectrum: Add missing error code on allocation failure
From: Dan Carpenter @ 2017-10-03 10:53 UTC (permalink / raw)
To: Jiri Pirko, Yotam Gigi; +Cc: Ido Schimmel, netdev, kernel-janitors
In-Reply-To: <20171003105303.u7yrzxknddmmerol@mwanda>
We accidentally return success if the kmalloc_array() call fails.
Fixes: 0e14c7777acb ("mlxsw: spectrum: Add the multicast routing hardware logic")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
index 5e4ccbf17e3d..839eadf0765b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
@@ -763,8 +763,10 @@ mlxsw_sp_mr_tcam_region_init(struct mlxsw_sp *mlxsw_sp,
parman_prios = kmalloc_array(MLXSW_SP_MR_ROUTE_PRIO_MAX + 1,
sizeof(*parman_prios), GFP_KERNEL);
- if (!parman_prios)
+ if (!parman_prios) {
+ err = -ENOMEM;
goto err_parman_prios_alloc;
+ }
mr_tcam_region->parman_prios = parman_prios;
for (i = 0; i < MLXSW_SP_MR_ROUTE_PRIO_MAX + 1; i++)
^ permalink raw reply related
* [PATCH 1/2 net-next] mlxsw: spectrum: Fix check for IS_ERR() instead of NULL
From: Dan Carpenter @ 2017-10-03 10:53 UTC (permalink / raw)
To: Jiri Pirko, Yotam Gigi; +Cc: Ido Schimmel, netdev, kernel-janitors
mlxsw_afa_block_create() doesn't return error pointers, it returns NULL
on error.
Fixes: 0e14c7777acb ("mlxsw: spectrum: Add the multicast routing hardware logic")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
index cda9e9ad10e3..5e4ccbf17e3d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
@@ -239,8 +239,8 @@ mlxsw_sp_mr_tcam_afa_block_create(struct mlxsw_sp *mlxsw_sp,
int err;
afa_block = mlxsw_afa_block_create(mlxsw_sp->afa);
- if (IS_ERR(afa_block))
- return afa_block;
+ if (!afa_block)
+ return ERR_PTR(-ENOMEM);
err = mlxsw_afa_block_append_counter(afa_block, counter_index);
if (err)
^ permalink raw reply related
* [PATCH] net: dsa: mt7530: make functions mt7530_phy_write static
From: Colin King @ 2017-10-03 10:46 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The function mt7530_phy_write is local to the source and does not need to
be in global scope, so make it static.
Cleans up sparse warnings:
symbol 'mt7530_phy_write' was not declared. Should it be static?
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/dsa/mt7530.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index faa3b88d2206..034241696ce2 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -564,7 +564,8 @@ static int mt7530_phy_read(struct dsa_switch *ds, int port, int regnum)
return mdiobus_read_nested(priv->bus, port, regnum);
}
-int mt7530_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val)
+static int mt7530_phy_write(struct dsa_switch *ds, int port, int regnum,
+ u16 val)
{
struct mt7530_priv *priv = ds->priv;
--
2.14.1
^ permalink raw reply related
* [PATCH] net: dsa: lan9303: make functions lan9303_mdio_phy_{read|write} static
From: Colin King @ 2017-10-03 10:39 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The functions lan9303_mdio_phy_write and lan9303_mdio_phy_read are local
to the source and do not need to be in global scope, so make them static.
Cleans up sparse warnings:
symbol 'lan9303_mdio_phy_write' was not declared. Should it be static?
symbol 'lan9303_mdio_phy_read' was not declared. Should it be static?
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/dsa/lan9303_mdio.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/lan9303_mdio.c b/drivers/net/dsa/lan9303_mdio.c
index fc16668a487f..0bc56b9900f9 100644
--- a/drivers/net/dsa/lan9303_mdio.c
+++ b/drivers/net/dsa/lan9303_mdio.c
@@ -67,14 +67,15 @@ static int lan9303_mdio_read(void *ctx, uint32_t reg, uint32_t *val)
return 0;
}
-int lan9303_mdio_phy_write(struct lan9303 *chip, int phy, int reg, u16 val)
+static int lan9303_mdio_phy_write(struct lan9303 *chip, int phy, int reg,
+ u16 val)
{
struct lan9303_mdio *sw_dev = dev_get_drvdata(chip->dev);
return mdiobus_write_nested(sw_dev->device->bus, phy, reg, val);
}
-int lan9303_mdio_phy_read(struct lan9303 *chip, int phy, int reg)
+static int lan9303_mdio_phy_read(struct lan9303 *chip, int phy, int reg)
{
struct lan9303_mdio *sw_dev = dev_get_drvdata(chip->dev);
--
2.14.1
^ permalink raw reply related
* Re: [patch net-next v2 3/7] ipv4: ipmr: Don't forward packets already forwarded by hardware
From: Nikolay Aleksandrov @ 2017-10-03 10:26 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, yotamg, idosch, mlxsw, andrew, dsa, edumazet, willemb,
johannes.berg, dcaratti, pabeni, daniel, f.fainelli, fw,
gfree.wind
In-Reply-To: <20171003075812.1540-4-jiri@resnulli.us>
On 03/10/17 10:58, Jiri Pirko wrote:
> From: Yotam Gigi <yotamg@mellanox.com>
>
> Change the ipmr module to not forward packets if:
> - The packet is marked with the offload_mr_fwd_mark, and
> - Both input interface and output interface share the same parent ID.
>
> This way, a packet can go through partial multicast forwarding in the
> hardware, where it will be forwarded only to the devices that share the
> same parent ID (AKA, reside inside the same hardware). The kernel will
> forward the packet to all other interfaces.
>
> To do this, add the ipmr_offload_forward helper, which per skb, ingress VIF
> and egress VIF, returns whether the forwarding was offloaded to hardware.
> The ipmr_queue_xmit frees the skb and does not forward it if the result is
> a true value.
>
> All the forwarding path code compiles out when the CONFIG_NET_SWITCHDEV is
> not set.
>
> Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
> Reviewed-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> v1->v2:
> - Use dev_parent_id.len field instead of the dev_parent_id_valid field
> ---
> net/ipv4/ipmr.c | 37 ++++++++++++++++++++++++++++++++-----
> 1 file changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index 1b161ad..b3ee01b 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -1859,10 +1859,33 @@ static inline int ipmr_forward_finish(struct net *net, struct sock *sk,
> return dst_output(net, sk, skb);
> }
>
Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* Re: [patch net-next v2 2/7] ipv4: ipmr: Add the parent ID field to VIF struct
From: Nikolay Aleksandrov @ 2017-10-03 10:25 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, yotamg, idosch, mlxsw, andrew, dsa, edumazet, willemb,
johannes.berg, dcaratti, pabeni, daniel, f.fainelli, fw,
gfree.wind
In-Reply-To: <20171003075812.1540-3-jiri@resnulli.us>
On 03/10/17 10:58, Jiri Pirko wrote:
> From: Yotam Gigi <yotamg@mellanox.com>
>
> In order to allow the ipmr module to do partial multicast forwarding
> according to the device parent ID, add the device parent ID field to the
> VIF struct. This way, the forwarding path can use the parent ID field
> without invoking switchdev calls, which requires the RTNL lock.
>
> When a new VIF is added, set the device parent ID field in it by invoking
> the switchdev_port_attr_get call.
>
> Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
> Reviewed-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> v1->v2:
> - Set the vif->dev_parent_id.len field
> - Remove the vif->dev_parent_id_valid field and use len instead
> - Set the vif->dev_parent_id to invalid if the device does not support the
> switchdev PARENT_ID attribute
> ---
> include/linux/mroute.h | 1 +
> net/ipv4/ipmr.c | 11 +++++++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/include/linux/mroute.h b/include/linux/mroute.h
> index b072a84..8242d05 100644
> --- a/include/linux/mroute.h
> +++ b/include/linux/mroute.h
> @@ -57,6 +57,7 @@ static inline bool ipmr_rule_default(const struct fib_rule *rule)
>
> struct vif_device {
> struct net_device *dev; /* Device we are using */
> + struct netdev_phys_item_id dev_parent_id; /* Device parent ID */
> unsigned long bytes_in,bytes_out;
> unsigned long pkt_in,pkt_out; /* Statistics */
> unsigned long rate_limit; /* Traffic shaping (NI) */
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index a844738..1b161ad 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -67,6 +67,7 @@
> #include <net/fib_rules.h>
> #include <linux/netconf.h>
> #include <net/nexthop.h>
> +#include <net/switchdev.h>
>
> struct ipmr_rule {
> struct fib_rule common;
> @@ -868,6 +869,9 @@ static int vif_add(struct net *net, struct mr_table *mrt,
> struct vifctl *vifc, int mrtsock)
> {
> int vifi = vifc->vifc_vifi;
> + struct switchdev_attr attr = {
> + .id = SWITCHDEV_ATTR_ID_PORT_PARENT_ID,
> + };
> struct vif_device *v = &mrt->vif_table[vifi];
> struct net_device *dev;
> struct in_device *in_dev;
> @@ -942,6 +946,13 @@ static int vif_add(struct net *net, struct mr_table *mrt,
>
> /* Fill in the VIF structures */
>
> + attr.orig_dev = dev;
> + if (!switchdev_port_attr_get(dev, &attr)) {
> + memcpy(v->dev_parent_id.id, attr.u.ppid.id, attr.u.ppid.id_len);
> + v->dev_parent_id.id_len = attr.u.ppid.id_len;
> + } else {
> + v->dev_parent_id.id_len = 0;
> + }
> v->rate_limit = vifc->vifc_rate_limit;
> v->local = vifc->vifc_lcl_addr.s_addr;
> v->remote = vifc->vifc_rmt_addr.s_addr;
>
Looks good, thanks!
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* [PATCH net] net: rtnetlink: fix info leak in RTM_GETSTATS call
From: Nikolay Aleksandrov @ 2017-10-03 10:20 UTC (permalink / raw)
To: netdev
Cc: keescook, dvyukov, andreyknvl, kcc, roopa, glider, davem,
edumazet, Nikolay Aleksandrov
When RTM_GETSTATS was added the fields of its header struct were not all
initialized when returning the result thus leaking 4 bytes of information
to user-space per rtnl_fill_statsinfo call, so initialize them now. Thanks
to Alexander Potapenko for the detailed report and bisection.
Reported-by: Alexander Potapenko <glider@google.com>
Fixes: 10c9ead9f3c6 ("rtnetlink: add new RTM_GETSTATS message to dump link stats")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/core/rtnetlink.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a78fd61da0ec..d4bcdcc68e92 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3854,6 +3854,9 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
return -EMSGSIZE;
ifsm = nlmsg_data(nlh);
+ ifsm->family = PF_UNSPEC;
+ ifsm->pad1 = 0;
+ ifsm->pad2 = 0;
ifsm->ifindex = dev->ifindex;
ifsm->filter_mask = filter_mask;
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net-next 2/2] flow_dissector: dissect tunnel info
From: Simon Horman @ 2017-10-03 9:40 UTC (permalink / raw)
To: Tom Herbert
Cc: David Miller, Jiri Pirko, Jamal Hadi Salim, Cong Wang,
Linux Kernel Network Developers, oss-drivers
In-Reply-To: <CALx6S34ULzKX=Q+CSdEuXVAA12MMM1oQ=x7JEV8EmD-2qaviNA@mail.gmail.com>
On Mon, Oct 02, 2017 at 01:37:55PM -0700, Tom Herbert wrote:
> On Mon, Oct 2, 2017 at 1:41 AM, Simon Horman <simon.horman@netronome.com> wrote:
> > Move dissection of tunnel info from the flower classifier to the flow
> > dissector where all other dissection occurs. This should not have any
> > behavioural affect on other users of the flow dissector.
...
Hi Tom,
> Simon,
>
> I think I'm missing something fundamental here. This code is
> populating flow dissector keys not based on the contents of the packet
> like rest of the flow dissector, but on external meta data related to
> the packet which I believe is constant during the whole flow
> dissection.
Yes, I believe that is correct on all counts.
> Why can't this be handled by the caller?
It certainly can be. And indeed it was before this patch. But it seems odd
for some population of dissector keys to occur in the dissector and some
elsewhere.
I feel that we are circling back the perennial issue of flower using the
flow dissector in a somewhat broader/different way than many/all other
users of the flow dissector.
> Also, if I read this correctly, this code could be called multiple times
> and it seems like it does the exact same thing in each call.
I'm not sure what you are getting at there. If there are flower classifiers
for the same device at different priority levels then the dissection
will be called multiple times and the data in question cannot have changed
as far as I know. But this was also the case before this patch.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox