* Re: [PATCH net 00/15] netpoll: avoid capture effects for NAPI drivers
From: Song Liu @ 2018-09-25 18:25 UTC (permalink / raw)
To: Michael Chan
Cc: Eric Dumazet, David Miller, Netdev, Ariel Elior, Eric Dumazet,
tariqt@mellanox.com, Saeed Mahameed, jeffrey.t.kirsher@intel.com,
Jakub Kicinski, j.vosburgh@gmail.com, vfalico@gmail.com,
Andy Gospodarek
In-Reply-To: <CACKFLin1samKmyTJSYnZxEUGMue7SPWaFcv=ZrSvK-DrHhuxJA@mail.gmail.com>
> On Sep 25, 2018, at 7:43 AM, Michael Chan <michael.chan@broadcom.com> wrote:
>
> On Tue, Sep 25, 2018 at 7:20 AM Eric Dumazet <edumazet@google.com> wrote:
>>
>> On Tue, Sep 25, 2018 at 7:02 AM Michael Chan <michael.chan@broadcom.com> wrote:
>>>
>>> On Mon, Sep 24, 2018 at 2:18 PM Song Liu <songliubraving@fb.com> wrote:
>>>>
>>>>
>>>>
>>>>> On Sep 24, 2018, at 2:05 PM, Eric Dumazet <edumazet@google.com> wrote:
>>>>>
>>>>>>
>>>>>> Interesting, maybe a bnxt specific issue.
>>>>>>
>>>>>> It seems their model is to process TX/RX notification in the same queue,
>>>>>> they throw away RX events if budget == 0
>>>>>>
>>>>>> It means commit e7b9569102995ebc26821789628eef45bd9840d8 is wrong and
>>>>>> must be reverted.
>>>>>>
>>>>>> Otherwise, we have a possibility of blocking a queue under netpoll pressure.
>>>>>
>>>>> Hmm, actually a revert might not be enough, since code at lines 2030-2031
>>>>> would fire and we might not call napi_complete_done() anyway.
>>>>>
>>>>> Unfortunately this driver logic is quite complex.
>>>>>
>>>>> Could you test on other NIC eventually ?
>>>>>
>>>>
>>>> It actually runs OK on ixgbe.
>>>>
>>>> @Michael, could you please help us with this?
>>>>
>>> I've taken a quick look using today's net tree plus Eric's
>>> poll_one_napi() patch. The problem I'm seeing is that netpoll calls
>>> bnxt_poll() with budget 0. And since work_done >= budget of 0, we
>>> return without calling napi_complete_done() and without arming the
>>> interrupt. netpoll doesn't always call us back until we call
>>> napi_complete_done(), right? So I think if there are in-flight TX
>>> completions, we'll miss those.
>>
>> That's the whole point of netpoll :
>>
>> We drain the TX queues, without interrupts being involved at all,
>> by calling ->napi() with a zero budget.
>>
>> napi_complete(), even if called from ->napi() while budget was zero,
>> should do nothing but return early.
>>
>> budget==0 means that ->napi() should process all TX completions.
>
> All TX completions that we can see. We cannot see the in-flight ones.
>
> If budget is exceeded, I think the assumption is that poll will always
> be called again.
>
>>
>> So it looks like bnxt has a bug, that is showing up after the latest
>> poll_one_napi() patch.
>> This latest patch is needed otherwise the cpu attempting the
>> netpoll-TX-drain might drain nothing at all,
>> since it does not anymore call ndo_poll_controller() that was grabbing
>> SCHED bits on all queues (napi_schedule() like calls)
>
> I think the latest patch is preventing the normal interrupt -> NAPI
> path from coming in and cleaning the remaining TX completions and
> arming the interrupt.
Hi Michael,
This may not be related. But I am looking at this:
bnxt_poll_work() {
while (1) {
....
if (rx_pkts == budget)
return
}
}
With budget of 0, the loop will terminate after processing one packet.
But I think the expectation is to finish all tx packets. So it doesn't
feel right. Could you please confirm?
Thanks,
Song
^ permalink raw reply
* Re: [PATCH] net/ncsi: Add NCSI OEM command for FB Tiogapass
From: Vijay Khemka @ 2018-09-25 18:16 UTC (permalink / raw)
To: Joel Stanley, Sam Mendoza-Jonas
Cc: linux-aspeed@lists.ozlabs.org, OpenBMC Maillist, Sai Dasari,
Amithash Prasad, netdev@vger.kernel.org
In-Reply-To: <CACPK8XcDExGvCNBcFqOOP+1zQFFs6pMao+7rsZrJFqxPgcY+1Q@mail.gmail.com>
Hi Joel,
Thanks, I am adding netdev mailing list here.
Yes, this command is supported for all Mellanox card. It is as per Mellanox specification.
Regards
-Vijay
On 9/24/18, 5:30 PM, "Joel Stanley" <joel@jms.id.au> wrote:
Hi Vijay,
On Tue, 25 Sep 2018 at 09:39, Vijay Khemka <vijaykhemka@fb.com> wrote:
>
> This patch adds OEM command to get mac address from NCSI device and and
> configure the same to the network card.
>
> ncsi_cmd_arg - Modified this structure to include bigger payload data.
> ncsi_cmd_handler_oem: This function handes oem command request
> ncsi_rsp_handler_oem: This function handles response for OEM command.
> get_mac_address_oem_mlx: This function will send OEM command to get
> mac address for Mellanox card
> set_mac_affinity_mlx: This will send OEM command to set Mac affinity
> for Mellanox card
Thanks for the patch. The code looks okay, but I wanted to get some
input from our NCSI maintainer as to how OEM commands would be
structured. Sam, can you please provide some review here?
Is the command supported on all melanox cards, just some, or does
TiogaPass have a special firmware that enables it?
We should include the netdev mailing list in this discussion as the
patch needs to be acceptable for upstream.
Cheers,
Joel
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> net/ncsi/Kconfig | 3 ++
> net/ncsi/internal.h | 11 +++++--
> net/ncsi/ncsi-cmd.c | 24 +++++++++++++--
> net/ncsi/ncsi-manage.c | 68 ++++++++++++++++++++++++++++++++++++++++++
> net/ncsi/ncsi-pkt.h | 16 ++++++++++
> net/ncsi/ncsi-rsp.c | 33 +++++++++++++++++++-
> 6 files changed, 149 insertions(+), 6 deletions(-)
>
> diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
> index 08a8a6031fd7..b8bf89fea7c8 100644
> --- a/net/ncsi/Kconfig
> +++ b/net/ncsi/Kconfig
> @@ -10,3 +10,6 @@ config NET_NCSI
> support. Enable this only if your system connects to a network
> device via NCSI and the ethernet driver you're using supports
> the protocol explicitly.
> +config NCSI_OEM_CMD_GET_MAC
> + bool "Get NCSI OEM MAC Address"
> + depends on NET_NCSI
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index 8055e3965cef..da17958e6a4b 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -68,6 +68,10 @@ enum {
> NCSI_MODE_MAX
> };
>
> +#define NCSI_OEM_MFR_MLX_ID 0x8119
> +#define NCSI_OEM_MLX_CMD_GET_MAC 0x1b00
> +#define NCSI_OEM_MLX_CMD_SET_AFFINITY 0x010700
> +
> struct ncsi_channel_version {
> u32 version; /* Supported BCD encoded NCSI version */
> u32 alpha2; /* Supported BCD encoded NCSI version */
> @@ -236,6 +240,7 @@ enum {
> ncsi_dev_state_probe_dp,
> ncsi_dev_state_config_sp = 0x0301,
> ncsi_dev_state_config_cis,
> + ncsi_dev_state_config_oem_gma,
> ncsi_dev_state_config_clear_vids,
> ncsi_dev_state_config_svf,
> ncsi_dev_state_config_ev,
> @@ -301,9 +306,9 @@ struct ncsi_cmd_arg {
> unsigned short payload; /* Command packet payload length */
> unsigned int req_flags; /* NCSI request properties */
> union {
> - unsigned char bytes[16]; /* Command packet specific data */
> - unsigned short words[8];
> - unsigned int dwords[4];
> + unsigned char bytes[64]; /* Command packet specific data */
> + unsigned short words[32];
> + unsigned int dwords[16];
> };
> };
>
> diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
> index 7567ca63aae2..3205e22c1734 100644
> --- a/net/ncsi/ncsi-cmd.c
> +++ b/net/ncsi/ncsi-cmd.c
> @@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
> return 0;
> }
>
> +static int ncsi_cmd_handler_oem(struct sk_buff *skb,
> + struct ncsi_cmd_arg *nca)
> +{
> + struct ncsi_cmd_oem_pkt *cmd;
> + unsigned int len;
> +
> + len = sizeof(struct ncsi_cmd_pkt_hdr) + 4;
> + if (nca->payload < 26)
> + len += 26;
> + else
> + len += nca->payload;
> +
> + cmd = skb_put_zero(skb, len);
> + memcpy(cmd->data, nca->bytes, nca->payload);
> + ncsi_cmd_build_header(&cmd->cmd.common, nca);
> +
> + return 0;
> +}
> +
> static struct ncsi_cmd_handler {
> unsigned char type;
> int payload;
> @@ -244,7 +263,7 @@ static struct ncsi_cmd_handler {
> { NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default },
> { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default },
> { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default },
> - { NCSI_PKT_CMD_OEM, 0, NULL },
> + { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem },
> { NCSI_PKT_CMD_PLDM, 0, NULL },
> { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
> };
> @@ -317,7 +336,8 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
> }
>
> /* Get packet payload length and allocate the request */
> - nca->payload = nch->payload;
> + if (nch->payload >= 0)
> + nca->payload = nch->payload;
> nr = ncsi_alloc_command(nca);
> if (!nr)
> return -ENOMEM;
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 091284760d21..3b2b86560cc8 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -635,6 +635,58 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
> return 0;
> }
>
> +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> +/* NCSI Facebook OEM APIs */
> +static void get_mac_address_oem_mlx(struct ncsi_dev_priv *ndp)
> +{
> + struct ncsi_cmd_arg nca;
> + int ret = 0;
> +
> + memset(&nca, 0, sizeof(struct ncsi_cmd_arg));
> + nca.ndp = ndp;
> + nca.channel = ndp->active_channel->id;
> + nca.package = ndp->active_package->id;
> + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
> + nca.type = NCSI_PKT_CMD_OEM;
> + nca.payload = 8;
> +
> + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID);
> + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_GET_MAC);
> +
> + ret = ncsi_xmit_cmd(&nca);
> + if (ret)
> + netdev_err(ndp->ndev.dev,
> + "NCSI: Failed to transmit cmd 0x%x during probe\n",
> + nca.type);
> +}
> +
> +static void set_mac_affinity_mlx(struct ncsi_dev_priv *ndp)
> +{
> + struct ncsi_cmd_arg nca;
> + int ret = 0;
> +
> + memset(&nca, 0, sizeof(struct ncsi_cmd_arg));
> + nca.ndp = ndp;
> + nca.channel = ndp->active_channel->id;
> + nca.package = ndp->active_package->id;
> + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
> + nca.type = NCSI_PKT_CMD_OEM;
> + nca.payload = 60;
> +
> + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID);
> + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_SET_AFFINITY);
> +
> + memcpy(&(nca.bytes[8]), ndp->ndev.dev->dev_addr, ETH_ALEN);
> + nca.bytes[14] = 0x09;
> +
> + ret = ncsi_xmit_cmd(&nca);
> + if (ret)
> + netdev_err(ndp->ndev.dev,
> + "NCSI: Failed to transmit cmd 0x%x during probe\n",
> + nca.type);
> +}
> +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
> +
> static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> {
> struct ncsi_dev *nd = &ndp->ndev;
> @@ -685,6 +737,22 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> goto error;
> }
>
> +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> + /* Check Manufacture id if it is Mellanox then
> + * get and set mac address. To Do: Add code for
> + * other types of card if required
> + */
> + if (nc->version.mf_id == NCSI_OEM_MFR_MLX_ID)
> + nd->state = ncsi_dev_state_config_oem_gma;
> + else
> + nd->state = ncsi_dev_state_config_clear_vids;
> + break;
> + case ncsi_dev_state_config_oem_gma:
> + ndp->pending_req_num = 2;
> + get_mac_address_oem_mlx(ndp);
> + msleep(500);
> + set_mac_affinity_mlx(ndp);
> +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
> nd->state = ncsi_dev_state_config_clear_vids;
> break;
> case ncsi_dev_state_config_clear_vids:
> diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
> index 91b4b66438df..0653a893eb12 100644
> --- a/net/ncsi/ncsi-pkt.h
> +++ b/net/ncsi/ncsi-pkt.h
> @@ -151,6 +151,22 @@ struct ncsi_cmd_snfc_pkt {
> unsigned char pad[22];
> };
>
> +/* Oem Request Command */
> +struct ncsi_cmd_oem_pkt {
> + struct ncsi_cmd_pkt_hdr cmd; /* Command header */
> + unsigned char data[64]; /* OEM Payload Data */
> + __be32 checksum; /* Checksum */
> +};
> +
> +/* Oem Response Packet */
> +struct ncsi_rsp_oem_pkt {
> + struct ncsi_rsp_pkt_hdr rsp; /* Command header */
> + __be32 mfr_id; /* Manufacture ID */
> + __be32 oem_cmd; /* oem command */
> + unsigned char data[32]; /* Payload data */
> + __be32 checksum; /* Checksum */
> +};
> +
> /* Get Link Status */
> struct ncsi_rsp_gls_pkt {
> struct ncsi_rsp_pkt_hdr rsp; /* Response header */
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index 930c1d3796f0..3b94c96b9c7f 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -596,6 +596,37 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
> return 0;
> }
>
> +static int ncsi_rsp_handler_oem(struct ncsi_request *nr)
> +{
> + struct ncsi_rsp_oem_pkt *rsp;
> + struct ncsi_dev_priv *ndp = nr->ndp;
> + struct net_device *ndev = ndp->ndev.dev;
> + int ret = 0;
> + unsigned int oem_cmd, mfr_id;
> + const struct net_device_ops *ops = ndev->netdev_ops;
> + struct sockaddr saddr;
> +
> + /* Get the response header */
> + rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
> +
> + oem_cmd = ntohl(rsp->oem_cmd);
> + mfr_id = ntohl(rsp->mfr_id);
> +
> + /* Check for Mellanox manufacturer id */
> + if (mfr_id != NCSI_OEM_MFR_MLX_ID)
> + return 0;
> +
> + if (oem_cmd == NCSI_OEM_MLX_CMD_GET_MAC) {
> + saddr.sa_family = ndev->type;
> + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> + memcpy(saddr.sa_data, &(rsp->data[4]), ETH_ALEN);
> + ret = ops->ndo_set_mac_address(ndev, &saddr);
> + if (ret < 0)
> + netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n");
> + }
> + return ret;
> +}
> +
> static int ncsi_rsp_handler_gvi(struct ncsi_request *nr)
> {
> struct ncsi_rsp_gvi_pkt *rsp;
> @@ -932,7 +963,7 @@ static struct ncsi_rsp_handler {
> { NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns },
> { NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts },
> { NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps },
> - { NCSI_PKT_RSP_OEM, 0, NULL },
> + { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem },
> { NCSI_PKT_RSP_PLDM, 0, NULL },
> { NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid }
> };
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH net-next v3 00/10] Refactor classifier API to work with Qdisc/blocks without rtnl lock
From: Cong Wang @ 2018-09-25 18:16 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller, Stephen Hemminger, Kirill Tkhai, Nicolas Dichtel,
Greg KH, mark.rutland, Leon Romanovsky, Paul E. McKenney,
Florian Westphal, David Ahern, christian, lucien xin,
Jakub Kicinski, Jiri Benc
In-Reply-To: <1537806178-10944-1-git-send-email-vladbu@mellanox.com>
On Mon, Sep 24, 2018 at 9:23 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Currently, all netlink protocol handlers for updating rules, actions and
> qdiscs are protected with single global rtnl lock which removes any
> possibility for parallelism. This patch set is a third step to remove
> rtnl lock dependency from TC rules update path.
>
> Recently, new rtnl registration flag RTNL_FLAG_DOIT_UNLOCKED was added.
> Handlers registered with this flag are called without RTNL taken. End
> goal is to have rule update handlers(RTM_NEWTFILTER, RTM_DELTFILTER,
> etc.) to be registered with UNLOCKED flag to allow parallel execution.
> However, there is no intention to completely remove or split rtnl lock
> itself. This patch set addresses specific problems in implementation of
> classifiers API that prevent its control path from being executed
> concurrently. Additional changes are required to refactor classifiers
> API and individual classifiers for parallel execution. This patch set
> lays groundwork to eventually register rule update handlers as
> rtnl-unlocked by modifying code in cls API that works with Qdiscs and
> blocks. Following patch set does the same for chains and classifiers.
>
> The goal of this change is to refactor tcf_block_find() and its
> dependencies to allow concurrent execution:
> - Extend Qdisc API with rcu to lookup and take reference to Qdisc
> without relying on rtnl lock.
> - Extend tcf_block with atomic reference counting and rcu.
> - Always take reference to tcf_block while working with it.
> - Implement tcf_block_release() to release resources obtained by
> tcf_block_find()
> - Create infrastructure to allow registering Qdiscs with class ops that
> do not require the caller to hold rtnl lock.
>
> All three netlink rule update handlers use tcf_block_find() to lookup
> Qdisc and block, and this patch set introduces additional means of
> synchronization to substitute rtnl lock in cls API.
>
> Some functions in cls and sch APIs have historic names that no longer
> clearly describe their intent. In order not make this code even more
> confusing when introducing their concurrency-friendly versions, rename
> these functions to describe actual implementation.
>
> Changes from V2 to V3:
> - Patch 1:
> - Explicitly include refcount.h in rtnetlink.h.
> - Patch 3:
> - Move rcu_head field to the end of struct Qdisc.
> - Rearrange local variable declarations in qdisc_lookup_rcu().
> - Patch 5:
> - Remove tcf_qdisc_put() and inline its content to callers.
Overall looks good to me,
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
There are some minor issues like qdisc_free_cb() can be static,
please send follow-up patch to address it.
Thanks.
^ permalink raw reply
* Re: [PATCH 0/3] mac80211_hwsim: radio destruction fixes
From: Benjamin Beichler @ 2018-09-25 18:00 UTC (permalink / raw)
To: Martin Willi, Johannes Berg; +Cc: linux-wireless, netdev
In-Reply-To: <20180925074115.18169-1-martin@strongswan.org>
Am 25.09.2018 um 09:41 schrieb Martin Willi:
> This small series fixes two issues for cleaning up hwsim radios. The
> first one is rather easy to hit when terminating namespaces with many
> hwsim radios. The second one affects destroy-on-close users only.
>
> Given that the use of a work-queue for deferred cleanup with namespaces
> has been and still is tricky to get right, this series switches these
> users to a synchronous cleanup in hwsim; The removal of that work-queue
> is in a dedicated commit in case we want to skip that in backports.
Indeed, this is a much better solution. I wasn't that happy with the
workqueue thing, but I didn't know a better solution :-)
I see no errors at review, but I currently cannot test it quickly.
> Martin Willi (3):
> mac80211_hwsim: fix locking when iterating radios during ns exit
> mac80211_hwsim: fix race in radio destruction from netlink notifier
> mac80211_hwsim: drop now unused work-queue from hwsim
>
> drivers/net/wireless/mac80211_hwsim.c | 44 +++++++++++----------------
> 1 file changed, 17 insertions(+), 27 deletions(-)
>
kind regards
Benjamin
--
M.Sc. Benjamin Beichler
Universität Rostock, Fakultät für Informatik und Elektrotechnik
Institut für Angewandte Mikroelektronik und Datentechnik
University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE
Richard-Wagner-Straße 31
18119 Rostock
Deutschland/Germany
phone: +49 (0) 381 498 - 7278
email: Benjamin.Beichler@uni-rostock.de
www: http://www.imd.uni-rostock.de/
^ permalink raw reply
* Re: [net 1/1] tipc: fix flow control accounting for implicit connect
From: David Miller @ 2018-09-25 17:53 UTC (permalink / raw)
To: jon.maloy
Cc: netdev, gordan.mihaljevic, tung.q.nguyen, hoang.h.le, canh.d.luu,
ying.xue, tipc-discussion
In-Reply-To: <1537892518-30438-1-git-send-email-jon.maloy@ericsson.com>
From: Jon Maloy <jon.maloy@ericsson.com>
Date: Tue, 25 Sep 2018 18:21:58 +0200
> From: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
>
> In the case of implicit connect message with data > 1K, the flow
> control accounting is incorrect. At this state, the socket does not
> know the peer nodes capability and falls back to legacy flow control
> by return 1, however the receiver of this message will perform the
> new block accounting. This leads to a slack and eventually traffic
> disturbance.
>
> In this commit, we perform tipc_node_get_capabilities() at implicit
> connect and perform accounting based on the peer's capability.
>
> Signed-off-by: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: [PATCH net-next] tls: Fixed a memory leak during socket close
From: David Miller @ 2018-09-25 17:52 UTC (permalink / raw)
To: vakul.garg; +Cc: netdev, borisp, aviadye, davejwatson, doronrk
In-Reply-To: <20180925145151.10992-1-vakul.garg@nxp.com>
From: Vakul Garg <vakul.garg@nxp.com>
Date: Tue, 25 Sep 2018 20:21:51 +0530
> During socket close, if there is a open record with tx context, it needs
> to be be freed apart from freeing up plaintext and encrypted scatter
> lists. This patch frees up the open record if present in tx context.
>
> Also tls_free_both_sg() has been renamed to tls_free_open_rec() to
> indicate that the free record in tx context is being freed inside the
> function.
>
> Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption")
> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
Applied.
^ permalink raw reply
* KASAN: slab-out-of-bounds Read in tls_write_space
From: syzbot @ 2018-09-25 23:54 UTC (permalink / raw)
To: aviadye, borisp, davejwatson, davem, linux-kernel, netdev,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: bd4d08daeb95 Merge branch 'net-dsa-b53-SGMII-modes-fixes'
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=110b6a1a400000
kernel config: https://syzkaller.appspot.com/x/.config?x=e79b9299baeb2298
dashboard link: https://syzkaller.appspot.com/bug?extid=12638b747fd208f6cff0
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=167d9b9e400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15c4003a400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+12638b747fd208f6cff0@syzkaller.appspotmail.com
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
TCP: request_sock_TCPv6: Possible SYN flooding on port 20002. Sending
cookies. Check SNMP counters.
==================================================================
BUG: KASAN: slab-out-of-bounds in __swab64p include/uapi/linux/swab.h:192
[inline]
BUG: KASAN: slab-out-of-bounds in __be64_to_cpup
include/uapi/linux/byteorder/little_endian.h:74 [inline]
BUG: KASAN: slab-out-of-bounds in is_tx_ready include/net/tls.h:354 [inline]
BUG: KASAN: slab-out-of-bounds in tls_write_space+0x29d/0x2d0
net/tls/tls_main.c:236
Read of size 8 at addr ffff8801bc3c9ff0 by task ksoftirqd/1/18
CPU: 1 PID: 18 Comm: ksoftirqd/1 Not tainted 4.19.0-rc4+ #227
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
__asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
__swab64p include/uapi/linux/swab.h:192 [inline]
__be64_to_cpup include/uapi/linux/byteorder/little_endian.h:74 [inline]
is_tx_ready include/net/tls.h:354 [inline]
tls_write_space+0x29d/0x2d0 net/tls/tls_main.c:236
tcp_new_space net/ipv4/tcp_input.c:5154 [inline]
tcp_check_space+0x53f/0x920 net/ipv4/tcp_input.c:5165
tcp_data_snd_check net/ipv4/tcp_input.c:5175 [inline]
tcp_rcv_established+0xde8/0x2120 net/ipv4/tcp_input.c:5656
tcp_v6_do_rcv+0x4b3/0x13c0 net/ipv6/tcp_ipv6.c:1326
tcp_v6_rcv+0x2f7a/0x38a0 net/ipv6/tcp_ipv6.c:1555
ip6_input_finish+0x3fc/0x1aa0 net/ipv6/ip6_input.c:384
NF_HOOK include/linux/netfilter.h:287 [inline]
ip6_input+0xe9/0x600 net/ipv6/ip6_input.c:427
dst_input include/net/dst.h:450 [inline]
ip6_rcv_finish+0x17a/0x330 net/ipv6/ip6_input.c:76
NF_HOOK include/linux/netfilter.h:287 [inline]
ipv6_rcv+0x113/0x640 net/ipv6/ip6_input.c:272
__netif_receive_skb_one_core+0x14d/0x200 net/core/dev.c:4894
__netif_receive_skb+0x2c/0x1e0 net/core/dev.c:5004
process_backlog+0x217/0x760 net/core/dev.c:5808
napi_poll net/core/dev.c:6228 [inline]
net_rx_action+0x7c5/0x1950 net/core/dev.c:6294
__do_softirq+0x30b/0xad8 kernel/softirq.c:292
run_ksoftirqd+0x94/0x100 kernel/softirq.c:653
smpboot_thread_fn+0x68b/0xa00 kernel/smpboot.c:164
kthread+0x35a/0x420 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:413
Allocated by task 3559:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
kmem_cache_alloc_trace+0x152/0x750 mm/slab.c:3620
kmalloc include/linux/slab.h:513 [inline]
kzalloc include/linux/slab.h:707 [inline]
kernfs_fop_open+0x358/0xf90 fs/kernfs/file.c:648
do_dentry_open+0x499/0x1250 fs/open.c:771
vfs_open+0xa0/0xd0 fs/open.c:880
do_last fs/namei.c:3418 [inline]
path_openat+0x12bf/0x5160 fs/namei.c:3534
do_filp_open+0x255/0x380 fs/namei.c:3564
do_sys_open+0x568/0x700 fs/open.c:1063
__do_sys_open fs/open.c:1081 [inline]
__se_sys_open fs/open.c:1076 [inline]
__x64_sys_open+0x7e/0xc0 fs/open.c:1076
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 3559:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kfree+0xcf/0x230 mm/slab.c:3813
kernfs_fop_release+0x12b/0x1a0 fs/kernfs/file.c:783
__fput+0x385/0xa30 fs/file_table.c:278
____fput+0x15/0x20 fs/file_table.c:309
task_work_run+0x1e8/0x2a0 kernel/task_work.c:113
tracehook_notify_resume include/linux/tracehook.h:193 [inline]
exit_to_usermode_loop+0x318/0x380 arch/x86/entry/common.c:166
prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline]
syscall_return_slowpath arch/x86/entry/common.c:268 [inline]
do_syscall_64+0x6be/0x820 arch/x86/entry/common.c:293
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff8801bc3c9c80
which belongs to the cache kmalloc-512 of size 512
The buggy address is located 368 bytes to the right of
512-byte region [ffff8801bc3c9c80, ffff8801bc3c9e80)
The buggy address belongs to the page:
page:ffffea0006f0f240 count:1 mapcount:0 mapping:ffff8801da800940 index:0x0
flags: 0x2fffc0000000100(slab)
raw: 02fffc0000000100 ffffea0006ee1408 ffffea0007619c08 ffff8801da800940
raw: 0000000000000000 ffff8801bc3c9000 0000000100000006 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8801bc3c9e80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8801bc3c9f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8801bc3c9f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
^
ffff8801bc3ca000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8801bc3ca080: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH net-next] tls: Fix socket mem accounting error under async encryption
From: David Miller @ 2018-09-25 17:44 UTC (permalink / raw)
To: vakul.garg; +Cc: netdev, borisp, aviadye, davejwatson, doronrk
In-Reply-To: <20180925105617.22543-1-vakul.garg@nxp.com>
From: Vakul Garg <vakul.garg@nxp.com>
Date: Tue, 25 Sep 2018 16:26:17 +0530
> Current async encryption implementation sometimes showed up socket
> memory accounting error during socket close. This results in kernel
> warning calltrace. The root cause of the problem is that socket var
> sk_forward_alloc gets corrupted due to access in sk_mem_charge()
> and sk_mem_uncharge() being invoked from multiple concurrent contexts
> in multicore processor. The apis sk_mem_charge() and sk_mem_uncharge()
> are called from functions alloc_plaintext_sg(), free_sg() etc. It is
> required that memory accounting apis are called under a socket lock.
>
> The plaintext sg data sent for encryption is freed using free_sg() in
> tls_encryption_done(). It is wrong to call free_sg() from this function.
> This is because this function may run in irq context. We cannot acquire
> socket lock in this function.
>
> We remove calling of function free_sg() for plaintext data from
> tls_encryption_done() and defer freeing up of plaintext data to the time
> when the record is picked up from tx_list and transmitted/freed. When
> tls_tx_records() gets called, socket is already locked and thus there is
> no concurrent access problem.
>
> Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption")
> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
Applied.
^ permalink raw reply
* Re: [PATCH net 0/3 RESEND] xen-netback: hash mapping handling adjustments
From: David Miller @ 2018-09-25 17:40 UTC (permalink / raw)
To: JBeulich; +Cc: paul.durrant, wei.liu2, xen-devel, netdev
In-Reply-To: <5BA9EDB502000078001EB724@prv1-mh.provo.novell.com>
From: "Jan Beulich" <JBeulich@suse.com>
Date: Tue, 25 Sep 2018 02:11:33 -0600
> First and foremost the fix for XSA-270. On top of that further changes
> which looked desirable to me while investigating that XSA.
>
> 1: fix input validation in xenvif_set_hash_mapping()
> 2: validate queue numbers in xenvif_set_hash_mapping()
> 3: handle page straddling in xenvif_set_hash_mapping()
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Series applied and queued up for -stable.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/3] r8169: series with smaller improvements
From: David Miller @ 2018-09-25 17:36 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev
In-Reply-To: <2dcf188b-852a-f785-c6e4-e63041b76c86@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Tue, 25 Sep 2018 07:55:36 +0200
> This series includes smaller improvements, nothing exciting.
Series applied.
^ permalink raw reply
* Re: Marvell phy errata origins?
From: Harini Katakam @ 2018-09-25 17:30 UTC (permalink / raw)
To: Andrew Lunn
Cc: danielwa, Florian Fainelli, afleming, Harini Katakam, netdev,
hramdasi, jpitti, gcasheek
In-Reply-To: <20180925153916.GD1676@lunn.ch>
Hi Daniel,
On Tue, Sep 25, 2018 at 9:10 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > I hope this this thread isn't too old to bring back to life. So it seems
> > that Harini found that m88e1111 did not need this errata, and Cisco
> > previously found that Harini's patch fixed m88e1112, we included it
> > internally for that reason
> >
> > Now I'm getting reports that this errata fixes issues we're seeing on
> > m88e1111. We see an interrupt storm without the errata, despite the errata
> > not being defined in the datasheet.
>
> Is everybody actually using interrupts? It could be in one system
> phylib is polling.
>
Yes, we weren't using interrupts; we used phy poll.
As I recall, the register and page combination was reserved and
the access seemed to fail.
It will be useful if we can the errata description or version details.
I'll check if I can get any more information.
Regards,
Harini
^ permalink raw reply
* Re: [PATCH net v2 1/2] net: core: add member wol_enabled to struct net_device
From: Heiner Kallweit @ 2018-09-25 17:30 UTC (permalink / raw)
To: Michal Kubecek
Cc: Andrew Lunn, Florian Fainelli, David Miller,
Realtek linux nic maintainers, netdev@vger.kernel.org
In-Reply-To: <20180925082830.GC30601@unicorn.suse.cz>
On 25.09.2018 10:28, Michal Kubecek wrote:
> (I wrote my comment to v1 because I overlooked there is a v2 already;
> duplicating it here.)
>
> On Mon, Sep 24, 2018 at 09:58:59PM +0200, Heiner Kallweit wrote:
>> Add flag wol_enabled to struct net_device indicating whether
>> Wake-on-LAN is enabled. As first user phy_suspend() will use it to
>> decide whether PHY can be suspended or not.
>>
>> Fixes: f1e911d5d0df ("r8169: add basic phylib support")
>> Fixes: e8cfd9d6c772 ("net: phy: call state machine synchronously in phy_stop")
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> include/linux/netdevice.h | 3 +++
>> net/core/ethtool.c | 9 ++++++++-
>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 1cbbf77a6..f5f1f1450 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1756,6 +1756,8 @@ enum netdev_priv_flags {
>> * switch driver and used to set the phys state of the
>> * switch port.
>> *
>> + * @wol_enabled: Wake-on-LAN is enabled
>> + *
>> * FIXME: cleanup struct net_device such that network protocol info
>> * moves out.
>> */
>> @@ -2039,6 +2041,7 @@ struct net_device {
>> struct lock_class_key *qdisc_tx_busylock;
>> struct lock_class_key *qdisc_running_key;
>> bool proto_down;
>> + unsigned wol_enabled:1;
>> };
>> #define to_net_dev(d) container_of(d, struct net_device, dev)
>>
>
> As there is no bitfield yet, this would add 4 bytes to struct net_device.
> How about using a bit in net_device::priv_flags like IFF_RXFH_CONFIGURED
> in ethtool_set_rxfh_indir() and ethtool_set_rxfh()?
>
Indeed alternatively we could add a private flag and the related
getter/setter.
Regarding the size argument: We have few bool members in struct
net_device (uc_promisc, dismantle, needs_free_netdev, proto_down)
which most likely can be transparently converted to bitfield
members. Then a wol_enabled bitfield member would be no overhead.
I don't have a strong preference and would leave it up to David.
> Michal Kubecek
>
Heiner
^ permalink raw reply
* Re: [PATCH net-next v1 1/5] net: allow binding socket in a VRF when there's an unbound socket
From: David Ahern @ 2018-09-25 17:16 UTC (permalink / raw)
To: mmanning, netdev; +Cc: Robert Shearman
In-Reply-To: <97367b8a-dd62-0b42-3f07-30e4b897546a@vyatta.att-mail.com>
On 9/25/18 9:26 AM, Mike Manning wrote:
> On 24/09/2018 23:44, David Ahern wrote:
>> On 9/24/18 10:13 AM, Mike Manning wrote:
>>> From: Robert Shearman <rshearma@vyatta.att-mail.com>
>>>
>>> There is no easy way currently for applications that want to receive
>>> packets in the default VRF to be isolated from packets arriving in
>>> VRFs, which makes using VRF-unaware applications in a VRF-aware system
>>> a potential security risk.
>>
>> That comment is not correct.
>>
>> The point of the l3mdev sysctl's is to prohibit this case. Setting
>> net.ipv4.{tcp,udp}_l3mdev_accept=0 means that a packet arriving on an
>> interface enslaved to a VRF can not be received by a global socket.
> Hi David, thanks for reviewing this. The converse does not hold though,
> i.e. there is no guarantee that the unbound socket will be selected for
> packets when not in a VRF, if there is an unbound socket and a socket
> bound to a VRF. Also, such packets should not be handled by the socket
I need an explicit example here. You are saying a packet arriving on an
interface not enslaved to a VRF might match a socket bound to a VRF?
> in the VRF if there is no unbound socket. We also had an issue with raw
> socket lookup device bind matching. I can break this particular patch
> into smaller patches and provide more detail, would this help? I will
> also update/break up the other patches according to your comments.
Why not add an l3mdev sysctl for raw sockets then?
Yes, please send smaller patches. A diff stat of:
15 files changed, 109 insertions(+), 62 deletions(-)
is a bit harsh.
>
>>
>> Setting the l3mdev to 1 allows the default socket to work across VRFs.
>> If that is not what you want for a given app or a given VRF, then one
>> option is to add netfilter rules on the VRF device to prohibit it. I
>> just verified this works for both tcp and udp.
>
> Netfilter is per application and so does not scale. I have not checked
> if it is suitable for packet handling on raw sockets.
>
>>
>> Further, overlapping binds are allowed using SO_REUSEPORT meaning I can
>> have a server running in the default vrf bound to a port AND a server
>> running bound to a specific vrf and the same port:
>>
>> udp UNCONN 0 0 *%red:12345 *:*
>> users:(("vrf-test",pid=1376,fd=3))
>> udp UNCONN 0 0 *:12345 *:*
>> users:(("vrf-test",pid=1375,fd=3))
>>
>> tcp LISTEN 0 1 *%red:12345 *:*
>> users:(("vrf-test",pid=1356,fd=3))
>> tcp LISTEN 0 1 *:12345 *:*
>> users:(("vrf-test",pid=1352,fd=3))
>>
>> For packets arriving on an interface enslaved to a VRF the socket lookup
>> will pick the VRF server over the global one.
>
> Agreed, but the converse is not guaranteed to hold i.e. packets that are
> not in a VRF may be handled by a socket bound to a VRF.
>
> We do use SO_REUSEPORT for our own applications so as to run instances
> in the default and other VRFs, but still require these patches (or
> similar) due to how packets are handled when there is an unbound socket
> and sockets bound to different VRFs.
Why can't compute_score be adjusted to account for that case?
>
>>
>> --
>>
>> With this patch set I am seeing a number of tests failing -- socket
>> connections working when they should not or not working when they
>> should. I only skimmed the results. I am guessing this patch is the
>> reason, but that is just a guess.
>>
>> You need to make sure all permutations of:
>> 1. net.ipv4.{tcp,udp}_l3mdev_accept={0,1},
>> 2. connection in the default VRF and in a VRF,
>> 3. locally originated and remote traffic,
>> 4. ipv4 and ipv6
>>
>
> We are using raw, datagram and stream sockets for ipv4 & ipv6, require
> connectivity for local and remote addresses where appropriate and need
> route leaking between VRFs when configured, we are unaware of any
> outstanding bugs. Is there some way that I can run/analyze the tests
> that are failing for you?
I am not distributing my vrf tests right now. Before sending the
response I quickly verified one case is easy for you to see: set the udp
sysctl to 0, start a global server, send a packet to it via an interface
enslaved to a VRF. It should fail ECONNREFUSED (no socket match) but
instead packet reaches the server.
>
> Also cf patch 2/5 note that ping to link-local addresses is handled
> consistently with that to global addresses in a VRF, so this now
> succeeds if ping is done in the VRF, i.e. 'sudo ip vrf exec <vrf> ping
> <ll> -I <intf>
Shifting packets destined to a LLA from the real device to the vrf
device is a change in behavior. It is not clear to me at the moment that
it will not cause a problem.
>
>> continue to work as expected meaning packets flow when they should and
>> fail with the right error when they should not. I believe the UDP cases
>> were the main ones failing.
>>
>> Given the test failures, I did not look at the code changes in the patch.
>>
>
A couple of the patches are fine as is - or need a small change. It
might be easier for you to send those outside of the socket lookup set.
^ permalink raw reply
* RE: [PATCH net 3/7] lan78xx: Check for supported Wake-on-LAN modes
From: Woojung.Huh @ 2018-09-25 22:32 UTC (permalink / raw)
To: f.fainelli, netdev
Cc: davem, UNGLinuxDriver, steve.glendinning, keescook, akurz,
hayeswang, kai.heng.feng, grundler, zhongjiang, bigeasy,
ran.wang_1, edumazet, linux-usb, linux-kernel
In-Reply-To: <8de18c23-d846-cbb4-c852-9b7d97a95c11@gmail.com>
Hi Florian,
> @@ -1401,19 +1401,10 @@ static int lan78xx_set_wol(struct net_device
...
> + if (pdata->wol & ~WAKE_ALL)
> + return -EINVAL;
If I understand correctly, it needs to check againt "wol->wolopts" than pdata->wol.
> +
> + pdata->wol = wol->wolopts;
Thanks.
Woojung
^ permalink raw reply
* [net 1/1] tipc: fix flow control accounting for implicit connect
From: Jon Maloy @ 2018-09-25 16:21 UTC (permalink / raw)
To: davem, netdev
Cc: gordan.mihaljevic, tung.q.nguyen, hoang.h.le, jon.maloy,
canh.d.luu, ying.xue, tipc-discussion
From: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
In the case of implicit connect message with data > 1K, the flow
control accounting is incorrect. At this state, the socket does not
know the peer nodes capability and falls back to legacy flow control
by return 1, however the receiver of this message will perform the
new block accounting. This leads to a slack and eventually traffic
disturbance.
In this commit, we perform tipc_node_get_capabilities() at implicit
connect and perform accounting based on the peer's capability.
Signed-off-by: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/socket.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 3f03ddd..b6f99b0 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1419,8 +1419,10 @@ static int __tipc_sendstream(struct socket *sock, struct msghdr *m, size_t dlen)
/* Handle implicit connection setup */
if (unlikely(dest)) {
rc = __tipc_sendmsg(sock, m, dlen);
- if (dlen && (dlen == rc))
+ if (dlen && dlen == rc) {
+ tsk->peer_caps = tipc_node_get_capabilities(net, dnode);
tsk->snt_unacked = tsk_inc(tsk, dlen + msg_hdr_sz(hdr));
+ }
return rc;
}
--
2.1.4
^ permalink raw reply related
* Editing for 4
From: Heather @ 2018-09-25 11:17 UTC (permalink / raw)
To: netdev
Want editing for your photos? We can help you for this.
We can do cutting out and retouching for your ecommerce photos, jewelry
photos retouching,
portrait photos or wedding photos.
Turnaround time is fast
Send us one photo, we wil do testing for you.
Thanks,
Heather
^ permalink raw reply
* Re: Marvell phy errata origins?
From: Andrew Lunn @ 2018-09-25 15:39 UTC (permalink / raw)
To: Daniel Walker
Cc: Florian Fainelli, Andy Fleming, Harini Katakam,
netdev@vger.kernel.org, HEMANT RAMDASI,
Julius Hemanth Pitti -X (jpitti - MONTA VISTA SOFTWARE INC at Cisco),
GokulChand Casheekar (gcasheek)
In-Reply-To: <84bfdc12-800c-2f63-3eff-416cf560e18c@cisco.com>
> I hope this this thread isn't too old to bring back to life. So it seems
> that Harini found that m88e1111 did not need this errata, and Cisco
> previously found that Harini's patch fixed m88e1112, we included it
> internally for that reason
>
> Now I'm getting reports that this errata fixes issues we're seeing on
> m88e1111. We see an interrupt storm without the errata, despite the errata
> not being defined in the datasheet.
Is everybody actually using interrupts? It could be in one system
phylib is polling.
When you get an interrupt storm, which interrupt bit is not cleared by
reading the ievent register?
Andrew
^ permalink raw reply
* Re: [PATCH bpf-next] flow_dissector: lookup netns by skb->sk if skb->dev is NULL
From: Daniel Borkmann @ 2018-09-25 15:38 UTC (permalink / raw)
To: Willem de Bruijn, netdev; +Cc: ast, davem, Willem de Bruijn
In-Reply-To: <20180924204956.83718-1-willemdebruijn.kernel@gmail.com>
On 09/24/2018 10:49 PM, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> BPF flow dissectors are configured per network namespace.
> __skb_flow_dissect looks up the netns through dev_net(skb->dev).
>
> In some dissector paths skb->dev is NULL, such as for Unix sockets.
> In these cases fall back to looking up the netns by socket.
>
> Analyzing the codepaths leading to __skb_flow_dissect I did not find
> a case where both skb->dev and skb->sk are NULL. Warn and fall back to
> standard flow dissector if one is found.
>
> Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
> Reported-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Applied to bpf-next, thanks Willem!
^ permalink raw reply
* Re: netlink: 16 bytes leftover after parsing attributes in process `ip'.
From: David Ahern @ 2018-09-25 15:37 UTC (permalink / raw)
To: Jiri Benc, Christian Brauner; +Cc: netdev@vger.kernel.org, David Miller
In-Reply-To: <20180925164715.338bb059@redhat.com>
On 9/25/18 8:47 AM, Jiri Benc wrote:
> On Tue, 25 Sep 2018 11:49:10 +0200, Christian Brauner wrote:
>> So if people really want to hide this issue as much as we can then we
>> can play the guessing game. I could send a patch that roughly does the
>> following:
>>
>> if (nlmsg_len(cb->nlh) < sizeof(struct ifinfomsg))
>> guessed_header_len = sizeof(struct ifaddrmsg);
>> else
>> guessed_header_len = sizeof(struct ifinfomsg);
>>
>> This will work since sizeof(ifaddrmsg) == 8 and sizeof(ifinfomsg) == 16.
>> The only valid property for RTM_GETADDR requests is IFA_TARGET_NETNSID.
>> This propert is a __s32 which should bring the message up to 12 bytes
>> (not sure about alignment requiremnts and where we might wend up ten)
>> which is still less than the 16 bytes without that property from
>> ifinfomsg. That's a hacky hacky hack-hack and will likely work but will
>> break when ifaddrmsg grows a new member or we introduce another property
>> that is valid in RTM_GETADDR requests. It also will not work cleanly
>> when users stuff additional properties in there that are vaif (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,lid for the
>> address family but are not used int RTM_GETADDR requests.
>
> I'd expect that any potential existing code that makes use of other
> attributes already assumes ifaddrmsg. Hence, if the nlmsg_len is >
> sizeof(ifinfomsg), you can be sure that there are attributes and thus
> the struct used was ifaddrmsg.
>
> So, in order for RTM_GETADDR to work reliably with attributes, you have
> to ensure that the length is > sizeof(ifinfomsg).
One of the many on-going efforts I have in progress is kernel side
filtering of route dumps. It has this same problem. For it I am
proposing a new flag:
#define RTM_F_PROPER_HEADER 0x4000
ifinfomsg is 16 bytes which is > rtmsg at 12. If the message length is >
16, then rtm_flags can be checked to know if the proper header is sent.
For ifaddrmsg things do not line up as well. Worst all of the flag bits
are used. But, perhaps one can be overloaded with the limit that you can
never filter on its presence. Since you are adding the first filter to
address dumps such a limitation should be ok.
For example something like this (whitespace damaged on paste) to remove
the guess work:
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index dfcf3ce0097f..8e3e9d475db5 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -41,6 +41,11 @@ enum {
#define IFA_MAX (__IFA_MAX - 1)
/* ifa_flags */
+/* IFA_F_PROPER_HEADER is only set in ifa_flags for dumps
+ * to indicate the ancillary header is the expected ifaddrmsg
+ * vs ifinfomsg from legacy userspace
+ */
+#define IFA_F_PROPER_HEADER 0x01
#define IFA_F_SECONDARY 0x01
#define IFA_F_TEMPORARY IFA_F_SECONDARY
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index bfe3ec7ecb14..256b9f88db8f 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5022,8 +5022,13 @@ static int inet6_dump_addr(struct sk_buff *skb,
struct netlink_callback *cb,
s_idx = idx = cb->args[1];
s_ip_idx = ip_idx = cb->args[2];
- if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
- ifa_ipv6_policy, NULL) >= 0) {
+ if (nlmsg_len(cb->nlh) >= sizeof(struct ifaddrmsg) &&
+ ((struct ifaddrmsg *) nlmsg_data(cb->nlh))->ifa_flags &
IFA_F_PROPER_HEADER) {
+
+ if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb,
IFA_MAX,
+ ifa_ipv6_policy, NULL) >= 0) {
+ ...
+
if (tb[IFA_TARGET_NETNSID]) {
netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
For ifaddrmsg ifa_flags aligns with ifi_type which is set by kernel side
so this should be ok.
^ permalink raw reply related
* Re: Marvell phy errata origins?
From: Andrew Lunn @ 2018-09-25 15:34 UTC (permalink / raw)
To: Daniel Walker
Cc: Florian Fainelli, Andy Fleming, Harini Katakam,
netdev@vger.kernel.org, HEMANT RAMDASI,
Julius Hemanth Pitti -X (jpitti - MONTA VISTA SOFTWARE INC at Cisco),
GokulChand Casheekar (gcasheek)
In-Reply-To: <84bfdc12-800c-2f63-3eff-416cf560e18c@cisco.com>
> I've added Gokul who reported the issue to me. Is it possible that Harini
> and Cisco have different m88e1111 phys? Maybe there's an issue with how they
> are hooked up ?
Hi Daniel
The lower 4 bits of the ID registers normally indicate the revision of
the PHY. It might be worth checking if everybody has the same
revision, or the problems are limited to just one revision.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v1 1/5] net: allow binding socket in a VRF when there's an unbound socket
From: Mike Manning @ 2018-09-25 15:26 UTC (permalink / raw)
To: David Ahern, netdev; +Cc: Robert Shearman
In-Reply-To: <bfc0ba04-52e1-b742-090a-ada53bb9bd06@gmail.com>
On 24/09/2018 23:44, David Ahern wrote:
> On 9/24/18 10:13 AM, Mike Manning wrote:
>> From: Robert Shearman <rshearma@vyatta.att-mail.com>
>>
>> There is no easy way currently for applications that want to receive
>> packets in the default VRF to be isolated from packets arriving in
>> VRFs, which makes using VRF-unaware applications in a VRF-aware system
>> a potential security risk.
>
> That comment is not correct.
>
> The point of the l3mdev sysctl's is to prohibit this case. Setting
> net.ipv4.{tcp,udp}_l3mdev_accept=0 means that a packet arriving on an
> interface enslaved to a VRF can not be received by a global socket.
Hi David, thanks for reviewing this. The converse does not hold though,
i.e. there is no guarantee that the unbound socket will be selected for
packets when not in a VRF, if there is an unbound socket and a socket
bound to a VRF. Also, such packets should not be handled by the socket
in the VRF if there is no unbound socket. We also had an issue with raw
socket lookup device bind matching. I can break this particular patch
into smaller patches and provide more detail, would this help? I will
also update/break up the other patches according to your comments.
>
> Setting the l3mdev to 1 allows the default socket to work across VRFs.
> If that is not what you want for a given app or a given VRF, then one
> option is to add netfilter rules on the VRF device to prohibit it. I
> just verified this works for both tcp and udp.
Netfilter is per application and so does not scale. I have not checked
if it is suitable for packet handling on raw sockets.
>
> Further, overlapping binds are allowed using SO_REUSEPORT meaning I can
> have a server running in the default vrf bound to a port AND a server
> running bound to a specific vrf and the same port:
>
> udp UNCONN 0 0 *%red:12345 *:*
> users:(("vrf-test",pid=1376,fd=3))
> udp UNCONN 0 0 *:12345 *:*
> users:(("vrf-test",pid=1375,fd=3))
>
> tcp LISTEN 0 1 *%red:12345 *:*
> users:(("vrf-test",pid=1356,fd=3))
> tcp LISTEN 0 1 *:12345 *:*
> users:(("vrf-test",pid=1352,fd=3))
>
> For packets arriving on an interface enslaved to a VRF the socket lookup
> will pick the VRF server over the global one.
Agreed, but the converse is not guaranteed to hold i.e. packets that are
not in a VRF may be handled by a socket bound to a VRF.
We do use SO_REUSEPORT for our own applications so as to run instances
in the default and other VRFs, but still require these patches (or
similar) due to how packets are handled when there is an unbound socket
and sockets bound to different VRFs.
>
> --
>
> With this patch set I am seeing a number of tests failing -- socket
> connections working when they should not or not working when they
> should. I only skimmed the results. I am guessing this patch is the
> reason, but that is just a guess.
>
> You need to make sure all permutations of:
> 1. net.ipv4.{tcp,udp}_l3mdev_accept={0,1},
> 2. connection in the default VRF and in a VRF,
> 3. locally originated and remote traffic,
> 4. ipv4 and ipv6
>
We are using raw, datagram and stream sockets for ipv4 & ipv6, require
connectivity for local and remote addresses where appropriate and need
route leaking between VRFs when configured, we are unaware of any
outstanding bugs. Is there some way that I can run/analyze the tests
that are failing for you?
Also cf patch 2/5 note that ping to link-local addresses is handled
consistently with that to global addresses in a VRF, so this now
succeeds if ping is done in the VRF, i.e. 'sudo ip vrf exec <vrf> ping
<ll> -I <intf>
> continue to work as expected meaning packets flow when they should and
> fail with the right error when they should not. I believe the UDP cases
> were the main ones failing.
>
> Given the test failures, I did not look at the code changes in the patch.
>
^ permalink raw reply
* Re: Marvell phy errata origins?
From: Daniel Walker @ 2018-09-25 15:16 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Andy Fleming, Harini Katakam,
netdev@vger.kernel.org, HEMANT RAMDASI,
Julius Hemanth Pitti -X (jpitti - MONTA VISTA SOFTWARE INC at Cisco),
GokulChand Casheekar (gcasheek)
In-Reply-To: <20170418140450.GB13724@lunn.ch>
On 04/18/2017 07:04 AM, Andrew Lunn wrote:
> On Tue, Apr 18, 2017 at 06:16:33AM -0700, Daniel Walker wrote:
>>
>> Hi,
>>
>> Cisco is using a Marvell 88E1112 phy. It seems to be fairly similar
>> to the 88E1111 which Harini added a fix for.
>
> Hi Daniel
>
> If you look at Marvell reference drive, DSDT, they are actually quite
> different. Different virtual cable tester, different downshift
> configuration, different packet generator, different loopback. I would
> say they are different generations of PHY.
>
>> In Harini's commit
>> message for ,
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/phy/marvell.c?id=3ec0a0f10ceb
>>
>> "This function has a sequence accessing Page 5 and Register 31, both
>> of which are not defined or reserved for this PHY"
I hope this this thread isn't too old to bring back to life. So it seems
that Harini found that m88e1111 did not need this errata, and Cisco
previously found that Harini's patch fixed m88e1112, we included it
internally for that reason
Now I'm getting reports that this errata fixes issues we're seeing on
m88e1111. We see an interrupt storm without the errata, despite the
errata not being defined in the datasheet.
I would just send a patch adding the errata, but because Harini removed
it I guess we really need to suss out what's going on.
I've added Gokul who reported the issue to me. Is it possible that
Harini and Cisco have different m88e1111 phys? Maybe there's an issue
with how they are hooked up ?
Daniel
^ permalink raw reply
* [PATCH v2 bpf-next 06/10] bpftool: add support for PERCPU_CGROUP_STORAGE maps
From: Roman Gushchin @ 2018-09-25 15:21 UTC (permalink / raw)
To: netdev
Cc: Song Liu, linux-kernel, kernel-team, Roman Gushchin,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180925152114.13537-1-guro@fb.com>
This commit adds support for BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE
map type.
Signed-off-by: Roman Gushchin <guro@fb.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
---
tools/bpf/bpftool/map.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index af8ad32fa6e9..cb8177593a9a 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -71,13 +71,15 @@ static const char * const map_type_name[] = {
[BPF_MAP_TYPE_XSKMAP] = "xskmap",
[BPF_MAP_TYPE_SOCKHASH] = "sockhash",
[BPF_MAP_TYPE_CGROUP_STORAGE] = "cgroup_storage",
+ [BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE] = "percpu_cgroup_storage",
};
static bool map_is_per_cpu(__u32 type)
{
return type == BPF_MAP_TYPE_PERCPU_HASH ||
type == BPF_MAP_TYPE_PERCPU_ARRAY ||
- type == BPF_MAP_TYPE_LRU_PERCPU_HASH;
+ type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
+ type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE;
}
static bool map_is_map_of_maps(__u32 type)
--
2.17.1
^ permalink raw reply related
* [PATCH v2 bpf-next 09/10] samples/bpf: extend test_cgrp2_attach2 test to use per-cpu cgroup storage
From: Roman Gushchin @ 2018-09-25 15:21 UTC (permalink / raw)
To: netdev
Cc: Song Liu, linux-kernel, kernel-team, Roman Gushchin,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180925152114.13537-1-guro@fb.com>
This commit extends the test_cgrp2_attach2 test to cover per-cpu
cgroup storage. Bpf program will use shared and per-cpu cgroup
storages simultaneously, so a better coverage of corresponding
core code will be achieved.
Expected output:
$ ./test_cgrp2_attach2
Attached DROP prog. This ping in cgroup /foo should fail...
ping: sendmsg: Operation not permitted
Attached DROP prog. This ping in cgroup /foo/bar should fail...
ping: sendmsg: Operation not permitted
Attached PASS prog. This ping in cgroup /foo/bar should pass...
Detached PASS from /foo/bar while DROP is attached to /foo.
This ping in cgroup /foo/bar should fail...
ping: sendmsg: Operation not permitted
Attached PASS from /foo/bar and detached DROP from /foo.
This ping in cgroup /foo/bar should pass...
### override:PASS
### multi:PASS
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
---
samples/bpf/test_cgrp2_attach2.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/samples/bpf/test_cgrp2_attach2.c b/samples/bpf/test_cgrp2_attach2.c
index 180f9d813bca..d7b68ef5ba79 100644
--- a/samples/bpf/test_cgrp2_attach2.c
+++ b/samples/bpf/test_cgrp2_attach2.c
@@ -209,7 +209,7 @@ static int map_fd = -1;
static int prog_load_cnt(int verdict, int val)
{
- int cgroup_storage_fd;
+ int cgroup_storage_fd, percpu_cgroup_storage_fd;
if (map_fd < 0)
map_fd = bpf_create_map(BPF_MAP_TYPE_ARRAY, 4, 8, 1, 0);
@@ -225,6 +225,14 @@ static int prog_load_cnt(int verdict, int val)
return -1;
}
+ percpu_cgroup_storage_fd = bpf_create_map(
+ BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
+ sizeof(struct bpf_cgroup_storage_key), 8, 0, 0);
+ if (percpu_cgroup_storage_fd < 0) {
+ printf("failed to create map '%s'\n", strerror(errno));
+ return -1;
+ }
+
struct bpf_insn prog[] = {
BPF_MOV32_IMM(BPF_REG_0, 0),
BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -4), /* *(u32 *)(fp - 4) = r0 */
@@ -235,11 +243,20 @@ static int prog_load_cnt(int verdict, int val)
BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
BPF_MOV64_IMM(BPF_REG_1, val), /* r1 = 1 */
BPF_RAW_INSN(BPF_STX | BPF_XADD | BPF_DW, BPF_REG_0, BPF_REG_1, 0, 0), /* xadd r0 += r1 */
+
BPF_LD_MAP_FD(BPF_REG_1, cgroup_storage_fd),
BPF_MOV64_IMM(BPF_REG_2, 0),
BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_get_local_storage),
BPF_MOV64_IMM(BPF_REG_1, val),
BPF_RAW_INSN(BPF_STX | BPF_XADD | BPF_W, BPF_REG_0, BPF_REG_1, 0, 0),
+
+ BPF_LD_MAP_FD(BPF_REG_1, percpu_cgroup_storage_fd),
+ BPF_MOV64_IMM(BPF_REG_2, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_get_local_storage),
+ BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_0, 0),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 0x1),
+ BPF_STX_MEM(BPF_W, BPF_REG_0, BPF_REG_3, 0),
+
BPF_MOV64_IMM(BPF_REG_0, verdict), /* r0 = verdict */
BPF_EXIT_INSN(),
};
--
2.17.1
^ permalink raw reply related
* [PATCH v2 bpf-next 04/10] bpf: don't allow create maps of per-cpu cgroup local storages
From: Roman Gushchin @ 2018-09-25 15:21 UTC (permalink / raw)
To: netdev
Cc: Song Liu, linux-kernel, kernel-team, Roman Gushchin,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180925152114.13537-1-guro@fb.com>
Explicitly forbid creating map of per-cpu cgroup local storages.
This behavior matches the behavior of shared cgroup storages.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
---
kernel/bpf/map_in_map.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/map_in_map.c b/kernel/bpf/map_in_map.c
index 3bfbf4464416..99d243e1ad6e 100644
--- a/kernel/bpf/map_in_map.c
+++ b/kernel/bpf/map_in_map.c
@@ -24,7 +24,8 @@ struct bpf_map *bpf_map_meta_alloc(int inner_map_ufd)
* in the verifier is not enough.
*/
if (inner_map->map_type == BPF_MAP_TYPE_PROG_ARRAY ||
- inner_map->map_type == BPF_MAP_TYPE_CGROUP_STORAGE) {
+ inner_map->map_type == BPF_MAP_TYPE_CGROUP_STORAGE ||
+ inner_map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE) {
fdput(f);
return ERR_PTR(-ENOTSUPP);
}
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox