* Re: [PATCH v3 bpf-next 1/9] tcp: Use a struct to represent a saved_syn
From: Eric Dumazet @ 2020-07-31 15:57 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, kernel-team,
Lawrence Brakmo, Neal Cardwell, netdev, Yuchung Cheng
In-Reply-To: <20200730205704.3352619-1-kafai@fb.com>
On Thu, Jul 30, 2020 at 1:57 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> The TCP_SAVE_SYN has both the network header and tcp header.
> The total length of the saved syn packet is currently stored in
> the first 4 bytes (u32) of an array and the actual packet data is
> stored after that.
>
> A latter patch will add a bpf helper that allows to get the tcp header
s/latter/later/
> alone from the saved syn without the network header. It will be more
> convenient to have a direct offset to a specific header instead of
> re-parsing it. This requires to separately store the network hdrlen.
> The total header length (i.e. network + tcp) is still needed for the
> current usage in getsockopt. Although this total length can be obtained
> by looking into the tcphdr and then get the (th->doff << 2), this patch
> chooses to directly store the tcp hdrlen in the second four bytes of
> this newly created "struct saved_syn". By using a new struct, it can
> give a readable name to each individual header length.
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index a018bafd7bdf..6c38ca9de17e 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -6598,13 +6598,14 @@ static void tcp_reqsk_record_syn(const struct sock *sk,
> {
> if (tcp_sk(sk)->save_syn) {
> u32 len = skb_network_header_len(skb) + tcp_hdrlen(skb);
> - u32 *copy;
> -
> - copy = kmalloc(len + sizeof(u32), GFP_ATOMIC);
> - if (copy) {
> - copy[0] = len;
> - memcpy(©[1], skb_network_header(skb), len);
> - req->saved_syn = copy;
> + struct saved_syn *saved_syn;
> +
> + saved_syn = kmalloc(len + sizeof(*saved_syn), GFP_ATOMIC);
Please use
saved_syn = kmalloc(struct_size(saved_syn, data,
len), GFP_ATOMIC)
This will avoid yet another trivial patch in the future.
> + if (saved_syn) {
> + saved_syn->network_hdrlen = skb_network_header_len(skb);
> + saved_syn->tcp_hdrlen = tcp_hdrlen(skb);
> + memcpy(saved_syn->data, skb_network_header(skb), len);
> + req->saved_syn = saved_syn;
> }
> }
> }
> --
> 2.24.1
>
^ permalink raw reply
* Re: linux-next: Tree for Jul 31 (net/decnet/ & FIB_RULES)
From: Randy Dunlap @ 2020-07-31 15:53 UTC (permalink / raw)
To: Stephen Rothwell, Linux Next Mailing List
Cc: Linux Kernel Mailing List, netdev@vger.kernel.org,
linux-decnet-user
In-Reply-To: <20200731211909.33b550ec@canb.auug.org.au>
On 7/31/20 4:19 AM, Stephen Rothwell wrote:
> Hi all,
>
> Changes since 20200730:
>
on i386:
ld: net/core/fib_rules.o: in function `fib_rules_lookup':
fib_rules.c:(.text+0x16b8): undefined reference to `fib4_rule_match'
ld: fib_rules.c:(.text+0x16bf): undefined reference to `fib4_rule_match'
ld: fib_rules.c:(.text+0x170d): undefined reference to `fib4_rule_action'
ld: fib_rules.c:(.text+0x171e): undefined reference to `fib4_rule_action'
ld: fib_rules.c:(.text+0x1751): undefined reference to `fib4_rule_suppress'
ld: fib_rules.c:(.text+0x175d): undefined reference to `fib4_rule_suppress'
CONFIG_DECNET=y
CONFIG_DECNET_ROUTER=y
DECNET_ROUTER selects FIB_RULES.
--
~Randy
Reported-by: Randy Dunlap <rdunlap@infradead.org>
^ permalink raw reply
* Re: [PATCH v3 bpf-next 9/9] tcp: bpf: Optionally store mac header in TCP_SAVE_SYN
From: Eric Dumazet @ 2020-07-31 15:51 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, kernel-team,
Lawrence Brakmo, Neal Cardwell, netdev, Yuchung Cheng
In-Reply-To: <20200730205754.3355160-1-kafai@fb.com>
On Thu, Jul 30, 2020 at 1:58 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> This patch is adapted from Eric's patch in an earlier discussion [1].
>
> The TCP_SAVE_SYN currently only stores the network header and
> tcp header. This patch allows it to optionally store
> the mac header also if the setsockopt's optval is 2.
>
> It requires one more bit for the "save_syn" bit field in tcp_sock.
> This patch achieves this by moving the syn_smc bit next to the is_mptcp.
> The syn_smc is currently used with the TCP experimental option. Since
> syn_smc is only used when CONFIG_SMC is enabled, this patch also puts
> the "IS_ENABLED(CONFIG_SMC)" around it like the is_mptcp did
> with "IS_ENABLED(CONFIG_MPTCP)".
>
> The mac_hdrlen is also stored in the "struct saved_syn"
> to allow a quick offset from the bpf prog if it chooses to start
> getting from the network header or the tcp header.
>
> [1]: https://lore.kernel.org/netdev/CANn89iLJNWh6bkH7DNhy_kmcAexuUCccqERqe7z2QsvPhGrYPQ@mail.gmail.com/
>
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [net 0/2][pull request] Intel Wired LAN Driver Updates 2020-07-30
From: Nguyen, Anthony L @ 2020-07-31 15:33 UTC (permalink / raw)
To: kuba@kernel.org
Cc: nhorman@redhat.com, davem@davemloft.net, netdev@vger.kernel.org,
Kirsher, Jeffrey T, sassmann@redhat.com
In-Reply-To: <20200730162703.277cc07b@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Thu, 2020-07-30 at 16:27 -0700, Jakub Kicinski wrote:
> On Thu, 30 Jul 2020 10:09:36 -0700 Tony Nguyen wrote:
> > This series contains updates to the e1000e and igb drivers.
> >
> > Aaron Ma allows PHY initialization to continue if ULP disable
> > failed for
> > e1000e.
> >
> > Francesco Ruggeri fixes race conditions in igb reset that could
> > cause panics.
>
> Reviewed-by: Jakub Kicinski <kuba@kernel.org>
>
> In the future please try to add Fixes tags on all net submissions
> (patch 2).
Will do.
> Also - are similar fixes for other Intel drivers in the
> works?
Jeff said he had someone looking into it. I'll track down who's working
on it.
Thanks,
Tony
^ permalink raw reply
* Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY
From: Andrew Lunn @ 2020-07-31 15:31 UTC (permalink / raw)
To: Vikas Singh
Cc: f.fainelli, hkallweit1, linux, netdev, Calvin Johnson (OSS),
Kuldip Dwivedi, Madalin Bucur (OSS), Vikas Singh
In-Reply-To: <CADvVLtXVVfU3-U8DYPtDnvGoEK2TOXhpuE=1vz6nnXaFBA8pNA@mail.gmail.com>
On Wed, Jul 29, 2020 at 07:34:19PM +0530, Vikas Singh wrote:
> Hi Andrew,
>
> I understand your concern But my use case is pretty simple and straightforward.
> As you are already aware of the fact that operating systems running on standard
> server hardware requires standard firmware interfaces to be present in order to
> boot
> and function correctly.
> Here SBBR describes these firmware requirements which covers UEFI & ACPI.
> Therefore I would like to provide dual boot support to such systems which means
> supporting ACPI along with existing DT.
> Now kernels "__fixed_phy_registe() " being a wonderful implementation catters
> almost every thing required for both ACPI & DT
> But fails to register fixed PHYs in case of ACPI because it has no clue of the
> fwnode (of_node in case of DT) to attach with a PHY device.
You failed to answer my question. Why do you need a fixed PHY? Please
could you point me at your DT files so i can at least understand your
hardware.
Andrew
^ permalink raw reply
* Re: pull-request: bpf 2020-07-31
From: Jiri Olsa @ 2020-07-31 15:24 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, kuba, ast, jolsa, netdev, bpf
In-Reply-To: <20200731135145.15003-1-daniel@iogearbox.net>
On Fri, Jul 31, 2020 at 03:51:45PM +0200, Daniel Borkmann wrote:
> Hi David,
>
> The following pull-request contains BPF updates for your *net* tree.
>
> We've added 5 non-merge commits during the last 21 day(s) which contain
> a total of 5 files changed, 126 insertions(+), 18 deletions(-).
>
> The main changes are:
>
> 1) Fix a map element leak in HASH_OF_MAPS map type, from Andrii Nakryiko.
>
> 2) Fix a NULL pointer dereference in __btf_resolve_helper_id() when no
> btf_vmlinux is available, from Peilin Ye.
>
> 3) Init pos variable in __bpfilter_process_sockopt(), from Christoph Hellwig.
>
> 4) Fix a cgroup sockopt verifier test by specifying expected attach type,
> from Jean-Philippe Brucker.
>
> Please consider pulling these changes from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
>
> Thanks a lot!
>
> Note that when net gets merged into net-next later on, there is a small
> merge conflict in kernel/bpf/btf.c between commit 5b801dfb7feb ("bpf: Fix
> NULL pointer dereference in __btf_resolve_helper_id()") from the bpf tree
> and commit 138b9a0511c7 ("bpf: Remove btf_id helpers resolving") from the
> net-next tree.
>
> Resolve as follows: remove the old hunk with the __btf_resolve_helper_id()
> function. Change the btf_resolve_helper_id() so it actually tests for a
> NULL btf_vmlinux and bails out:
>
> int btf_resolve_helper_id(struct bpf_verifier_log *log,
> const struct bpf_func_proto *fn, int arg)
> {
> int id;
>
> if (fn->arg_type[arg] != ARG_PTR_TO_BTF_ID || !btf_vmlinux)
> return -EINVAL;
> id = fn->btf_id[arg];
> if (!id || id > btf_vmlinux->nr_types)
> return -EINVAL;
> return id;
> }
>
> Let me know if you run into any others issues (CC'ing Jiri Olsa so he's in
> the loop with regards to merge conflict resolution).
we'll loose the bpf_log message, but I'm fine with that ;-) looks good
thanks,
jirka
^ permalink raw reply
* Re: [net-next v4 1/6] net: marvell: prestera: Add driver for Prestera family ASIC devices
From: Vadym Kochan @ 2020-07-31 15:22 UTC (permalink / raw)
To: Andy Shevchenko
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, Ido Schimmel,
Andrew Lunn, Oleksandr Mazur, Serhiy Boiko, Serhiy Pshyk,
Volodymyr Mytnyk, Taras Chornyi, Andrii Savka, netdev,
Linux Kernel Mailing List, Mickey Rachamim
In-Reply-To: <CAHp75Ve-MyFg5QqHjywGk6X+v_F77HkRBuQsJ0Cx3WLJ5ZV43w@mail.gmail.com>
Hi Andy,
On Mon, Jul 27, 2020 at 03:59:13PM +0300, Andy Shevchenko wrote:
> On Mon, Jul 27, 2020 at 3:23 PM Vadym Kochan <vadym.kochan@plvision.eu> wrote:
> >
> > Marvell Prestera 98DX326x integrates up to 24 ports of 1GbE with 8
> > ports of 10GbE uplinks or 2 ports of 40Gbps stacking for a largely
> > wireless SMB deployment.
> >
> > The current implementation supports only boards designed for the Marvell
> > Switchdev solution and requires special firmware.
> >
> > The core Prestera switching logic is implemented in prestera_main.c,
> > there is an intermediate hw layer between core logic and firmware. It is
> > implemented in prestera_hw.c, the purpose of it is to encapsulate hw
> > related logic, in future there is a plan to support more devices with
> > different HW related configurations.
> >
> > This patch contains only basic switch initialization and RX/TX support
> > over SDMA mechanism.
> >
> > Currently supported devices have DMA access range <= 32bit and require
> > ZONE_DMA to be enabled, for such cases SDMA driver checks if the skb
> > allocated in proper range supported by the Prestera device.
> >
> > Also meanwhile there is no TX interrupt support in current firmware
> > version so recycling work is scheduled on each xmit.
> >
> > Port's mac address is generated from the switch base mac which may be
> > provided via device-tree (static one or as nvme cell), or randomly
> > generated.
>
> ...
>
> > Signed-off-by: Andrii Savka <andrii.savka@plvision.eu>
> > Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
> > Signed-off-by: Serhiy Boiko <serhiy.boiko@plvision.eu>
> > Signed-off-by: Serhiy Pshyk <serhiy.pshyk@plvision.eu>
> > Signed-off-by: Taras Chornyi <taras.chornyi@plvision.eu>
> > Signed-off-by: Volodymyr Mytnyk <volodymyr.mytnyk@plvision.eu>
> > Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu>
>
> This needs more work. You have to really understand the role of each
> person in the above list.
> I highly recommend (re-)read sections 11-13 of Submitting Patches.
>
At least looks like I need to add these persons as Co-author's.
> ...
>
> > +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
>
> The idea of SPDX is to have it as a separate (standalone) comment.
OK, thanks.
>
> ...
>
> > +enum prestera_event_type {
> > + PRESTERA_EVENT_TYPE_UNSPEC,
> > +
> > + PRESTERA_EVENT_TYPE_PORT,
> > + PRESTERA_EVENT_TYPE_RXTX,
> > +
> > + PRESTERA_EVENT_TYPE_MAX,
>
> Commas in the terminators are not good.
>
OK
> > +};
>
> ...
>
> > +#include "prestera_dsa.h"
>
> The idea that you include more generic headers earlier than more custom ones.
>
Thanks
> > +#include <linux/string.h>
> > +#include <linux/bitops.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/errno.h>
>
> Perhaps ordered?
>
alphabetical ?
> ...
>
> > +/* TrgDev[4:0] = {Word0[28:24]} */
>
> > + * SrcPort/TrgPort[7:0] = {Word2[20], Word1[11:10], Word0[23:19]}
>
> > +/* bits 13:15 -- UP */
>
> > +/* bits 0:11 -- VID */
>
> These are examples of useless comments.
>
OK, removed.
> ...
>
> > + dsa->vlan.is_tagged = (bool)FIELD_GET(PRESTERA_W0_IS_TAGGED, words[0]);
> > + dsa->vlan.cfi_bit = (u8)FIELD_GET(PRESTERA_W1_CFI_BIT, words[1]);
> > + dsa->vlan.vpt = (u8)FIELD_GET(PRESTERA_W0_VPT, words[0]);
> > + dsa->vlan.vid = (u16)FIELD_GET(PRESTERA_W0_VID, words[0]);
>
> Do you need those castings?
>
Looks like not, because the struct fields are same type as cast'ed ones.
> ...
>
> > + struct prestera_msg_event_port *hw_evt;
> > +
> > + hw_evt = (struct prestera_msg_event_port *)msg;
>
> Can be one line I suppose.
>
Yes, but I am trying to avoid line-breaking because of 80 chars
limitation.
> ...
>
> > + if (evt->id == PRESTERA_PORT_EVENT_STATE_CHANGED)
> > + evt->port_evt.data.oper_state = hw_evt->param.oper_state;
> > + else
> > + return -EINVAL;
> > +
> > + return 0;
>
> Perhaps traditional pattern, i.e.
>
> if (...)
> return -EINVAL;
> ...
> return 0;
>
I am not sure if it is applicable here, because the error state here
is if 'evt->id' did not matched after all checks. Actually this is
simply a 'switch', but I use 'if' to have shorter code.
> ...
>
> > + err = fw_event_parsers[msg->type].func(buf, &evt);
> > + if (!err)
> > + eh.func(sw, &evt, eh.arg);
>
> Ditto.
Makes sense.
>
> > + return err;
>
> ...
>
> > + memcpy(&req.param.mac, mac, sizeof(req.param.mac));
>
> Consider to use ether_addr_*() APIs instead of open-coded mem*() ones.
>
> ...
>
> > +#define PRESTERA_MTU_DEFAULT 1536
>
> Don't we have global default for this?
>
> ...
>
> > +#define PRESTERA_STATS_DELAY_MS msecs_to_jiffies(1000)
>
> It's not _MS.
>
> ...
>
> > + if (!is_up)
> > + netif_stop_queue(dev);
> > +
> > + err = prestera_hw_port_state_set(port, is_up);
> > +
> > + if (is_up && !err)
> > + netif_start_queue(dev);
>
> Much better if will look lke
>
> if (is_up) {
> ...
> err = ...(..., true);
> if (err)
> return err;
> ...
> } else {
> return prestera_*(..., false);
> }
> return 0;
>
> > + return err;
>
> ...
>
> > + /* Only 0xFF mac addrs are supported */
> > + if (port->fp_id >= 0xFF)
> > + goto err_port_init;
>
> You meant 255, right?
> Otherwise you have to mentioned is it byte limitation or what?
>
> ...
Yes, 255 is a limitation because of max byte value.
>
> > +static int prestera_switch_set_base_mac_addr(struct prestera_switch *sw)
> > +{
> > + struct device_node *base_mac_np;
> > + struct device_node *np;
>
> > + np = of_find_compatible_node(NULL, NULL, "marvell,prestera");
> > + if (np) {
> > + base_mac_np = of_parse_phandle(np, "base-mac-provider", 0);
> > + if (base_mac_np) {
> > + const char *base_mac;
> > +
> > + base_mac = of_get_mac_address(base_mac_np);
> > + of_node_put(base_mac_np);
> > + if (!IS_ERR(base_mac))
> > + ether_addr_copy(sw->base_mac, base_mac);
> > + }
> > + }
> > +
> > + if (!is_valid_ether_addr(sw->base_mac)) {
> > + eth_random_addr(sw->base_mac);
> > + dev_info(sw->dev->dev, "using random base mac address\n");
> > + }
>
> Isn't it device_get_mac_address() reimplementation?
>
device_get_mac_address() just tries to get mac via fwnode.
> > +
> > + return prestera_hw_switch_mac_set(sw, sw->base_mac);
> > +}
>
> ...
>
> > + err = prestera_switch_init(sw);
> > + if (err) {
> > + kfree(sw);
> > + return err;
> > + }
> > +
> > + return 0;
>
> if (err)
> kfree(...);
> return err;
>
> Also, check reference counting.
>
> ...
>
> > +#define PRESTERA_SDMA_RX_DESC_PKT_LEN(desc) \
>
> > + ((le32_to_cpu((desc)->word2) >> 16) & 0x3FFF)
>
> Why not GENMASK() ?
Yes, GENMASK is right way to go, thanks.
>
> ...
>
> > + if (dma + sizeof(struct prestera_sdma_desc) > sdma->dma_mask) {
> > + dev_err(dma_dev, "failed to alloc desc\n");
> > + dma_pool_free(sdma->desc_pool, desc, dma);
>
> Better first undo something *then* print a message.
>
> > + return -ENOMEM;
> > + }
>
> ...
>
> > +static void prestera_sdma_rx_desc_set_len(struct prestera_sdma_desc *desc,
> > + size_t val)
> > +{
> > + u32 word = le32_to_cpu(desc->word2);
> > +
> > + word = (word & ~GENMASK(15, 0)) | val;
>
> Shouldn't you do traditional pattern?
>
> word = (word & ~mask) | (val & mask);
Looks like this is safer form.
>
> > + desc->word2 = cpu_to_le32(word);
> > +}
>
> ...
>
> > + dma = dma_map_single(dev, skb->data, skb->len, DMA_FROM_DEVICE);
>
> > +
>
> Redundant blank line.
>
> > + if (dma_mapping_error(dev, dma))
> > + goto err_dma_map;
>
> ...
>
> > + pr_warn_ratelimited("received pkt for non-existent port(%u, %u)\n",
> > + dev_id, hw_port);
>
> netdev_warn_ratelimited() ? Or something closer to that?
>
> ...
>
> > + qmask = GENMASK(qnum - 1, 0);
>
> BIT(qnum) - 1 will produce much better code I suppose.
>
> ...
>
> > + if (pkts_done < budget && napi_complete_done(napi, pkts_done))
> > + prestera_write(sdma->sw, PRESTERA_SDMA_RX_INTR_MASK_REG,
> > + 0xff << 2);
>
> GENMASK() ?
>
> ...
>
> > + word = (word & ~GENMASK(30, 16)) | ((len + ETH_FCS_LEN) << 16);
>
> Consider traditional pattern.
>
> ...
>
> > + word |= PRESTERA_SDMA_TX_DESC_DMA_OWN << 31;
>
> I hope that was defined with U. Otherwise it's UB.
>
> ...
>
> > + new_skb = alloc_skb(len, GFP_ATOMIC | GFP_DMA);
>
> Atomic? Why?
>
TX path might be called from net_tx_action which is softirq.
> ...
>
> > +static int prestera_sdma_tx_wait(struct prestera_sdma *sdma,
> > + struct prestera_tx_ring *tx_ring)
> > +{
>
> > + int tx_retry_num = 10 * tx_ring->max_burst;
>
> Magic!
You mean the code is magic ? Yes, I am trying to relax the
calling of SDMA engine.
>
> > + while (--tx_retry_num) {
> > + if (prestera_sdma_is_ready(sdma))
> > + return 0;
> > +
> > + udelay(1);
> > + }
>
> unsigned int counter = ...;
>
> do { } while (--counter);
>
> looks better.
>
> Also, why udelay()? Is it atomic context?
TX path might be called from net_tx_action which is softirq.
>
> > + return -EBUSY;
> > +}
>
> ...
>
> > + if (!tx_ring->burst--) {
>
> Don't do like this. It makes code harder to understand.
>
> if (tx_ring->...) {
> ...->burst--;
> } else {
> ...
> }
I will try.
>
> > + tx_ring->burst = tx_ring->max_burst;
> > +
> > + err = prestera_sdma_tx_wait(sdma, tx_ring);
> > + if (err)
> > + goto drop_skb_unmap;
> > + }
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* Re: iproute2 DDMMYY versioning - why?
From: Stephen Hemminger @ 2020-07-31 15:21 UTC (permalink / raw)
To: Or Gerlitz; +Cc: Linux Netdev List
In-Reply-To: <CAJ3xEMisQFoFzghnoTC7joD5JxNi95o8T7gR8fW9OkxEbsuaQQ@mail.gmail.com>
On Fri, 31 Jul 2020 16:23:06 +0300
Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Tue, Jul 28, 2020 at 10:51 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
>
> > It is only an historical leftover, because 15 yrs ago that is how Alexy did it
>
> So how about putting behind the burden created by this historical leftover
> and moving to use the kernel releases as the emitted version?
The only downside to kernel versioning is the distros will think
"can't use v5.10 because our kernel is still v4.3" and then want
LTS versions of iproute2 which would create significant resource efforts.
Open to changing to kernel versions, it would be trivial to do.
The tagging and releases are by kernel version.
The SNAPSHOT.h is autogenerated by a script, that can be changed.
^ permalink raw reply
* Re: [net-next PATCH v7 1/6] Documentation: ACPI: DSD: Document MDIO PHY
From: Andrew Lunn @ 2020-07-31 15:14 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Florian Fainelli, Dan Callaghan, Jeremy Linton, Calvin Johnson,
Russell King, Jon, Cristi Sovaiala, Ioana Ciornei, Madalin Bucur,
netdev, linux.cj, linux-acpi
In-Reply-To: <CAHp75VejnW23LEfyEO6Py8=e3_W0YMomk8jQ3JQeHqYcaeDitg@mail.gmail.com>
> > > DT can be used on x86, and i suspect it is a much easier path of least
> > > resistance.
> >
> > And you can easily overlay Device Tree to an existing system by using
> > either a full Device Tree overlay (dtbo) or using CONFIG_OF_DYNAMIC and
> > creating nodes on the fly.
>
> Why do you need DT on a system that runs without it and Linux has all
> means to extend to cover a lot of stuff DT provides for other types of
> firmware nodes?
As i said, path of least resistance. It is here today, heavily used,
well understood by lots of network developers, has a very active
maintainer in the form of Rob Herring, and avoids 'showflakes' as
Florian likes to call it, so we are all sharing the same code,
providing a lot of testing and maintenance.
Andrew
^ permalink raw reply
* RE: [PATCH v2 net-next] mscc: Add LCPLL Reset to VSC8574 Family of phy drivers
From: Bryan.Whitehead @ 2020-07-31 15:09 UTC (permalink / raw)
To: f.fainelli, davem; +Cc: netdev, UNGLinuxDriver, andrew, hkallweit1, linux
In-Reply-To: <94cacb22-c31d-4cd3-3872-2e431bafb0da@gmail.com>
Thanks David and Florian, see below.
> On 7/30/20 4:36 PM, David Miller wrote:
> > From: Bryan Whitehead <Bryan.Whitehead@microchip.com>
> > Date: Mon, 27 Jul 2020 13:18:28 -0400
> >
> >> @@ -929,6 +929,77 @@ static bool vsc8574_is_serdes_init(struct
> >> phy_device *phydev) }
> >>
> >> /* bus->mdio_lock should be locked when using this function */
> >> +/* Page should already be set to MSCC_PHY_PAGE_EXTENDED_GPIO */
> >> +static int vsc8574_micro_command(struct phy_device *phydev, u16
> >> +command)
> > ...
> >> +/* bus->mdio_lock should be locked when using this function */
> >
> > Please don't dup this comment, it's not appropriate.
>
> Agree put a mutex assertion instead if you want to catch offenders at run time?
> --
> Florian
I was simply following the pattern that already exists in the driver.
Would you like me to remove the same comment from the rest of the functions in the driver?
The lock is already checked in the existing low level functions, phy_base_read, and phy_base_write.
The check is of the following form
if (unlikely(!mutex_is_locked(&phydev->mdio.bus->mdio_lock))) {
dev_err(&phydev->mdio.dev, "MDIO bus lock not held!\n");
dump_stack();
}
Is this a reasonable mutex assertion, or is there an existing preferred helper macro that can be used instead?
Bryan
^ permalink raw reply
* Re: [net-next PATCH v7 1/6] Documentation: ACPI: DSD: Document MDIO PHY
From: Andrew Lunn @ 2020-07-31 15:08 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Sudeep Holla, Jeremy Linton, Calvin Johnson,
Russell King - ARM Linux admin, Jon, Cristi Sovaiala,
Ioana Ciornei, Andy Shevchenko, Florian Fainelli, Madalin Bucur,
netdev, linux.cj, ACPI Devel Maling List, Vikas Singh
In-Reply-To: <CAJZ5v0i+a+MS+J_auuuMmq25c1HNb7oV2sqQ87WOtfBBQ6MF7w@mail.gmail.com>
> However, if those properties are never going to be supplied via ACPI
> on any production systems, the code added in order to be able to
> process them will turn out to be useless and I don't think anyone
> wants useless code in the kernel.
>
> So the real question is whether or not there will be production
> systems in which those properties will be supplied via ACPI and I
> cannot answer that question.
Hi Rafael
I suspect we are going to have a lot of newbie ACPI questions over the
next few weeks/months as vendors and the PHY maintainers get up to
speed on all this.
In the device tree world, we would expect a patch as part of the
patchset to a device tree file somewhere under arch/*/boot/dts to make
use of any new property added. We then know it is used.
How does this work in the ACPI world? How does somebody show they are
supplying the property in a production system?
> Basically, the interested vendors need to agree on how exactly they
> want ACPI to be used and come up with a specification setting the
> rules to be followed by the platform firmware on the one side and by
> the code in the kernel on the other side.
...
> Besides, you really should be asking for a spec the work is based on,
> IMO, instead of asking for an ACPI maintainer ACK which is not going
> to be sufficient if the former is missing anyway.
Could you point us towards real world example specs? Giving us a good
best practice example would likely help us to do this work. And reduce
the amount of work you need to do keeping the process going in the
correct direction.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: fix compilation warning of selftests
From: Daniel Borkmann @ 2020-07-31 15:00 UTC (permalink / raw)
To: Jianlin Lv, bpf; +Cc: davem, kuba, ast, yhs, Song.Zhu, linux-kernel, netdev
In-Reply-To: <20200731061600.18344-1-Jianlin.Lv@arm.com>
On 7/31/20 8:16 AM, Jianlin Lv wrote:
> Clang compiler version: 12.0.0
> The following warning appears during the selftests/bpf compilation:
>
> prog_tests/send_signal.c:51:3: warning: ignoring return value of ‘write’,
> declared with attribute warn_unused_result [-Wunused-result]
> 51 | write(pipe_c2p[1], buf, 1);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~
> prog_tests/send_signal.c:54:3: warning: ignoring return value of ‘read’,
> declared with attribute warn_unused_result [-Wunused-result]
> 54 | read(pipe_p2c[0], buf, 1);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> ......
>
> prog_tests/stacktrace_build_id_nmi.c:13:2: warning: ignoring return value
> of ‘fscanf’,declared with attribute warn_unused_result [-Wunused-resul]
> 13 | fscanf(f, "%llu", &sample_freq);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> test_tcpnotify_user.c:133:2: warning:ignoring return value of ‘system’,
> declared with attribute warn_unused_result [-Wunused-result]
> 133 | system(test_script);
> | ^~~~~~~~~~~~~~~~~~~
> test_tcpnotify_user.c:138:2: warning:ignoring return value of ‘system’,
> declared with attribute warn_unused_result [-Wunused-result]
> 138 | system(test_script);
> | ^~~~~~~~~~~~~~~~~~~
> test_tcpnotify_user.c:143:2: warning:ignoring return value of ‘system’,
> declared with attribute warn_unused_result [-Wunused-result]
> 143 | system(test_script);
> | ^~~~~~~~~~~~~~~~~~~
>
> Add code that fix compilation warning about ignoring return value and
> handles any errors; Check return value of library`s API make the code
> more secure.
>
> Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
Looks good overall, there is one small bug that slipped in, see below:
[...]
> diff --git a/tools/testing/selftests/bpf/test_tcpnotify_user.c b/tools/testing/selftests/bpf/test_tcpnotify_user.c
> index f9765ddf0761..869e28c92d73 100644
> --- a/tools/testing/selftests/bpf/test_tcpnotify_user.c
> +++ b/tools/testing/selftests/bpf/test_tcpnotify_user.c
> @@ -130,17 +130,26 @@ int main(int argc, char **argv)
> sprintf(test_script,
> "iptables -A INPUT -p tcp --dport %d -j DROP",
> TESTPORT);
> - system(test_script);
> + if (system(test_script)) {
> + printf("FAILED: execute command: %s\n", test_script);
> + goto err;
> + }
>
> sprintf(test_script,
> "nc 127.0.0.1 %d < /etc/passwd > /dev/null 2>&1 ",
> TESTPORT);
> - system(test_script);
> + if (system(test_script)) {
> + printf("FAILED: execute command: %s\n", test_script);
> + goto err;
> + }
Did you try to run this test case? With the patch here it will fail:
# ./test_tcpnotify_user
FAILED: execute command: nc 127.0.0.1 12877 < /etc/passwd > /dev/null 2>&1
This is because nc returns 1 as exit code and for the test it is actually expected
to fail given the iptables rule we installed for TESTPORT right above and remove
again below.
Please adapt this and send a v2, thanks!
> sprintf(test_script,
> "iptables -D INPUT -p tcp --dport %d -j DROP",
> TESTPORT);
> - system(test_script);
> + if (system(test_script)) {
> + printf("FAILED: execute command: %s\n", test_script);
> + goto err;
> + }
>
> rv = bpf_map_lookup_elem(bpf_map__fd(global_map), &key, &g);
> if (rv != 0) {
>
^ permalink raw reply
* Re: [PATCH RFC net-next] net: phy: add Marvell PHY PTP support [multicast/DSA issues]
From: Andrew Lunn @ 2020-07-31 14:41 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Richard Cochran, Florian Fainelli, Heiner Kallweit,
David S. Miller, Jakub Kicinski, netdev
In-Reply-To: <20200730115419.GX1605@shell.armlinux.org.uk>
> Hmm, and even with IP_MULTICAST=y, I still can't get the "timestamping"
> program from the kernel sources to work.
>
> On two different machines, I'm running:
>
> # ./timestamping32 eno0 SOF_TIMESTAMPING_RX_HARDWARE SOF_TIMESTAMPING_RAW_HARDWARE
>
> On one machine (arm32) this works - it can see the traffic it generates
> and receives from the other machine.
>
> On the other machine (arm64), it sees _no_ traffic at all, but tcpdump
> on that machine can see the traffic being received:
>
> 12:36:40.002065 00:51:82:11:33:03 > 01:00:5e:00:01:82, ethertype IPv4 (0x0800),
> length 166: (tos 0x0, ttl 1, id 15045, offset 0, flags [DF], proto UDP (17), length 152)
> 192.168.3.1.319 > 224.0.1.130.319: [bad udp cksum 0xa5c1 -> 0x7aaa!] UDP, length 124
> 12:36:41.105391 00:50:43:02:03:02 > 01:00:5e:00:01:82, ethertype IPv4 (0x0800),
> length 166: (tos 0x0, ttl 1, id 9715, offset 0, flags [DF], proto UDP (17), length 152)
> 192.168.3.2.319 > 224.0.1.130.319: [udp sum ok] UDP, length 124
>
> The bad udp cksum is due to checksum offloadong on transmit - 3.1 is the
> arm64 host running that tcpdump.
>
> When I look at /proc/net/snmp, I can see the IP InReceives incrementing
> but not the IP InDelivers nor UDP InDatagrams counter:
>
> Ip: 2 64 1602 0 3 0 0 0 297 531 0 46 0 0 0 0 0 0 0
> Udp: 572 0 0 480 0 0 0 4
> Ip: 2 64 1603 0 3 0 0 0 297 531 0 46 0 0 0 0 0 0 0
> Udp: 572 0 0 480 0 0 0 4
>
> I don't have any firewall rules on the machine.
Hi Russell
A shot in the dark....
I remember somewhere in the stack BPF is used to identify PTP
packets. Is it maybe getting JITed wrongly on arm64? Maybe disable the
BPF JIT and see if it starts working?
Andrew
^ permalink raw reply
* Re: [Linux-kernel-mentees] [PATCH net] rds: Prevent kernel-infoleak in rds_notify_queue_get()
From: Jason Gunthorpe @ 2020-07-31 14:36 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Leon Romanovsky, Peilin Ye, Santosh Shilimkar, David S. Miller,
Jakub Kicinski, Dan Carpenter, Arnd Bergmann,
linux-kernel-mentees, netdev, linux-rdma, rds-devel, linux-kernel
In-Reply-To: <20200731142148.GA1718799@kroah.com>
On Fri, Jul 31, 2020 at 04:21:48PM +0200, Greg Kroah-Hartman wrote:
> > The spec was updated in C11 to require zero'ing padding when doing
> > partial initialization of aggregates (eg = {})
> >
> > """if it is an aggregate, every member is initialized (recursively)
> > according to these rules, and any padding is initialized to zero
> > bits;"""
>
> But then why does the compilers not do this?
Do you have an example?
> > Considering we have thousands of aggregate initializers it
> > seems likely to me Linux also requires a compiler with this C11
> > behavior to operate correctly.
>
> Note that this is not an "operate correctly" thing, it is a "zero out
> stale data in structure paddings so that data will not leak to
> userspace" thing.
Yes, not being insecure is "operate correctly", IMHO :)
> > Does this patch actually fix anything? My compiler generates identical
> > assembly code in either case.
>
> What compiler version?
I tried clang 10 and gcc 9.3 for x86-64.
#include <string.h>
void test(void *out)
{
struct rds_rdma_notify {
unsigned long user_token;
unsigned int status;
} foo = {};
memcpy(out, &foo, sizeof(foo));
}
$ gcc -mno-sse2 -O2 -Wall -std=c99 t.c -S
test:
endbr64
movq $0, (%rdi)
movq $0, 8(%rdi)
ret
Just did this same test with gcc 4.4 and it also gave the same output..
Made it more complex with this:
struct rds_rdma_notify {
unsigned long user_token;
unsigned char status;
unsigned long user_token1;
unsigned char status1;
unsigned long user_token2;
unsigned char status2;
unsigned long user_token3;
unsigned char status3;
unsigned long user_token4;
unsigned char status4;
} foo;
And still got the same assembly vs memset on gcc 4.4.
I tried for a bit and didn't find a way to get even old gcc 4.4 to not
initialize the holes.
Jason
^ permalink raw reply
* pull request: bluetooth-next 2020-07-31
From: Johan Hedberg @ 2020-07-31 14:35 UTC (permalink / raw)
To: davem; +Cc: linux-bluetooth, netdev
[-- Attachment #1: Type: text/plain, Size: 10259 bytes --]
Hi Dave,
Here's the main bluetooth-next pull request for 5.9:
- Fix firmware filenames for Marvell chipsets
- Several suspend-related fixes
- Addedd mgmt commands for runtime configuration
- Multiple fixes for Qualcomm-based controllers
- Add new monitoring feature for mgmt
- Fix handling of legacy cipher (E4) together with security level 4
- Add support for Realtek 8822CE controller
- Fix issues with Chinese controllers using fake VID/PID values
- Multiple other smaller fixes & improvements
Please let me know if there are any issues pulling. Thanks.
Johan
---
The following changes since commit 065fcfd49763ec71ae345bb5c5a74f961031e70e:
selftests: net: ip_defrag: ignore EPERM (2020-06-02 15:54:20 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git for-upstream
for you to fetch changes up to 075f77324f90149bac12c8a705dae5786a1d24fb:
Bluetooth: Remove CRYPTO_ALG_INTERNAL flag (2020-07-31 16:42:04 +0300)
----------------------------------------------------------------
Abhishek Pandit-Subedi (15):
Bluetooth: Allow suspend even when preparation has failed
Bluetooth: btmrvl_sdio: Set parent dev to hdev
Bluetooth: btmrvl_sdio: Implement prevent_wake
Bluetooth: btmrvl_sdio: Refactor irq wakeup
Bluetooth: Add bdaddr_list_with_flags for classic whitelist
Bluetooth: Replace wakeable list with flag
Bluetooth: Replace wakeable in hci_conn_params
Bluetooth: Add get/set device flags mgmt op
Bluetooth: Add hci_dev_lock to get/set device flags
Bluetooth: btusb: Reset port on cmd timeout
Bluetooth: btusb: BTUSB_WAKEUP_DISABLE prevents wake
Bluetooth: Don't restart scanning if paused
Bluetooth: btusb: Comment on unbalanced pm reference
Bluetooth: Fix suspend notifier race
Revert "Bluetooth: btusb: Disable runtime suspend on Realtek devices"
Alain Michaud (11):
Bluetooth: Removing noisy dbg message
Bluetooth: Add support for BT_PKT_STATUS CMSG data for SCO connections
Bluetooth: Use only 8 bits for the HCI CMSG state flags
Bluetooth: mgmt: read/set system parameter definitions
Bluetooth: centralize default value initialization.
Bluetooth: implement read/set default system parameters mgmt
Bluetooth: use configured params for ext adv
Bluetooth: Adding a configurable autoconnect timeout
Bluetooth: use configured default params for active scans
Bluetooth: le_simult_central_peripheral experimental feature
Bluetooth: use the proper scan params when conn is pending
Alexander A. Klimov (1):
Replace HTTP links with HTTPS ones: BLUETOOTH SUBSYSTEM
Balakrishna Godavarthi (3):
Bluetooth: hci_qca: Disable SoC debug logging for WCN3991
Bluetooth: hci_qca: Increase SoC idle timeout to 200ms
Bluetooth: hci_qca: Request Tx clock vote off only when Tx is pending
Chethan T N (2):
Bluetooth: btusb: Add support to read Intel debug feature
Bluetooth: btusb: Configure Intel debug feature based on available support
Dan Carpenter (1):
Bluetooth: hci_qca: Fix an error pointer dereference
Daniel Winkler (1):
Bluetooth: Add per-instance adv disable/remove
Gustavo A. R. Silva (3):
Bluetooth: core: Use fallthrough pseudo-keyword
Bluetooth: RFCOMM: Use fallthrough pseudo-keyword
Bluetooth: Use fallthrough pseudo-keyword
Herbert Xu (1):
Bluetooth: Remove CRYPTO_ALG_INTERNAL flag
Hilda Wu (1):
Bluetooth: btusb: USB alternate setting 1 for WBS
Ismael Ferreras Morezuelas (1):
Bluetooth: btusb: Fix and detect most of the Chinese Bluetooth controllers
Joseph Hwang (1):
Bluetooth: btusb: add Realtek 8822CE to usb_device_id table
Kiran K (1):
Bluetooth: btusb: Refactor of firmware download flow for Intel conrollers
Lihong Kou (1):
Bluetooth: add a mutex lock to avoid UAF in do_enale_set
Luiz Augusto von Dentz (1):
Bluetooth: Disconnect if E0 is used for Level 4
Manish Mandlik (2):
Bluetooth: Check scan state before disabling during suspend
Bluetooth: Terminate the link if pairing is cancelled
Marcel Holtmann (6):
Bluetooth: mgmt: Add commands for runtime configuration
Bluetooth: mgmt: Use command complete on success for set system config
Bluetooth: Translate additional address type correctly
Bluetooth: Configure controller address resolution if available
Bluetooth: Update resolving list when updating whitelist
Bluetooth: Increment management interface revision
Martin Blumenstingl (1):
dt-bindings: net: bluetooth: realtek: Fix uart-has-rtscts example
Matthias Kaehlcke (4):
Bluetooth: hci_qca: Simplify determination of serial clock on/off state from votes
Bluetooth: hci_qca: Only remove TX clock vote after TX is completed
Bluetooth: hci_qca: Skip serdev wait when no transfer is pending
Bluetooth: hci_qca: Refactor error handling in qca_suspend()
Max Chou (1):
Bluetooth: Return NOTIFY_DONE for hci_suspend_notifier
Miao-chen Chou (9):
Bluetooth: Add definitions for advertisement monitor features
Bluetooth: Add handler of MGMT_OP_READ_ADV_MONITOR_FEATURES
Bluetooth: Add handler of MGMT_OP_ADD_ADV_PATTERNS_MONITOR
Bluetooth: Add handler of MGMT_OP_REMOVE_ADV_MONITOR
Bluetooth: Notify adv monitor added event
Bluetooth: Notify adv monitor removed event
Bluetooth: Update background scan and report device based on advertisement monitors
Bluetooth: Fix kernel oops triggered by hci_adv_monitors_clear()
Bluetooth: Use whitelist for scan policy when suspending
Nicolas Boichat (2):
Bluetooth: hci_h5: Set HCI_UART_RESET_ON_INIT to correct flags
Bluetooth: hci_serdev: Only unregister device if it was registered
Pali Rohár (4):
mwifiex: Fix firmware filename for sd8977 chipset
mwifiex: Fix firmware filename for sd8997 chipset
btmrvl: Fix firmware filename for sd8977 chipset
btmrvl: Fix firmware filename for sd8997 chipset
Patrick Steinhardt (1):
Bluetooth: Fix update of connection state in `hci_encrypt_cfm`
Peilin Ye (3):
Bluetooth: Fix slab-out-of-bounds read in hci_extended_inquiry_result_evt()
Bluetooth: Prevent out-of-bounds read in hci_inquiry_result_evt()
Bluetooth: Prevent out-of-bounds read in hci_inquiry_result_with_rssi_evt()
Sathish Narasimman (5):
Bluetooth: Translate additional address type during le_conn
Bluetooth: Let controller creates RPA during le create conn
Bluetooth: Enable/Disable address resolution during le create conn
Bluetooth: Enable RPA Timeout
Bluetooth: Enable controller RPA resolution using Experimental feature
Sean Wang (2):
Bluetooth: btusb: fix up firmware download sequence
Bluetooth: btmtksdio: fix up firmware download sequence
Venkata Lakshmi Narayana Gubba (3):
Bluetooth: hci_qca: Bug fix during SSR timeout
Bluetooth: hci_qca: Bug fixes for SSR
Bluetooth: hci_qca: Stop collecting memdump again for command timeout during SSR
.../devicetree/bindings/net/realtek-bluetooth.yaml | 2 +-
drivers/bluetooth/bcm203x.c | 2 +-
drivers/bluetooth/bluecard_cs.c | 2 -
drivers/bluetooth/btintel.c | 59 +++
drivers/bluetooth/btintel.h | 21 +
drivers/bluetooth/btmrvl_main.c | 11 +
drivers/bluetooth/btmrvl_sdio.c | 21 +-
drivers/bluetooth/btmtksdio.c | 16 +-
drivers/bluetooth/btqca.c | 27 +
drivers/bluetooth/btqca.h | 2 +
drivers/bluetooth/btusb.c | 303 +++++++----
drivers/bluetooth/hci_h5.c | 2 +-
drivers/bluetooth/hci_ll.c | 2 +-
drivers/bluetooth/hci_qca.c | 134 +++--
drivers/bluetooth/hci_serdev.c | 3 +-
drivers/net/wireless/marvell/mwifiex/sdio.h | 4 +-
include/net/bluetooth/bluetooth.h | 12 +
include/net/bluetooth/hci.h | 28 +-
include/net/bluetooth/hci_core.h | 107 +++-
include/net/bluetooth/hci_sock.h | 4 +-
include/net/bluetooth/mgmt.h | 95 ++++
include/net/bluetooth/sco.h | 2 +
net/bluetooth/6lowpan.c | 5 +
net/bluetooth/Kconfig | 2 +-
net/bluetooth/Makefile | 2 +-
net/bluetooth/af_bluetooth.c | 5 +-
net/bluetooth/hci_conn.c | 51 +-
net/bluetooth/hci_core.c | 212 +++++++-
net/bluetooth/hci_event.c | 71 ++-
net/bluetooth/hci_request.c | 286 ++++++++--
net/bluetooth/hci_request.h | 5 +-
net/bluetooth/hci_sock.c | 7 +-
net/bluetooth/l2cap_core.c | 25 +-
net/bluetooth/l2cap_sock.c | 4 +-
net/bluetooth/mgmt.c | 577 ++++++++++++++++++++-
net/bluetooth/mgmt_config.c | 283 ++++++++++
net/bluetooth/mgmt_config.h | 17 +
net/bluetooth/msft.c | 7 +
net/bluetooth/msft.h | 9 +
net/bluetooth/rfcomm/core.c | 2 +-
net/bluetooth/rfcomm/sock.c | 2 +-
net/bluetooth/sco.c | 32 ++
net/bluetooth/selftest.c | 2 +-
net/bluetooth/smp.c | 8 +-
44 files changed, 2149 insertions(+), 324 deletions(-)
create mode 100644 net/bluetooth/mgmt_config.c
create mode 100644 net/bluetooth/mgmt_config.h
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 01/11] xen/manage: keep track of the on-going suspend mode
From: Rafael J. Wysocki @ 2020-07-31 14:25 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: Anchal Agarwal, Stefano Stabellini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, the arch/x86 maintainers,
Juergen Gross, Linux PM, Linux Memory Management List,
Kamata, Munehisa, Konrad Rzeszutek Wilk, roger.pau, Jens Axboe,
David Miller, Rafael J. Wysocki, Len Brown, Pavel Machek,
Peter Zijlstra, Eduardo Valentin, Singh, Balbir, xen-devel,
Vitaly Kuznetsov, netdev, Linux Kernel Mailing List,
David Woodhouse, Benjamin Herrenschmidt
In-Reply-To: <53b577a3-6af9-5587-7e47-485be38b3653@oracle.com>
On Fri, Jul 31, 2020 at 4:14 PM Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
>
> On 7/30/20 7:06 PM, Anchal Agarwal wrote:
> > On Mon, Jul 27, 2020 at 06:08:29PM -0400, Boris Ostrovsky wrote:
> >> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> >>
> >>
> >>
> >> On 7/24/20 7:01 PM, Stefano Stabellini wrote:
> >>> Yes, it does, thank you. I'd rather not introduce unknown regressions so
> >>> I would recommend to add an arch-specific check on registering
> >>> freeze/thaw/restore handlers. Maybe something like the following:
> >>>
> >>> #ifdef CONFIG_X86
> >>> .freeze = blkfront_freeze,
> >>> .thaw = blkfront_restore,
> >>> .restore = blkfront_restore
> >>> #endif
> >>>
> >>>
> >>> maybe Boris has a better suggestion on how to do it
> >>
> >> An alternative might be to still install pm notifier in
> >> drivers/xen/manage.c (I think as result of latest discussions we decided
> >> we won't need it) and return -ENOTSUPP for ARM for
> >> PM_HIBERNATION_PREPARE and friends. Would that work?
> >>
> > I think the question here is for registering driver specific freeze/thaw/restore
> > callbacks for x86 only. I have dropped the pm_notifier in the v3 still pending
> > testing. So I think just registering driver specific callbacks for x86 only is a
> > good option. What do you think?
>
>
> I suggested using the notifier under assumption that if it returns an
> error then that will prevent callbacks to be called because hibernation
> will be effectively disabled.
That's correct.
^ permalink raw reply
* Re: [Linux-kernel-mentees] [PATCH net] rds: Prevent kernel-infoleak in rds_notify_queue_get()
From: Greg Kroah-Hartman @ 2020-07-31 14:21 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Leon Romanovsky, Peilin Ye, Santosh Shilimkar, David S. Miller,
Jakub Kicinski, Dan Carpenter, Arnd Bergmann,
linux-kernel-mentees, netdev, linux-rdma, rds-devel, linux-kernel
In-Reply-To: <20200731140452.GE24045@ziepe.ca>
On Fri, Jul 31, 2020 at 11:04:52AM -0300, Jason Gunthorpe wrote:
> On Fri, Jul 31, 2020 at 07:33:33AM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Jul 31, 2020 at 07:33:06AM +0200, Greg Kroah-Hartman wrote:
> > > On Fri, Jul 31, 2020 at 07:53:01AM +0300, Leon Romanovsky wrote:
> > > > On Thu, Jul 30, 2020 at 03:20:26PM -0400, Peilin Ye wrote:
> > > > > rds_notify_queue_get() is potentially copying uninitialized kernel stack
> > > > > memory to userspace since the compiler may leave a 4-byte hole at the end
> > > > > of `cmsg`.
> > > > >
> > > > > In 2016 we tried to fix this issue by doing `= { 0 };` on `cmsg`, which
> > > > > unfortunately does not always initialize that 4-byte hole. Fix it by using
> > > > > memset() instead.
> > > >
> > > > Of course, this is the difference between "{ 0 }" and "{}" initializations.
> > >
> > > Really? Neither will handle structures with holes in it, try it and
> > > see.
> >
> > And if true, where in the C spec does it say that?
>
> The spec was updated in C11 to require zero'ing padding when doing
> partial initialization of aggregates (eg = {})
>
> """if it is an aggregate, every member is initialized (recursively)
> according to these rules, and any padding is initialized to zero
> bits;"""
But then why does the compilers not do this?
> The difference between {0} and the {} extension is only that {}
> reliably triggers partial initialization for all kinds of aggregates,
> while {0} has a number of edge cases where it can fail to compile.
>
> IIRC gcc has cleared the padding during aggregate initialization for a
> long time.
Huh? Last we checked a few months ago, no, it did not do that.
> Considering we have thousands of aggregate initializers it
> seems likely to me Linux also requires a compiler with this C11
> behavior to operate correctly.
Note that this is not an "operate correctly" thing, it is a "zero out
stale data in structure paddings so that data will not leak to
userspace" thing.
> Does this patch actually fix anything? My compiler generates identical
> assembly code in either case.
What compiler version?
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC PATCH bpf-next 3/3] samples/bpf: Add a simple bridge example accelerated with XDP
From: Jesper Dangaard Brouer @ 2020-07-31 14:15 UTC (permalink / raw)
To: Yoshiki Komachi
Cc: brouer, David S. Miller, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Jakub Kicinski,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
KP Singh, Roopa Prabhu, Nikolay Aleksandrov, David Ahern, netdev,
bridge, bpf
In-Reply-To: <1596170660-5582-4-git-send-email-komachi.yoshiki@gmail.com>
I really appreciate that you are working on adding this helper.
Some comments below.
On Fri, 31 Jul 2020 13:44:20 +0900
Yoshiki Komachi <komachi.yoshiki@gmail.com> wrote:
> diff --git a/samples/bpf/xdp_bridge_kern.c b/samples/bpf/xdp_bridge_kern.c
> new file mode 100644
> index 000000000000..00f802503199
> --- /dev/null
> +++ b/samples/bpf/xdp_bridge_kern.c
> @@ -0,0 +1,129 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2020 NTT Corp. All Rights Reserved.
> + *
[...]
> +
> +struct {
> + __uint(type, BPF_MAP_TYPE_DEVMAP_HASH);
> + __uint(key_size, sizeof(int));
> + __uint(value_size, sizeof(int));
> + __uint(max_entries, 64);
> +} xdp_tx_ports SEC(".maps");
> +
> +static __always_inline int xdp_bridge_proto(struct xdp_md *ctx, u16 br_vlan_proto)
> +{
> + void *data_end = (void *)(long)ctx->data_end;
> + void *data = (void *)(long)ctx->data;
> + struct bpf_fdb_lookup fdb_lookup_params;
> + struct vlan_hdr *vlan_hdr = NULL;
> + struct ethhdr *eth = data;
> + u16 h_proto;
> + u64 nh_off;
> + int rc;
> +
> + nh_off = sizeof(*eth);
> + if (data + nh_off > data_end)
> + return XDP_DROP;
> +
> + __builtin_memset(&fdb_lookup_params, 0, sizeof(fdb_lookup_params));
> +
> + h_proto = eth->h_proto;
> +
> + if (unlikely(ntohs(h_proto) < ETH_P_802_3_MIN))
> + return XDP_PASS;
> +
> + /* Handle VLAN tagged packet */
> + if (h_proto == br_vlan_proto) {
> + vlan_hdr = (void *)eth + nh_off;
> + nh_off += sizeof(*vlan_hdr);
> + if ((void *)eth + nh_off > data_end)
> + return XDP_PASS;
> +
> + fdb_lookup_params.vlan_id = ntohs(vlan_hdr->h_vlan_TCI) &
> + VLAN_VID_MASK;
> + }
> +
> + /* FIXME: Although Linux bridge provides us with vlan filtering (contains
> + * PVID) at ingress, the feature is currently unsupported in this XDP program.
> + *
> + * Two ideas to realize the vlan filtering are below:
> + * 1. usespace daemon monitors bridge vlan events and notifies XDP programs
^^
Typo: usespace -> userspace
> + * of them through BPF maps
> + * 2. introduce another bpf helper to retrieve bridge vlan information
The comment appears two times time this file.
> + *
> + *
> + * FIXME: After the vlan filtering, learning feature is required here, but
> + * it is currently unsupported as well. If another bpf helper for learning
> + * is accepted, the processing could be implemented in the future.
> + */
> +
> + memcpy(&fdb_lookup_params.addr, eth->h_dest, ETH_ALEN);
> +
> + /* Note: This program definitely takes ifindex of ingress interface as
> + * a bridge port. Linux networking devices can be stacked and physical
> + * interfaces are not necessarily slaves of bridges (e.g., bonding or
> + * vlan devices can be slaves of bridges), but stacked bridge ports are
> + * currently unsupported in this program. In such cases, XDP programs
> + * should be attached to a lower device in order to process packets with
> + * higher speed. Then, a new bpf helper to find upper devices will be
> + * required here in the future because they will be registered on FDB
> + * in the kernel.
> + */
> + fdb_lookup_params.ifindex = ctx->ingress_ifindex;
> +
> + rc = bpf_fdb_lookup(ctx, &fdb_lookup_params, sizeof(fdb_lookup_params), 0);
> + if (rc != BPF_FDB_LKUP_RET_SUCCESS) {
> + /* In cases of flooding, XDP_PASS will be returned here */
> + return XDP_PASS;
> + }
> +
> + /* FIXME: Although Linux bridge provides us with vlan filtering (contains
> + * untagged policy) at egress as well, the feature is currently unsupported
> + * in this XDP program.
> + *
> + * Two ideas to realize the vlan filtering are below:
> + * 1. usespace daemon monitors bridge vlan events and notifies XDP programs
> + * of them through BPF maps
> + * 2. introduce another bpf helper to retrieve bridge vlan information
> + */
(2nd time the comment appears)
> +
A comment about below bpf_redirect_map() would be good. Explaining
that we depend on fallback behavior, to let normal bridge code handle
other cases (e.g. flood/broadcast). And also that if lookup fails,
XDP_PASS/fallback also happens.
> + return bpf_redirect_map(&xdp_tx_ports, fdb_lookup_params.ifindex, XDP_PASS);
> +}
> +
> +SEC("xdp_bridge")
> +int xdp_bridge_prog(struct xdp_md *ctx)
> +{
> + return xdp_bridge_proto(ctx, 0);
> +}
> +
> +SEC("xdp_8021q_bridge")
> +int xdp_8021q_bridge_prog(struct xdp_md *ctx)
> +{
> + return xdp_bridge_proto(ctx, htons(ETH_P_8021Q));
> +}
> +
> +SEC("xdp_8021ad_bridge")
> +int xdp_8021ad_bridge_prog(struct xdp_md *ctx)
> +{
> + return xdp_bridge_proto(ctx, htons(ETH_P_8021AD));
> +}
> +
> +char _license[] SEC("license") = "GPL";
--
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 v2 01/11] xen/manage: keep track of the on-going suspend mode
From: Boris Ostrovsky @ 2020-07-31 14:13 UTC (permalink / raw)
To: Anchal Agarwal
Cc: Stefano Stabellini, tglx, mingo, bp, hpa, x86, jgross, linux-pm,
linux-mm, kamatam, konrad.wilk, roger.pau, axboe, davem, rjw,
len.brown, pavel, peterz, eduval, sblbir, xen-devel, vkuznets,
netdev, linux-kernel, dwmw, benh
In-Reply-To: <20200730230634.GA17221@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com>
On 7/30/20 7:06 PM, Anchal Agarwal wrote:
> On Mon, Jul 27, 2020 at 06:08:29PM -0400, Boris Ostrovsky wrote:
>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>>
>>
>>
>> On 7/24/20 7:01 PM, Stefano Stabellini wrote:
>>> Yes, it does, thank you. I'd rather not introduce unknown regressions so
>>> I would recommend to add an arch-specific check on registering
>>> freeze/thaw/restore handlers. Maybe something like the following:
>>>
>>> #ifdef CONFIG_X86
>>> .freeze = blkfront_freeze,
>>> .thaw = blkfront_restore,
>>> .restore = blkfront_restore
>>> #endif
>>>
>>>
>>> maybe Boris has a better suggestion on how to do it
>>
>> An alternative might be to still install pm notifier in
>> drivers/xen/manage.c (I think as result of latest discussions we decided
>> we won't need it) and return -ENOTSUPP for ARM for
>> PM_HIBERNATION_PREPARE and friends. Would that work?
>>
> I think the question here is for registering driver specific freeze/thaw/restore
> callbacks for x86 only. I have dropped the pm_notifier in the v3 still pending
> testing. So I think just registering driver specific callbacks for x86 only is a
> good option. What do you think?
I suggested using the notifier under assumption that if it returns an
error then that will prevent callbacks to be called because hibernation
will be effectively disabled. But I haven't looked at PM code so I don't
know whether this is actually the case.
The advantage of doing it in the notifier is that instead of adding
ifdefs to each driver you will be able to prevent callbacks from a
single place. Plus you can use this do disable hibernation for PVH dom0
as well.
-boris
^ permalink raw reply
* Re: [PATCH v2] drivers/net/wan/lapbether: Use needed_headroom instead of hard_header_len
From: Willem de Bruijn @ 2020-07-31 14:12 UTC (permalink / raw)
To: Xie He
Cc: David S. Miller, Jakub Kicinski, Linux Kernel Network Developers,
LKML, Linux X25, briannorris
In-Reply-To: <CAJht_ENjHRExBEHx--xmqnOy1MXY_6F5XZ_exinSfa6xU_XDJg@mail.gmail.com>
On Thu, Jul 30, 2020 at 9:36 PM Xie He <xie.he.0141@gmail.com> wrote:
>
> I'm really sorry to have re-sent the patch when the patch is still in
> review. I don't intend to be disrespectful to anyone. And I apologize
> for any disrespectfulness this might appear. Sorry.
>
> I'm also sorry for not having sent the patch with the proper subject
> prefixed with "net" or "net-next". If anyone requests I can re-send
> this patch with the proper subject "PATCH net".
>
> This patch actually fixes a kernel panic when this driver is used with
> a AF_PACKET/RAW socket. This driver runs on top of Ethernet
> interfaces. So I created a pair of virtual Ethernet (veth) interfaces,
> loaded this driver to create a pair of X.25 interfaces on top of them,
> and wrote C programs to use AF_PACKET sockets to send/receive data
> through them.
>
> At first I used AF_PACKET/DGRAM sockets. I prepared packet data
> according to the requirements of X.25 drivers. I first sent an
> one-byte packet ("\x01") to instruct the driver to connect, then I
> sent data prefixed with an one-byte pseudo header ("\x00") to instruct
> the driver to send the data, and then I sent another one-byte packet
> ("\x02") to instruct the driver to disconnect.
>
> This works fine with AF_PACKET/DGRAM sockets. However, when I change
> it to AF_PACKET/RAW sockets, kernel panic occurs. The stack trace is
> as follows. We can see the kernel panicked because of insufficient
> header space when pushing the Ethernet header.
>
> [ 168.399197] skbuff: skb_under_panic: text:ffffffff819d95fb len:20
> put:14 head:ffff8882704c0a00 data:ffff8882704c09fd tail:0x11 end:0xc0
> dev:veth0
> ...
> [ 168.399255] Call Trace:
> [ 168.399259] skb_push.cold+0x14/0x24
> [ 168.399262] eth_header+0x2b/0xc0
> [ 168.399267] lapbeth_data_transmit+0x9a/0xb0 [lapbether]
> [ 168.399275] lapb_data_transmit+0x22/0x2c [lapb]
> [ 168.399277] lapb_transmit_buffer+0x71/0xb0 [lapb]
> [ 168.399279] lapb_kick+0xe3/0x1c0 [lapb]
> [ 168.399281] lapb_data_request+0x76/0xc0 [lapb]
> [ 168.399283] lapbeth_xmit+0x56/0x90 [lapbether]
> [ 168.399286] dev_hard_start_xmit+0x91/0x1f0
> [ 168.399289] ? irq_init_percpu_irqstack+0xc0/0x100
> [ 168.399291] __dev_queue_xmit+0x721/0x8e0
> [ 168.399295] ? packet_parse_headers.isra.0+0xd2/0x110
> [ 168.399297] dev_queue_xmit+0x10/0x20
> [ 168.399298] packet_sendmsg+0xbf0/0x19b0
> ......
>
> After applying this patch, the kernel panic no longer appears, and
> AF_PACKET/RAW sockets would then behave the same as AF_PACKET/DGRAM
> sockets.
Thanks for fixing a kernel panic. The existing line was added recently
in commit 9dc829a135fb ("drivers/net/wan/lapbether: Fixed the value of
hard_header_len"). I assume a kernel with that commit reverted also
panics? It does looks like it would.
If this driver submits a modified packet to an underlying eth device,
it is akin to tunnel drivers. The hard_header_len vs needed_headroom
discussion also came up there recently [1]. That discussion points to
commit c95b819ad75b ("gre: Use needed_headroom"). So the general
approach in this patch is fine. Do note the point about mtu
calculations -- but this device just hardcodes a 1000 byte dev->mtu
irrespective of underlying ethernet device mtu, so I guess it has
bigger issues on that point.
But, packet sockets with SOCK_RAW have to pass a fully formed packet
with all the headers the ndo_start_xmit expects, i.e., it should be
safe for the device to just pull that many bytes. X25 requires the
peculiar one byte pseudo header you mention: lapbeth_xmit
unconditionally reads skb->data[0] and then calls skb_pull(skb, 1).
This could be considered the device hard header len.
[1] https://lore.kernel.org/netdev/86c71cc0-462c-2365-00ea-7f9e79c204b7@6wind.com/
^ permalink raw reply
* Re: [Linux-kernel-mentees] [PATCH net] rds: Prevent kernel-infoleak in rds_notify_queue_get()
From: Jason Gunthorpe @ 2020-07-31 14:04 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Leon Romanovsky, Peilin Ye, Santosh Shilimkar, David S. Miller,
Jakub Kicinski, Dan Carpenter, Arnd Bergmann,
linux-kernel-mentees, netdev, linux-rdma, rds-devel, linux-kernel
In-Reply-To: <20200731053333.GB466103@kroah.com>
On Fri, Jul 31, 2020 at 07:33:33AM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 31, 2020 at 07:33:06AM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Jul 31, 2020 at 07:53:01AM +0300, Leon Romanovsky wrote:
> > > On Thu, Jul 30, 2020 at 03:20:26PM -0400, Peilin Ye wrote:
> > > > rds_notify_queue_get() is potentially copying uninitialized kernel stack
> > > > memory to userspace since the compiler may leave a 4-byte hole at the end
> > > > of `cmsg`.
> > > >
> > > > In 2016 we tried to fix this issue by doing `= { 0 };` on `cmsg`, which
> > > > unfortunately does not always initialize that 4-byte hole. Fix it by using
> > > > memset() instead.
> > >
> > > Of course, this is the difference between "{ 0 }" and "{}" initializations.
> >
> > Really? Neither will handle structures with holes in it, try it and
> > see.
>
> And if true, where in the C spec does it say that?
The spec was updated in C11 to require zero'ing padding when doing
partial initialization of aggregates (eg = {})
"""if it is an aggregate, every member is initialized (recursively)
according to these rules, and any padding is initialized to zero
bits;"""
The difference between {0} and the {} extension is only that {}
reliably triggers partial initialization for all kinds of aggregates,
while {0} has a number of edge cases where it can fail to compile.
IIRC gcc has cleared the padding during aggregate initialization for a
long time. Considering we have thousands of aggregate initializers it
seems likely to me Linux also requires a compiler with this C11
behavior to operate correctly.
Does this patch actually fix anything? My compiler generates identical
assembly code in either case.
Jason
^ permalink raw reply
* pull-request: bpf 2020-07-31
From: Daniel Borkmann @ 2020-07-31 13:51 UTC (permalink / raw)
To: davem; +Cc: kuba, daniel, ast, jolsa, netdev, bpf
Hi David,
The following pull-request contains BPF updates for your *net* tree.
We've added 5 non-merge commits during the last 21 day(s) which contain
a total of 5 files changed, 126 insertions(+), 18 deletions(-).
The main changes are:
1) Fix a map element leak in HASH_OF_MAPS map type, from Andrii Nakryiko.
2) Fix a NULL pointer dereference in __btf_resolve_helper_id() when no
btf_vmlinux is available, from Peilin Ye.
3) Init pos variable in __bpfilter_process_sockopt(), from Christoph Hellwig.
4) Fix a cgroup sockopt verifier test by specifying expected attach type,
from Jean-Philippe Brucker.
Please consider pulling these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
Thanks a lot!
Note that when net gets merged into net-next later on, there is a small
merge conflict in kernel/bpf/btf.c between commit 5b801dfb7feb ("bpf: Fix
NULL pointer dereference in __btf_resolve_helper_id()") from the bpf tree
and commit 138b9a0511c7 ("bpf: Remove btf_id helpers resolving") from the
net-next tree.
Resolve as follows: remove the old hunk with the __btf_resolve_helper_id()
function. Change the btf_resolve_helper_id() so it actually tests for a
NULL btf_vmlinux and bails out:
int btf_resolve_helper_id(struct bpf_verifier_log *log,
const struct bpf_func_proto *fn, int arg)
{
int id;
if (fn->arg_type[arg] != ARG_PTR_TO_BTF_ID || !btf_vmlinux)
return -EINVAL;
id = fn->btf_id[arg];
if (!id || id > btf_vmlinux->nr_types)
return -EINVAL;
return id;
}
Let me know if you run into any others issues (CC'ing Jiri Olsa so he's in
the loop with regards to merge conflict resolution).
Also thanks to reporters, reviewers and testers of commits in this pull-request:
Christian Brauner, Jakub Sitnicki, Rodrigo Madera, Song Liu
----------------------------------------------------------------
The following changes since commit c8b1d7436045d3599bae56aef1682813ecccaad7:
bnxt_en: fix NULL dereference in case SR-IOV configuration fails (2020-07-10 14:20:03 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
for you to fetch changes up to 4f010246b4087ab931b060481014ec110e6a8a46:
net/bpfilter: Initialize pos in __bpfilter_process_sockopt (2020-07-31 01:07:32 +0200)
----------------------------------------------------------------
Andrii Nakryiko (2):
bpf: Fix map leak in HASH_OF_MAPS map
selftests/bpf: Extend map-in-map selftest to detect memory leaks
Christoph Hellwig (1):
net/bpfilter: Initialize pos in __bpfilter_process_sockopt
Jean-Philippe Brucker (1):
selftests/bpf: Fix cgroup sockopt verifier test
Peilin Ye (1):
bpf: Fix NULL pointer dereference in __btf_resolve_helper_id()
kernel/bpf/btf.c | 5 +
kernel/bpf/hashtab.c | 12 +-
net/bpfilter/bpfilter_kern.c | 2 +-
.../selftests/bpf/prog_tests/btf_map_in_map.c | 124 ++++++++++++++++++---
.../testing/selftests/bpf/verifier/event_output.c | 1 +
5 files changed, 126 insertions(+), 18 deletions(-)
^ permalink raw reply
* [PATCH net-next 1/2] ipv6/addrconf: call addrconf_ifdown with consistent values
From: Florent Fourcot @ 2020-07-31 13:32 UTC (permalink / raw)
To: netdev; +Cc: Florent Fourcot
Second parameter of addrconf_ifdown "how" is used as a boolean
internally. It does not make sense to call it with something different
of 0 or 1.
This value is set to 2 in all git history.
Signed-off-by: Florent Fourcot <florent.fourcot@wifirst.fr>
---
net/ipv6/addrconf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 840bfdb3d7bd..861265fa9d6d 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -7189,7 +7189,7 @@ void addrconf_cleanup(void)
continue;
addrconf_ifdown(dev, 1);
}
- addrconf_ifdown(init_net.loopback_dev, 2);
+ addrconf_ifdown(init_net.loopback_dev, 1);
/*
* Check hash table.
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 2/2] ipv6/addrconf: use a boolean to choose between UNREGISTER/DOWN
From: Florent Fourcot @ 2020-07-31 13:32 UTC (permalink / raw)
To: netdev; +Cc: Florent Fourcot
In-Reply-To: <20200731133207.26964-1-florent.fourcot@wifirst.fr>
"how" was used as a boolean. Change the type to bool, and improve
variable name
Signed-off-by: Florent Fourcot <florent.fourcot@wifirst.fr>
---
net/ipv6/addrconf.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 861265fa9d6d..0acf6a9796ca 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -163,7 +163,7 @@ static void addrconf_leave_anycast(struct inet6_ifaddr *ifp);
static void addrconf_type_change(struct net_device *dev,
unsigned long event);
-static int addrconf_ifdown(struct net_device *dev, int how);
+static int addrconf_ifdown(struct net_device *dev, bool unregister);
static struct fib6_info *addrconf_get_prefix_route(const struct in6_addr *pfx,
int plen,
@@ -3630,7 +3630,7 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event,
* an L3 master device (e.g., VRF)
*/
if (info->upper_dev && netif_is_l3_master(info->upper_dev))
- addrconf_ifdown(dev, 0);
+ addrconf_ifdown(dev, false);
}
return NOTIFY_OK;
@@ -3663,9 +3663,9 @@ static bool addr_is_local(const struct in6_addr *addr)
(IPV6_ADDR_LINKLOCAL | IPV6_ADDR_LOOPBACK);
}
-static int addrconf_ifdown(struct net_device *dev, int how)
+static int addrconf_ifdown(struct net_device *dev, bool unregister)
{
- unsigned long event = how ? NETDEV_UNREGISTER : NETDEV_DOWN;
+ unsigned long event = unregister ? NETDEV_UNREGISTER : NETDEV_DOWN;
struct net *net = dev_net(dev);
struct inet6_dev *idev;
struct inet6_ifaddr *ifa, *tmp;
@@ -3684,7 +3684,7 @@ static int addrconf_ifdown(struct net_device *dev, int how)
* Step 1: remove reference to ipv6 device from parent device.
* Do not dev_put!
*/
- if (how) {
+ if (unregister) {
idev->dead = 1;
/* protected by rtnl_lock */
@@ -3698,7 +3698,7 @@ static int addrconf_ifdown(struct net_device *dev, int how)
/* combine the user config with event to determine if permanent
* addresses are to be removed from address hash table
*/
- if (!how && !idev->cnf.disable_ipv6) {
+ if (!unregister && !idev->cnf.disable_ipv6) {
/* aggregate the system setting and interface setting */
int _keep_addr = net->ipv6.devconf_all->keep_addr_on_down;
@@ -3736,7 +3736,7 @@ static int addrconf_ifdown(struct net_device *dev, int how)
addrconf_del_rs_timer(idev);
/* Step 2: clear flags for stateless addrconf */
- if (!how)
+ if (!unregister)
idev->if_flags &= ~(IF_RS_SENT|IF_RA_RCVD|IF_READY);
/* Step 3: clear tempaddr list */
@@ -3806,7 +3806,7 @@ static int addrconf_ifdown(struct net_device *dev, int how)
write_unlock_bh(&idev->lock);
/* Step 5: Discard anycast and multicast list */
- if (how) {
+ if (unregister) {
ipv6_ac_destroy_dev(idev);
ipv6_mc_destroy_dev(idev);
} else {
@@ -3816,7 +3816,7 @@ static int addrconf_ifdown(struct net_device *dev, int how)
idev->tstamp = jiffies;
/* Last: Shot the device (if unregistered) */
- if (how) {
+ if (unregister) {
addrconf_sysctl_unregister(idev);
neigh_parms_release(&nd_tbl, idev->nd_parms);
neigh_ifdown(&nd_tbl, dev);
@@ -4038,7 +4038,7 @@ static void addrconf_dad_work(struct work_struct *w)
in6_ifa_hold(ifp);
addrconf_dad_stop(ifp, 1);
if (disable_ipv6)
- addrconf_ifdown(idev->dev, 0);
+ addrconf_ifdown(idev->dev, false);
goto out;
}
@@ -7187,9 +7187,9 @@ void addrconf_cleanup(void)
for_each_netdev(&init_net, dev) {
if (__in6_dev_get(dev) == NULL)
continue;
- addrconf_ifdown(dev, 1);
+ addrconf_ifdown(dev, true);
}
- addrconf_ifdown(init_net.loopback_dev, 1);
+ addrconf_ifdown(init_net.loopback_dev, true);
/*
* Check hash table.
--
2.20.1
^ permalink raw reply related
* Re: iproute2 DDMMYY versioning - why?
From: Or Gerlitz @ 2020-07-31 13:23 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Linux Netdev List
In-Reply-To: <20200728125136.6c5b46e8@hermes.lan>
On Tue, Jul 28, 2020 at 10:51 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
> It is only an historical leftover, because 15 yrs ago that is how Alexy did it
So how about putting behind the burden created by this historical leftover
and moving to use the kernel releases as the emitted version?
Or.
^ 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