* Re: [PATCH net-next v2 1/4] enetc: Introduce basic PF and VF ENETC ethernet drivers
From: Andrew Lunn @ 2018-11-22 18:05 UTC (permalink / raw)
To: Claudiu Manoil
Cc: David Miller, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Alexandru Marginean,
Catalin Horghidan
In-Reply-To: <VI1PR04MB48803D4A3D8D331046CFC63196DB0@VI1PR04MB4880.eurprd04.prod.outlook.com>
On Thu, Nov 22, 2018 at 01:06:01PM +0000, Claudiu Manoil wrote:
> >-----Original Message-----
> >From: David Miller <davem@davemloft.net>
> >Sent: Thursday, November 22, 2018 2:21 AM
> >To: Claudiu Manoil <claudiu.manoil@nxp.com>
> >Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Alexandru
> >Marginean <alexandru.marginean@nxp.com>; Catalin Horghidan
> ><catalin.horghidan@nxp.com>
> >Subject: Re: [PATCH net-next v2 1/4] enetc: Introduce basic PF and VF ENETC
> >ethernet drivers
> >
> >From: Claudiu Manoil <claudiu.manoil@nxp.com>
> >Date: Tue, 20 Nov 2018 20:15:31 +0200
> >
> >> diff --git a/drivers/net/ethernet/freescale/Makefile
> >b/drivers/net/ethernet/freescale/Makefile
> >> index 3b4ff08..20e5c2f9 100644
> >> --- a/drivers/net/ethernet/freescale/Makefile
> >> +++ b/drivers/net/ethernet/freescale/Makefile
> >> @@ -23,3 +23,4 @@ obj-$(CONFIG_FSL_FMAN) += fman/
> >> obj-$(CONFIG_FSL_DPAA_ETH) += dpaa/
> >>
> >> obj-$(CONFIG_FSL_DPAA2_ETH) += dpaa2/
> >> +obj-$(CONFIG_NET_VENDOR_FREESCALE) += enetc/
> >
> >The driver enable Kconfig option should guard traversing into the
> >driver subdirectory, not the vendor enable Kconfig knob.
>
> The enetc/ dir contains 2 drivers, that share a lot of common code.
> Would you agree if I change the vendor enable with the two configs
> (one for each driver) as below?
>
> -obj-$(CONFIG_NET_VENDOR_FREESCALE) += enetc/
> +obj-$(CONFIG_FSL_ENETC) += enetc/
> +obj-$(CONFIG_FSL_ENETC_VF) += enetc/
CONFIG_NET_VENDOR_FREESCALE should hide/show all Freescale drivers.
Once you show all Freescale drivers, there should be an option to
enable each individual driver.
Does the ENETC_VF driver depend on the ENETC driver? If so, when
CONFIG_FSL_ENETC is enabled, the CONFIG_FSL_ENETC_VF should be
unhidden.
Andrew
^ permalink raw reply
* Re: [PATCH perf,bpf 0/5] reveal invisible bpf programs
From: Song Liu @ 2018-11-22 18:13 UTC (permalink / raw)
To: Peter Zijlstra
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
ast@kernel.org, daniel@iogearbox.net, acme@kernel.org,
Kernel Team
In-Reply-To: <20181122093219.GK2131@hirez.programming.kicks-ass.net>
Hi Peter,
> On Nov 22, 2018, at 1:32 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Nov 21, 2018 at 11:54:57AM -0800, Song Liu wrote:
>> Changes RFC -> PATCH v1:
>>
>> 1. In perf-record, poll vip events in a separate thread;
>> 2. Add tag to bpf prog name;
>> 3. Small refactorings.
>>
>> Original cover letter (with minor revisions):
>>
>> This is to follow up Alexei's early effort to show bpf programs
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_netdev_msg524232.html&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=dR8692q0_uaizy0jkrBJQM5k2hfm4CiFxYT8KaysFrg&m=3--n8O2EZfFY2WyGCKt0u4zd73778zD7xmoNHi9tMCU&s=3DY93pysLN-m1tgYmd7YAyQGNSq6KpYKucIJcB3nofc&e=
>>
>> In this version, PERF_RECORD_BPF_EVENT is introduced to send real time BPF
>> load/unload events to user space. In user space, perf-record is modified
>> to listen to these events (through a dedicated ring buffer) and generate
>> detailed information about the program (struct bpf_prog_info_event). Then,
>> perf-report translates these events into proper symbols.
>>
>> With this set, perf-report will show bpf program as:
>>
>> 18.49% 0.16% test [kernel.vmlinux] [k] ksys_write
>> 18.01% 0.47% test [kernel.vmlinux] [k] vfs_write
>> 17.02% 0.40% test bpf_prog [k] bpf_prog_07367f7ba80df72b_
>> 16.97% 0.10% test [kernel.vmlinux] [k] __vfs_write
>> 16.86% 0.12% test [kernel.vmlinux] [k] comm_write
>> 16.67% 0.39% test [kernel.vmlinux] [k] bpf_probe_read
>>
>> Note that, the program name is still work in progress, it will be cleaner
>> with function types in BTF.
>>
>> Please share your comments on this.
>
> So I see:
>
> kernel/bpf/core.c:void bpf_prog_kallsyms_add(struct bpf_prog *fp)
>
> which should already provide basic symbol information for extant eBPF
> programs, right?
Right, if the BPF program is still loaded when perf-report runs, symbols
are available.
> And (AFAIK) perf uses /proc/kcore for annotate on the current running
> kernel (if not, it really should, given alternatives, jump_labels and
> all other other self-modifying code).
>
> So this fancy new stuff is only for the case where your profile spans
> eBPF load/unload events (which should be relatively rare in the normal
> case, right), or when you want source annotated asm output (I normally
> don't bother with that).
This patch set adds two pieces of information:
1. At the beginning of perf-record, save info of existing BPF programs;
2. Gather information of BPF programs load/unload during perf-record.
(1) is all in user space. It is necessary to show symbols of BPF program
that are unloaded _after_ perf-record. (2) needs PERF_RECORD_BPF_EVENT
from the ring buffer. It covers BPF program loaded during perf-record
(perf record -- bpf_test).
> That is; I would really like this fancy stuff to be an optional extra
> that is typically not needed.
>
> Does that make sense?
(1) above is always enabled with this set. I added option no-bpf-events
to disable (2). I guess you prefer the (2) is disabled by default, and
enabled with an option?
Thanks,
Song
^ permalink raw reply
* Re: Issue with RTL8111 NIC after upgrade to kernel 4.19
From: Heiner Kallweit @ 2018-11-22 18:17 UTC (permalink / raw)
To: Marc Dionne
Cc: andrew, norbert.jurkeit, nic_swsd, Florian Fainelli, David Miller,
netdev, Linux Kernel Mailing List, michael.wiktowy, jcline
In-Reply-To: <CAB9dFdsq7EBNm1Y1nK2tqOvg5ikyTm4ZGTQddsNAp1BaA0J_XQ@mail.gmail.com>
On 22.11.2018 00:13, Marc Dionne wrote:
> On Wed, Nov 21, 2018 at 6:28 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> On 21.11.2018 22:53, Marc Dionne wrote:
>>> On Wed, Nov 21, 2018 at 4:52 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>
>>>> On 21.11.2018 21:49, Heiner Kallweit wrote:
>>>>> On 21.11.2018 21:32, Heiner Kallweit wrote:
>>>>>> On 21.11.2018 21:20, Andrew Lunn wrote:
>>>>>>>> request_module() is supposed to be synchronous, however after some
>>>>>>>> reading this may not be 100% guaranteed. Maybe the module init
>>>>>>>> function on some systems isn't finished yet when request_module()
>>>>>>>> returns. As a result the genphy driver may be used instead of
>>>>>>>> the PHY version-specific driver.
>>>>>>>
>>>>>>> Hi Heiner
>>>>>>>
>>>>>>> That would be true for all PHYs i think. We would of noticed this
>>>>>>> problem with other systems using other PHY drivers.
>>>>>>>
>>>>>>> Andrew
>>>>>>>
>>>>>> It could be a timing issue affecting certain systems only. At least
>>>>>> for now I don't have a good explanation why loading the module via
>>>>>> request_module() and loading it upfront manually makes a difference.
>>>>>>
>>>>>> One affected user just reported the PHY to be a RTL8211B. This is
>>>>>> what I expected, because this PHY crashes when writing to the MMD
>>>>>> registers (the MMD registers are used otherwise by this PHY).
>>>>>> See also commit 0231b1a074c6 ("net: phy: realtek: Use the dummy
>>>>>> stubs for MMD register access for rtl8211b").
>>>>>>
>>>>>> Let's see whether the other affected systems use the same PHY
>>>>>> version.
>>>>>>
>>>>> Next report is also about a RTL8211B and as I assumed:
>>>>> - W/o manually loading the realtek module the genphy driver is used
>>>>> and network fails.
>>>>> - W/ manually loading the realtek module the proper RTL8211B PHY
>>>>> driver is used and network works.
>>>>>
>>>>> So it seems that even after request_module() the PHY driver isn't
>>>>> yet available when device and driver are matched.
>>>>>
>>>>> If further reports support this (pre-)analysis, then indeed it
>>>>> seems to be a timing issue and a proper fix most likely is
>>>>> difficult. As a workaround I could imagine to add a delay loop
>>>>> after request_module() checking for a Realtek PHY driver via
>>>>> driver_find(). When adding one small delay after this we should
>>>>> be sufficiently sure that all Realtek PHY drivers are registered.
>>>>>
>>>> Uups, no. We talk about phylib here, not about the r8169 driver.
>>>> So we need a different solution.
>>>>
>>>>>> Heiner
>>>
>>> Thanks for the explanation, better than my crude attempt at
>>> understanding what was going on.
>>>
>>> If you have any proposed fixes or diagnostic patches based on current
>>> mainline I can quickly compile and test them here on an affected
>>> system. It doesn't fail consistently for me (as others have
>>> reported), but that could be because it depends on the timing.
>>>
>>
>> Thanks for the offer. Can you try the following diagnostic patch
>> and check whether it helps?
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 55202a0ac..84f417f8b 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -607,6 +607,8 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
>> */
>> request_module(MDIO_MODULE_PREFIX MDIO_ID_FMT, MDIO_ID_ARGS(phy_id));
>>
>> + msleep(1000);
>> +
>> device_initialize(&mdiodev->dev);
>>
>> return dev;
>
> I was not able to get it to fail with that extra delay; hard to be
> 100% sure but the network starts even when switching over from the
> distro kernel after a boot where network failed to start.
>
Thanks a lot for testing. Could you please test also the following
as an alternative to the delay?
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 55202a0ac..aeccb2323 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2254,6 +2254,7 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner)
new_driver->mdiodrv.driver.probe = phy_probe;
new_driver->mdiodrv.driver.remove = phy_remove;
new_driver->mdiodrv.driver.owner = owner;
+ new_driver->mdiodrv.driver.probe_type = PROBE_FORCE_SYNCHRONOUS;
retval = driver_register(&new_driver->mdiodrv.driver);
if (retval) {
> There's a side issue that network startup is taking a full minute
> longer than it should, but that's possibly unrelated.
>
> Marc
>
Thanks, Heiner
^ permalink raw reply related
* Re: [PATCH v3 13/13] nvme-tcp: add NVMe over TCP host driver
From: Christoph Hellwig @ 2018-11-22 8:02 UTC (permalink / raw)
To: Sagi Grimberg
Cc: linux-nvme, linux-block, netdev, Christoph Hellwig, Keith Busch,
David S. Miller
In-Reply-To: <20181122015615.15763-14-sagi@grimberg.me>
A few reandom nitpicks:
> +static int nvme_tcp_verify_hdgst(struct nvme_tcp_queue *queue,
> + void *pdu, size_t pdu_len)
Please use two tabs for indenting prototype continuations
> + len = le32_to_cpu(hdr->plen) - hdr->hlen -
> + ((hdr->flags & NVME_TCP_F_HDGST) ? nvme_tcp_hdgst_len(queue) : 0);
Overly long line. But it would be much cleaner with a local digest_len
variable anyway.
> +static enum nvme_tcp_recv_state nvme_tcp_recv_state(struct nvme_tcp_queue *queue)
> +{
> + return (queue->pdu_remaining) ? NVME_TCP_RECV_PDU :
> + (queue->ddgst_remaining) ? NVME_TCP_RECV_DDGST :
> + NVME_TCP_RECV_DATA;
> +}
This just seems to be used in a single switch statement. Why the detour
theough the state enum?
> +{
> + struct request *rq;
> + struct nvme_tcp_request *req;
> +
> + rq = blk_mq_tag_to_rq(nvme_tcp_tagset(queue), cqe->command_id);
> + if (!rq) {
> + dev_err(queue->ctrl->ctrl.device,
> + "queue %d tag 0x%x not found\n",
> + nvme_tcp_queue_id(queue), cqe->command_id);
> + nvme_tcp_error_recovery(&queue->ctrl->ctrl);
> + return -EINVAL;
> + }
> + req = blk_mq_rq_to_pdu(rq);
> +
> + nvme_end_request(rq, cqe->status, cqe->result);
req seems unused here.
> + nvme_tcp_queue_id(queue), pdu->command_id);
> + return -ENOENT;
> + }
> + req = blk_mq_rq_to_pdu(rq);
> +
> + if (!blk_rq_payload_bytes(rq)) {
> + dev_err(queue->ctrl->ctrl.device,
> + "queue %d tag %#x unexpected data\n",
> + nvme_tcp_queue_id(queue), rq->tag);
> + return -EIO;
> + }
> +
> + queue->data_remaining = le32_to_cpu(pdu->data_length);
> + /* No support for out-of-order */
> + WARN_ON(le32_to_cpu(pdu->data_offset));
> +
> + return 0;
And here as well.
Also can we just WARN_ON on the offset?
> + ret = skb_copy_bits(skb, *offset,
> + &queue->pdu[queue->pdu_offset], rcv_len);
More of this can go on th first line.
> + if (unlikely(ret))
> + return ret;
> +
> + queue->pdu_remaining -= rcv_len;
> + queue->pdu_offset += rcv_len;
> + *offset += rcv_len;
> + *len -= rcv_len;
> + if (queue->pdu_remaining)
> + return 0;
> +
> + hdr = (void *)queue->pdu;
hdr is a struct nvme_tcp_hdr *, please use the right cast if we have
to cast - but then again queue->pdu probably should be a void pointer
so that we can use it everywhere without casts.
> +static void nvme_tcp_init_recv_iter(struct nvme_tcp_request *req)
> +{
> + struct bio *bio = req->curr_bio;
> + struct bio_vec *vec = __bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter);
> + unsigned int nsegs = bio_segments(bio);
> +
> + iov_iter_bvec(&req->iter, READ, vec, nsegs,
> + bio->bi_iter.bi_size);
> + req->iter.iov_offset = bio->bi_iter.bi_bvec_done;
This code seems largely identical to that in nvme_tcp_init_send_iter
except for passing READ vs WRITE. Please use a common helper.
> + /*
> + * FIXME: This assumes that data comes in-order,
> + * need to handle the out-of-order case.
> + */
That sounds like something we should really address before merging.
> + read_lock(&sk->sk_callback_lock);
> + queue = sk->sk_user_data;
> + if (unlikely(!queue || !queue->rd_enabled))
> + goto done;
> +
> + queue_work_on(queue->io_cpu, nvme_tcp_wq, &queue->io_work);
> +done:
> + read_unlock(&sk->sk_callback_lock);
Don't we need a rcu_dereference_sk_user_data here?
Also why not:
queue = rcu_dereference_sk_user_data(sk);
if (likely(queue && queue->rd_enabled))
queue_work_on(queue->io_cpu, nvme_tcp_wq, &queue->io_work);
read_unlock(&sk->sk_callback_lock);
> +static void nvme_tcp_write_space(struct sock *sk)
> +{
> + struct nvme_tcp_queue *queue;
> +
> + read_lock_bh(&sk->sk_callback_lock);
> + queue = sk->sk_user_data;
> +
> + if (!queue)
> + goto done;
> +
> + if (sk_stream_is_writeable(sk)) {
> + clear_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
> + queue_work_on(queue->io_cpu, nvme_tcp_wq, &queue->io_work);
> + }
> +done:
> + read_unlock_bh(&sk->sk_callback_lock);
Same here:
queue = rcu_dereference_sk_user_data(sk);
if (queue && sk_stream_is_writeable(sk)) {
clear_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
queue_work_on(queue->io_cpu, nvme_tcp_wq, &queue->io_work);
}
read_unlock(&sk->sk_callback_lock);
(there are a few more places where rcu_dereference_sk_user_data should
be used, skipping them now).
> +static inline void nvme_tcp_done_send_req(struct nvme_tcp_queue *queue)
> +{
> + queue->request = NULL;
> +}
> +
> +static void nvme_tcp_fail_request(struct nvme_tcp_request *req)
> +{
> + union nvme_result res = {};
> +
> + nvme_end_request(blk_mq_rq_from_pdu(req),
> + NVME_SC_DATA_XFER_ERROR, res);
This looks like odd formatting, needs one more tab. But
NVME_SC_DATA_XFER_ERROR is also generally a status that should be
returned from the nvme controller, not made up on the host.
> + if (queue->data_digest)
> + nvme_tcp_ddgst_update(queue->snd_hash, page, offset, ret);
Overly long line, please stick to 80 characters.
> + if (req->state == NVME_TCP_SEND_CMD_PDU) {
> + ret = nvme_tcp_try_send_cmd_pdu(req);
> + if (ret <= 0)
> + goto done;
> + if (!nvme_tcp_has_inline_data(req))
> + return ret;
> + }
> +
> + if (req->state == NVME_TCP_SEND_H2C_PDU) {
> + ret = nvme_tcp_try_send_data_pdu(req);
> + if (ret <= 0)
> + goto done;
> + }
> +
> + if (req->state == NVME_TCP_SEND_DATA) {
> + ret = nvme_tcp_try_send_data(req);
> + if (ret <= 0)
> + goto done;
> + }
> +
> + if (req->state == NVME_TCP_SEND_DDGST)
> + ret = nvme_tcp_try_send_ddgst(req);
Use a switch statement here?
> +static void nvme_tcp_free_tagset(struct nvme_ctrl *nctrl,
> + struct blk_mq_tag_set *set)
> +{
> + blk_mq_free_tag_set(set);
> +}
Please drop this wrapper.
> +static struct blk_mq_tag_set *nvme_tcp_alloc_tagset(struct nvme_ctrl *nctrl,
> + bool admin)
> +{
This function does two entirely different things based on the admin
paramter.
> +static void nvme_tcp_stop_admin_queue(struct nvme_ctrl *ctrl)
> +{
> + nvme_tcp_stop_queue(ctrl, 0);
> +}
This wrapper seems a bit pointless.
> +static int nvme_tcp_start_admin_queue(struct nvme_ctrl *ctrl)
> +{
> + return nvme_tcp_start_queue(ctrl, 0);
> +}
Same here.
> +int nvme_tcp_configure_admin_queue(struct nvme_ctrl *ctrl, bool new)
Shouldn't this (or anything in this file for that matter) be static?
> + if (ctrl->queue_count > 1) {
> + nvme_stop_queues(ctrl);
> + nvme_tcp_stop_io_queues(ctrl);
> + blk_mq_tagset_busy_iter(ctrl->tagset, nvme_cancel_request, ctrl);
> + if (remove)
> + nvme_start_queues(ctrl);
> + nvme_tcp_destroy_io_queues(ctrl, remove);
> + }
Overly long line above. Could be easily solved with an early return..
> +static void nvme_tcp_delete_ctrl(struct nvme_ctrl *ctrl)
> +{
> + nvme_tcp_teardown_ctrl(ctrl, true);
> +}
Pointless wrapper.
> +static void nvme_tcp_set_sg_null(struct nvme_command *c)
> +{
> + struct nvme_sgl_desc *sg = &c->common.dptr.sgl;
> +
> + sg->addr = 0;
> + sg->length = 0;
> + sg->type = (NVME_TRANSPORT_SGL_DATA_DESC << 4) |
> + NVME_SGL_FMT_TRANSPORT_A;
> +}
> +
> +static void nvme_tcp_set_sg_host_data(struct nvme_tcp_request *req,
> + struct nvme_command *c)
> +{
> + struct nvme_sgl_desc *sg = &c->common.dptr.sgl;
> +
> + sg->addr = 0;
> + sg->length = cpu_to_le32(req->data_len);
> + sg->type = (NVME_TRANSPORT_SGL_DATA_DESC << 4) |
> + NVME_SGL_FMT_TRANSPORT_A;
> +}
Do we really need nvme_tcp_set_sg_null? Any command it is called
on should have a request with a 0 length, so it could use
nvme_tcp_set_sg_host_data.
> +static enum blk_eh_timer_return
> +nvme_tcp_timeout(struct request *rq, bool reserved)
> +{
> + struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);
> + struct nvme_tcp_ctrl *ctrl = req->queue->ctrl;
> + struct nvme_tcp_cmd_pdu *pdu = req->pdu;
> +
> + dev_dbg(ctrl->ctrl.device,
> + "queue %d: timeout request %#x type %d\n",
> + nvme_tcp_queue_id(req->queue), rq->tag,
> + pdu->hdr.type);
> +
> + if (ctrl->ctrl.state != NVME_CTRL_LIVE) {
> + union nvme_result res = {};
> +
> + nvme_req(rq)->flags |= NVME_REQ_CANCELLED;
> + nvme_end_request(rq, NVME_SC_ABORT_REQ, res);
> + return BLK_EH_DONE;
This looks odd. It's not really the timeout handlers job to
call nvme_end_request here.
> + if (rq_data_dir(rq) == WRITE) {
> + req->curr_bio = rq->bio;
> + if (req->data_len <= nvme_tcp_inline_data_size(queue))
> + req->pdu_len = req->data_len;
> + } else {
> + req->curr_bio = rq->bio;
> + if (req->curr_bio)
> + nvme_tcp_init_recv_iter(req);
> + }
The curr_bio setup is duplicated in both branches.
^ permalink raw reply
* Re: [PATCH net-next 2/4] netns: add support of NETNSA_TARGET_NSID
From: Nicolas Dichtel @ 2018-11-22 8:06 UTC (permalink / raw)
To: David Ahern, davem; +Cc: netdev
In-Reply-To: <ce2e9e24-73ba-9780-ac38-124d6cf95a1b@gmail.com>
Le 21/11/2018 à 22:07, David Ahern a écrit :
> On 11/21/18 1:58 PM, Nicolas Dichtel wrote:
>> The target-nsid is not stored in net_cb (not needed). ref_net is set only if
>> tgt_net comes from TARGET_NSID. I can add a comment.
>
> ref_net is added by this patch and it is only used (unless I missed
> something) to know if the put_net is needed. ie/, drop ref_net in place
> of nsid
It is used by a following patch (4/4) to get NETNSA_CURRENT_NSID.
^ permalink raw reply
* [PATCH net] team: no need to do team_notify_peers or team_mcast_rejoin when disabling port
From: Hangbin Liu @ 2018-11-22 8:15 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Jiri Pirko, Xin Long, Hangbin Liu
team_notify_peers() will send ARP and NA to notify peers. team_mcast_rejoin()
will send multicast join group message to notify peers. We should do this when
enabling/changed to a new port. But it doesn't make sense to do it when a port
is disabled.
On the other hand, when we set mcast_rejoin_count to 2, and do a failover,
team_port_disable() will increase mcast_rejoin.count_pending to 2 and then
team_port_enable() will increase mcast_rejoin.count_pending to 4. We will send
4 mcast rejoin messages at latest, which will make user confused. The same
with notify_peers.count.
Fix it by deleting team_notify_peers() and team_mcast_rejoin() in
team_port_disable().
Reported-by: Liang Li <liali@redhat.com>
Fixes: fc423ff00df3a ("team: add peer notification")
Fixes: 492b200efdd20 ("team: add support for sending multicast rejoins")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
drivers/net/team/team.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index db633ae..364f514 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -985,8 +985,6 @@ static void team_port_disable(struct team *team,
team->en_port_count--;
team_queue_override_port_del(team, port);
team_adjust_ops(team);
- team_notify_peers(team);
- team_mcast_rejoin(team);
team_lower_state_changed(port);
}
--
2.5.5
^ permalink raw reply related
* Re: Issue with RTL8111 NIC after upgrade to kernel 4.19
From: Andrew Lunn @ 2018-11-22 18:57 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Marc Dionne, norbert.jurkeit, nic_swsd, Florian Fainelli,
David Miller, netdev, Linux Kernel Mailing List, michael.wiktowy,
jcline
In-Reply-To: <d38a9ed9-b801-5622-1d3a-1b9620ed9d8d@gmail.com>
> Thanks a lot for testing. Could you please test also the following
> as an alternative to the delay?
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 55202a0ac..aeccb2323 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -2254,6 +2254,7 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner)
> new_driver->mdiodrv.driver.probe = phy_probe;
> new_driver->mdiodrv.driver.remove = phy_remove;
> new_driver->mdiodrv.driver.owner = owner;
> + new_driver->mdiodrv.driver.probe_type = PROBE_FORCE_SYNCHRONOUS;
>
> retval = driver_register(&new_driver->mdiodrv.driver);
> if (retval) {
Humm, maybe i don't understand the issue correctly.
When the MDIO bus is registered, we scan the bus looking for PHYs.
When we find a PHY, we call phy_device_create(). That will then
trigger the loading of the kernel module which should driver this phy.
Sometime later, the PHY driver module gets loaded and calls
phy_drivers_register() to register the list of IDs it supports. The
driver core will then call phy_bus_match() to see if the newly loaded
driver matches to a device we have created. If so, the driver will be
associated to the device.
Sometime later, the MAC tries to attach to the phy using
phy_attach_direct(). If there is no driver associated to the device,
we use the generic PHY driver.
I thought the issue was the race condition between loading the module
and the MAC attaching to it? We are getting the generic driver because
the specific driver is still loading?
If that really is the issue, i think phy_attach_direct() should try
loading the module again, doing it synchronously. Only when it fails
should the generic driver be associated to the device.
Andrew
^ permalink raw reply
* Re: consistency for statistics with XDP mode
From: Toke Høiland-Jørgensen @ 2018-11-22 8:26 UTC (permalink / raw)
To: Saeed Mahameed, pstaszewski@itcare.pl, netdev@vger.kernel.org,
dsahern@gmail.com
Cc: davem@davemloft.net, jasowang@redhat.com, brouer@redhat.com,
mst@redhat.com
In-Reply-To: <636c1a4b9010eab5d461c13c7544a1d9e9f9ff3f.camel@mellanox.com>
Saeed Mahameed <saeedm@mellanox.com> writes:
>> > I'd say it sounds reasonable to include XDP in the normal traffic
>> > counters, but having the detailed XDP-specific counters is quite
>> > useful
>> > as well... So can't we do both (for all drivers)?
>> >
>
> What are you thinking ?
> reporting XDP_DROP in interface dropped counter ?
> and XDP_TX/REDIRECT in the TX counter ?
> XDP_ABORTED in the err/drop counter ?
>
> how about having a special XDP command in the .ndo_bpf that would query
> the standardized XDP stats ?
Don't have any strong opinions on the mechanism; just pointing out that
the XDP-specific stats are useful to have separately as well :)
-Toke
^ permalink raw reply
* Re: [PATCH net] net: phy: mscc: fix deadlock in vsc85xx_default_config
From: Andrew Lunn @ 2018-11-22 19:28 UTC (permalink / raw)
To: Quentin Schulz
Cc: davem, f.fainelli, allan.nielsen, linux-kernel, netdev,
thomas.petazzoni, alexandre.belloni
In-Reply-To: <20181122131232.32032-1-quentin.schulz@bootlin.com>
On Thu, Nov 22, 2018 at 02:12:32PM +0100, Quentin Schulz wrote:
> The vsc85xx_default_config function called in the vsc85xx_config_init
> function which is used by VSC8530, VSC8531, VSC8540 and VSC8541 PHYs
> mistakenly calls phy_read and phy_write in-between phy_select_page and
> phy_restore_page.
>
> phy_select_page and phy_restore_page actually take and release the MDIO
> bus lock so __phy_write and __phy_read (which assume that you already
> have the MDIO bus lock unlike phy_write and phy_read) should be used for
> any call in between the two said functions.
>
> Let's fix this deadlock.
>
> Fixes: 6a0bfbbe20b0 ("net: phy: mscc: migrate to phy_select/restore_page functions")
>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> ---
> drivers/net/phy/mscc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
> index 62269e578718..6856fe4d1a60 100644
> --- a/drivers/net/phy/mscc.c
> +++ b/drivers/net/phy/mscc.c
> @@ -814,10 +814,10 @@ static int vsc85xx_default_config(struct phy_device *phydev)
> if (rc < 0)
> goto out_unlock;
>
> - reg_val = phy_read(phydev, MSCC_PHY_RGMII_CNTL);
> + reg_val = __phy_read(phydev, MSCC_PHY_RGMII_CNTL);
> reg_val &= ~(RGMII_RX_CLK_DELAY_MASK);
> reg_val |= (RGMII_RX_CLK_DELAY_1_1_NS << RGMII_RX_CLK_DELAY_POS);
> - phy_write(phydev, MSCC_PHY_RGMII_CNTL, reg_val);
> + __phy_write(phydev, MSCC_PHY_RGMII_CNTL, reg_val);
Hi Quentin
You appear to be only accessing a single register, read/modify/write.
I think you can use phy_modify_paged(), which will take care of all
the locking for you.
Andrew
^ permalink raw reply
* Re: Issue with RTL8111 NIC after upgrade to kernel 4.19
From: Marc Dionne @ 2018-11-22 19:29 UTC (permalink / raw)
To: hkallweit1
Cc: andrew, norbert.jurkeit, nic_swsd, Florian Fainelli, David Miller,
netdev, Linux Kernel Mailing List, michael.wiktowy, jcline
In-Reply-To: <d38a9ed9-b801-5622-1d3a-1b9620ed9d8d@gmail.com>
On Thu, Nov 22, 2018 at 2:17 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> Thanks a lot for testing. Could you please test also the following
> as an alternative to the delay?
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 55202a0ac..aeccb2323 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -2254,6 +2254,7 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner)
> new_driver->mdiodrv.driver.probe = phy_probe;
> new_driver->mdiodrv.driver.remove = phy_remove;
> new_driver->mdiodrv.driver.owner = owner;
> + new_driver->mdiodrv.driver.probe_type = PROBE_FORCE_SYNCHRONOUS;
>
> retval = driver_register(&new_driver->mdiodrv.driver);
> if (retval) {
That also gets me network reliably, switching between a kernel where
it fails (distro 4.19 kernel) and the custom kernel with the patch.
> > There's a side issue that network startup is taking a full minute
> > longer than it should, but that's possibly unrelated.
BTW that's an unrelated rng issue, for which I have a workaround.
> >
> Thanks, Heiner
Marc
^ permalink raw reply
* Re: [PATCH v3 11/13] nvmet-tcp: add NVMe over TCP target driver
From: Christoph Hellwig @ 2018-11-22 9:06 UTC (permalink / raw)
To: Sagi Grimberg
Cc: linux-nvme, linux-block, netdev, Christoph Hellwig, Keith Busch,
David S. Miller
In-Reply-To: <20181122015615.15763-12-sagi@grimberg.me>
> +enum nvmet_tcp_send_state {
> + NVMET_TCP_SEND_DATA_PDU = 0,
> + NVMET_TCP_SEND_DATA,
> + NVMET_TCP_SEND_R2T,
> + NVMET_TCP_SEND_DDGST,
> + NVMET_TCP_SEND_RESPONSE
> +};
> +
> +enum nvmet_tcp_recv_state {
> + NVMET_TCP_RECV_PDU,
> + NVMET_TCP_RECV_DATA,
> + NVMET_TCP_RECV_DDGST,
> + NVMET_TCP_RECV_ERR,
> +};
I think you can drop the explicit initialization for
NVMET_TCP_SEND_DATA_PDU.
> +struct nvmet_tcp_recv_ctx {
> +};
There are no users of this empty struct, so it can probably be
dropped..
> + void (*dr)(struct sock *);
> + void (*sc)(struct sock *);
> + void (*ws)(struct sock *);
These looks very cryptic. Can you please at least spell out the
full names as used in the networking code (data_ready, etc).
> +struct nvmet_tcp_port {
> + struct socket *sock;
> + struct work_struct accept_work;
> + struct nvmet_port *nport;
> + struct sockaddr_storage addr;
> + int last_cpu;
> + void (*dr)(struct sock *);
> +};
Same here.
> + pdu->hdr.plen =
> + cpu_to_le32(pdu->hdr.hlen + hdgst + cmd->req.transfer_len + ddgst);
Overly long line.
> +static struct nvmet_tcp_cmd *nvmet_tcp_reverse_list(struct nvmet_tcp_queue *queue, struct llist_node *node)
Way too long line.
Also this function does not reverse a list, it removes from a llist,
adds to a regular list in reverse order and increments a counter. Maybe
there is a better name? It would also seem more readable if the
llist_del_all from the caller moved in here.
> +{
> + struct nvmet_tcp_cmd *cmd;
> +
> + while (node) {
> + struct nvmet_tcp_cmd *cmd = container_of(node, struct nvmet_tcp_cmd, lentry);
> +
Also shouldn't this use llist_entry instead of container_of to document
the intent?
> + list_add(&cmd->entry, &queue->resp_send_list);
> + node = node->next;
> + queue->send_list_len++;
> + }
> +
> + cmd = list_first_entry(&queue->resp_send_list, struct nvmet_tcp_cmd, entry);
> + return cmd;
Besides the way too long line this can be a direct return. Then
again moving the assignment of this in would probably make sense
as well.
> +}
> +
> +static struct nvmet_tcp_cmd *nvmet_tcp_fetch_send_command(struct nvmet_tcp_queue *queue)
Another way too long line. Please just fix this up everwhere.
> + if (!cmd || queue->state == NVMET_TCP_Q_DISCONNECTING) {
> + cmd = nvmet_tcp_fetch_send_command(queue);
> + if (unlikely(!cmd))
> + return 0;
> + }
> +
> + if (cmd->state == NVMET_TCP_SEND_DATA_PDU) {
> + ret = nvmet_try_send_data_pdu(cmd);
> + if (ret <= 0)
> + goto done_send;
> + }
> +
> + if (cmd->state == NVMET_TCP_SEND_DATA) {
> + ret = nvmet_try_send_data(cmd);
> + if (ret <= 0)
> + goto done_send;
> + }
> +
> + if (cmd->state == NVMET_TCP_SEND_DDGST) {
> + ret = nvmet_try_send_ddgst(cmd);
> + if (ret <= 0)
> + goto done_send;
> + }
> +
> + if (cmd->state == NVMET_TCP_SEND_R2T) {
> + ret = nvmet_try_send_r2t(cmd, last_in_batch);
> + if (ret <= 0)
> + goto done_send;
> + }
> +
> + if (cmd->state == NVMET_TCP_SEND_RESPONSE)
> + ret = nvmet_try_send_response(cmd, last_in_batch);
Use a switch statement?
> + if (queue->left) {
> + return -EAGAIN;
> + } else if (queue->offset == sizeof(struct nvme_tcp_hdr)) {
No need for an else after a return.
> +
> + if (unlikely(queue->rcv_state == NVMET_TCP_RECV_ERR))
> + return 0;
> +
> + if (queue->rcv_state == NVMET_TCP_RECV_PDU) {
> + result = nvmet_tcp_try_recv_pdu(queue);
> + if (result != 0)
> + goto done_recv;
> + }
> +
> + if (queue->rcv_state == NVMET_TCP_RECV_DATA) {
> + result = nvmet_tcp_try_recv_data(queue);
> + if (result != 0)
> + goto done_recv;
> + }
> +
> + if (queue->rcv_state == NVMET_TCP_RECV_DDGST) {
> + result = nvmet_tcp_try_recv_ddgst(queue);
> + if (result != 0)
> + goto done_recv;
> + }
switch statement?
> + spin_lock(&queue->state_lock);
> + if (queue->state == NVMET_TCP_Q_DISCONNECTING)
> + goto out;
> +
> + queue->state = NVMET_TCP_Q_DISCONNECTING;
> + schedule_work(&queue->release_work);
> +out:
> + spin_unlock(&queue->state_lock);
No real need for the goto here.
> +static void nvmet_tcp_data_ready(struct sock *sk)
> +{
> + struct nvmet_tcp_queue *queue;
> +
> + read_lock_bh(&sk->sk_callback_lock);
> + queue = sk->sk_user_data;
> + if (!queue)
> + goto out;
> +
> + queue_work_on(queue->cpu, nvmet_tcp_wq, &queue->io_work);
> +out:
> + read_unlock_bh(&sk->sk_callback_lock);
> +}
This should only need rcu_read_proctection, right?
Also no need for the goto.
^ permalink raw reply
* Re: net: thunderx: nicvf_xdp_setup error code path
From: Lorenzo Bianconi @ 2018-11-22 9:11 UTC (permalink / raw)
To: David Miller; +Cc: netdev, sgoutham
In-Reply-To: <20181121.161846.32682644237371562.davem@davemloft.net>
>
> Sunil did review this, so please resubmit formally.
Hi David,
I have already posted a formal patch:
https://patchwork.ozlabs.org/patch/1001213/
Reviewing the mail I have not explicitly added you in cc, sorry
Regards,
Lorenzo
^ permalink raw reply
* [PATCH AUTOSEL 4.19 25/36] SUNRPC: Fix a bogus get/put in generic_key_to_expire()
From: Sasha Levin @ 2018-11-22 19:52 UTC (permalink / raw)
To: stable, linux-kernel; +Cc: Trond Myklebust, Sasha Levin, linux-nfs, netdev
In-Reply-To: <20181122195240.13123-1-sashal@kernel.org>
From: Trond Myklebust <trond.myklebust@hammerspace.com>
[ Upstream commit e3d5e573a54dabdc0f9f3cb039d799323372b251 ]
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/sunrpc/auth_generic.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/net/sunrpc/auth_generic.c b/net/sunrpc/auth_generic.c
index f1df9837f1ac..1ac08dcbf85d 100644
--- a/net/sunrpc/auth_generic.c
+++ b/net/sunrpc/auth_generic.c
@@ -281,13 +281,7 @@ static bool generic_key_to_expire(struct rpc_cred *cred)
{
struct auth_cred *acred = &container_of(cred, struct generic_cred,
gc_base)->acred;
- bool ret;
-
- get_rpccred(cred);
- ret = test_bit(RPC_CRED_KEY_EXPIRE_SOON, &acred->ac_flags);
- put_rpccred(cred);
-
- return ret;
+ return test_bit(RPC_CRED_KEY_EXPIRE_SOON, &acred->ac_flags);
}
static const struct rpc_credops generic_credops = {
--
2.17.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 15/21] SUNRPC: Fix a bogus get/put in generic_key_to_expire()
From: Sasha Levin @ 2018-11-22 19:54 UTC (permalink / raw)
To: stable, linux-kernel; +Cc: Trond Myklebust, Sasha Levin, linux-nfs, netdev
In-Reply-To: <20181122195452.13520-1-sashal@kernel.org>
From: Trond Myklebust <trond.myklebust@hammerspace.com>
[ Upstream commit e3d5e573a54dabdc0f9f3cb039d799323372b251 ]
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/sunrpc/auth_generic.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/net/sunrpc/auth_generic.c b/net/sunrpc/auth_generic.c
index f1df9837f1ac..1ac08dcbf85d 100644
--- a/net/sunrpc/auth_generic.c
+++ b/net/sunrpc/auth_generic.c
@@ -281,13 +281,7 @@ static bool generic_key_to_expire(struct rpc_cred *cred)
{
struct auth_cred *acred = &container_of(cred, struct generic_cred,
gc_base)->acred;
- bool ret;
-
- get_rpccred(cred);
- ret = test_bit(RPC_CRED_KEY_EXPIRE_SOON, &acred->ac_flags);
- put_rpccred(cred);
-
- return ret;
+ return test_bit(RPC_CRED_KEY_EXPIRE_SOON, &acred->ac_flags);
}
static const struct rpc_credops generic_credops = {
--
2.17.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 11/15] SUNRPC: Fix a bogus get/put in generic_key_to_expire()
From: Sasha Levin @ 2018-11-22 19:56 UTC (permalink / raw)
To: stable, linux-kernel; +Cc: Trond Myklebust, Sasha Levin, linux-nfs, netdev
In-Reply-To: <20181122195621.13776-1-sashal@kernel.org>
From: Trond Myklebust <trond.myklebust@hammerspace.com>
[ Upstream commit e3d5e573a54dabdc0f9f3cb039d799323372b251 ]
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/sunrpc/auth_generic.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/net/sunrpc/auth_generic.c b/net/sunrpc/auth_generic.c
index f1df9837f1ac..1ac08dcbf85d 100644
--- a/net/sunrpc/auth_generic.c
+++ b/net/sunrpc/auth_generic.c
@@ -281,13 +281,7 @@ static bool generic_key_to_expire(struct rpc_cred *cred)
{
struct auth_cred *acred = &container_of(cred, struct generic_cred,
gc_base)->acred;
- bool ret;
-
- get_rpccred(cred);
- ret = test_bit(RPC_CRED_KEY_EXPIRE_SOON, &acred->ac_flags);
- put_rpccred(cred);
-
- return ret;
+ return test_bit(RPC_CRED_KEY_EXPIRE_SOON, &acred->ac_flags);
}
static const struct rpc_credops generic_credops = {
--
2.17.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.4 5/8] SUNRPC: Fix a bogus get/put in generic_key_to_expire()
From: Sasha Levin @ 2018-11-22 19:57 UTC (permalink / raw)
To: stable, linux-kernel; +Cc: Trond Myklebust, Sasha Levin, linux-nfs, netdev
In-Reply-To: <20181122195716.13961-1-sashal@kernel.org>
From: Trond Myklebust <trond.myklebust@hammerspace.com>
[ Upstream commit e3d5e573a54dabdc0f9f3cb039d799323372b251 ]
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/sunrpc/auth_generic.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/net/sunrpc/auth_generic.c b/net/sunrpc/auth_generic.c
index 41248b1820c7..cc27e38392ea 100644
--- a/net/sunrpc/auth_generic.c
+++ b/net/sunrpc/auth_generic.c
@@ -272,13 +272,7 @@ static bool generic_key_to_expire(struct rpc_cred *cred)
{
struct auth_cred *acred = &container_of(cred, struct generic_cred,
gc_base)->acred;
- bool ret;
-
- get_rpccred(cred);
- ret = test_bit(RPC_CRED_KEY_EXPIRE_SOON, &acred->ac_flags);
- put_rpccred(cred);
-
- return ret;
+ return test_bit(RPC_CRED_KEY_EXPIRE_SOON, &acred->ac_flags);
}
static const struct rpc_credops generic_credops = {
--
2.17.1
^ permalink raw reply related
* [PATCH AUTOSEL 3.18 4/7] SUNRPC: Fix a bogus get/put in generic_key_to_expire()
From: Sasha Levin @ 2018-11-22 19:57 UTC (permalink / raw)
To: stable, linux-kernel; +Cc: Trond Myklebust, Sasha Levin, linux-nfs, netdev
In-Reply-To: <20181122195749.14086-1-sashal@kernel.org>
From: Trond Myklebust <trond.myklebust@hammerspace.com>
[ Upstream commit e3d5e573a54dabdc0f9f3cb039d799323372b251 ]
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/sunrpc/auth_generic.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/net/sunrpc/auth_generic.c b/net/sunrpc/auth_generic.c
index 6f6b829c9e8e..42e93bf496e7 100644
--- a/net/sunrpc/auth_generic.c
+++ b/net/sunrpc/auth_generic.c
@@ -272,13 +272,7 @@ static bool generic_key_to_expire(struct rpc_cred *cred)
{
struct auth_cred *acred = &container_of(cred, struct generic_cred,
gc_base)->acred;
- bool ret;
-
- get_rpccred(cred);
- ret = test_bit(RPC_CRED_KEY_EXPIRE_SOON, &acred->ac_flags);
- put_rpccred(cred);
-
- return ret;
+ return test_bit(RPC_CRED_KEY_EXPIRE_SOON, &acred->ac_flags);
}
static const struct rpc_credops generic_credops = {
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 1/2] bpf: add __weak hook for allocating executable memory
From: kbuild test robot @ 2018-11-22 20:01 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: kbuild-all, linux-arm-kernel, Ard Biesheuvel, Daniel Borkmann,
Alexei Starovoitov, Rick Edgecombe, Eric Dumazet, Jann Horn,
Kees Cook, Jessica Yu, Arnd Bergmann, Catalin Marinas,
Will Deacon, Mark Rutland, David S. Miller, linux-kernel, netdev
In-Reply-To: <20181121131733.14910-2-ard.biesheuvel@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 1696 bytes --]
Hi Ard,
I love your patch! Yet something to improve:
[auto build test ERROR on bpf-next/master]
[also build test ERROR on v4.20-rc3 next-20181122]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ard-Biesheuvel/bpf-permit-JIT-allocations-to-be-served-outside-the-module-region/20181123-033144
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-randconfig-x002-201846 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
kernel//bpf/core.c: In function 'bpf_jit_free_exec':
>> kernel//bpf/core.c:632:24: error: 'size' undeclared (first use in this function); did you mean 'ksize'?
return module_memfree(size);
^~~~
ksize
kernel//bpf/core.c:632:24: note: each undeclared identifier is reported only once for each function it appears in
>> kernel//bpf/core.c:632:9: warning: 'return' with a value, in function returning void
return module_memfree(size);
^~~~~~~~~~~~~~
kernel//bpf/core.c:630:13: note: declared here
void __weak bpf_jit_free_exec(const void *addr)
^~~~~~~~~~~~~~~~~
vim +632 kernel//bpf/core.c
629
630 void __weak bpf_jit_free_exec(const void *addr)
631 {
> 632 return module_memfree(size);
633 }
634
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23659 bytes --]
^ permalink raw reply
* [PATCH net-next 01/10] net/smc: cleanup tcp_listen_worker initialization
From: Ursula Braun @ 2018-11-22 9:26 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20181122092643.26820-1-ubraun@linux.ibm.com>
The tcp_listen_worker is already initialized when socket is
created (in smc_sock_alloc()). Get rid of the duplicate
initialization in smc_listen(). No functional change.
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/af_smc.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 80e2119f1c70..d9b1a0e4446c 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1354,7 +1354,6 @@ static int smc_listen(struct socket *sock, int backlog)
sk->sk_max_ack_backlog = backlog;
sk->sk_ack_backlog = 0;
sk->sk_state = SMC_LISTEN;
- INIT_WORK(&smc->tcp_listen_work, smc_tcp_listen_work);
sock_hold(sk); /* sock_hold in tcp_listen_worker */
if (!schedule_work(&smc->tcp_listen_work))
sock_put(sk);
--
2.16.4
^ permalink raw reply related
* [PATCH net-next 00/10] net/smc: patches 2018-11-22
From: Ursula Braun @ 2018-11-22 9:26 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
Dave,
here are more patches for SMC:
* patches 1-3 and 7 are cleanups without functional change
* patches 4-6 and 8 are optimizations of existing code
* patches 9 and 10 introduce and exploit LLC message DELETE RKEY
Thanks, Ursula
Karsten Graul (3):
net/smc: avoid a delay by waiting for nothing
net/smc: add infrastructure to send delete rkey messages
net/smc: unregister rkeys of unused buffer
Ursula Braun (7):
net/smc: cleanup tcp_listen_worker initialization
net/smc: make smc_lgr_free() static
net/smc: remove sock_error detour in clc-functions
net/smc: allow fallback after clc timeouts
net/smc: no link delete for a never active link
net/smc: short wait for late smc_clc_wait_msg
net/smc: cleanup listen worker mutex unlocking
net/smc/af_smc.c | 51 ++++++++++++++++++++++++------------------------
net/smc/smc_clc.c | 33 ++++++++++++++-----------------
net/smc/smc_clc.h | 3 ++-
net/smc/smc_core.c | 16 ++++++++++++---
net/smc/smc_core.h | 6 ++++--
net/smc/smc_llc.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
net/smc/smc_llc.h | 2 ++
7 files changed, 117 insertions(+), 51 deletions(-)
--
2.16.4
^ permalink raw reply
* [PATCH net-next 02/10] net/smc: make smc_lgr_free() static
From: Ursula Braun @ 2018-11-22 9:26 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20181122092643.26820-1-ubraun@linux.ibm.com>
smc_lgr_free() is just called inside smc_core.c. Make it static.
Just cleanup, no functional change.
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_core.c | 4 +++-
net/smc/smc_core.h | 1 -
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 18daebcef181..4812ca30f1dc 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -149,6 +149,8 @@ static int smc_link_send_delete(struct smc_link *lnk)
return -ENOTCONN;
}
+static void smc_lgr_free(struct smc_link_group *lgr);
+
static void smc_lgr_free_work(struct work_struct *work)
{
struct smc_link_group *lgr = container_of(to_delayed_work(work),
@@ -408,7 +410,7 @@ static void smc_lgr_free_bufs(struct smc_link_group *lgr)
}
/* remove a link group */
-void smc_lgr_free(struct smc_link_group *lgr)
+static void smc_lgr_free(struct smc_link_group *lgr)
{
smc_lgr_free_bufs(lgr);
if (lgr->is_smcd)
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index c156674733c9..07364c0b41a1 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -243,7 +243,6 @@ struct smc_sock;
struct smc_clc_msg_accept_confirm;
struct smc_clc_msg_local;
-void smc_lgr_free(struct smc_link_group *lgr);
void smc_lgr_forget(struct smc_link_group *lgr);
void smc_lgr_terminate(struct smc_link_group *lgr);
void smc_port_terminate(struct smc_ib_device *smcibdev, u8 ibport);
--
2.16.4
^ permalink raw reply related
* [PATCH net-next 03/10] net/smc: remove sock_error detour in clc-functions
From: Ursula Braun @ 2018-11-22 9:26 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20181122092643.26820-1-ubraun@linux.ibm.com>
There is no need to store the return value in sk_err, if it is
afterwards cleared again with sock_error(). This patch sets the
return value directly. Just cleanup, no functional change.
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_clc.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 89c3a8c7859a..7278ec0cfa58 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -374,10 +374,8 @@ int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info)
len = kernel_sendmsg(smc->clcsock, &msg, &vec, 1,
sizeof(struct smc_clc_msg_decline));
if (len < sizeof(struct smc_clc_msg_decline))
- smc->sk.sk_err = EPROTO;
- if (len < 0)
- smc->sk.sk_err = -len;
- return sock_error(&smc->sk);
+ len = -EPROTO;
+ return len > 0 ? 0 : len;
}
/* send CLC PROPOSAL message across internal TCP socket */
@@ -536,7 +534,6 @@ int smc_clc_send_accept(struct smc_sock *new_smc, int srv_first_contact)
struct smc_link *link;
struct msghdr msg;
struct kvec vec;
- int rc = 0;
int len;
memset(&aclc, 0, sizeof(aclc));
@@ -589,13 +586,8 @@ int smc_clc_send_accept(struct smc_sock *new_smc, int srv_first_contact)
vec.iov_len = ntohs(aclc.hdr.length);
len = kernel_sendmsg(new_smc->clcsock, &msg, &vec, 1,
ntohs(aclc.hdr.length));
- if (len < ntohs(aclc.hdr.length)) {
- if (len >= 0)
- new_smc->sk.sk_err = EPROTO;
- else
- new_smc->sk.sk_err = new_smc->clcsock->sk->sk_err;
- rc = sock_error(&new_smc->sk);
- }
+ if (len < ntohs(aclc.hdr.length))
+ len = len >= 0 ? -EPROTO : -new_smc->clcsock->sk->sk_err;
- return rc;
+ return len > 0 ? 0 : len;
}
--
2.16.4
^ permalink raw reply related
* [PATCH net-next 04/10] net/smc: allow fallback after clc timeouts
From: Ursula Braun @ 2018-11-22 9:26 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20181122092643.26820-1-ubraun@linux.ibm.com>
If connection initialization fails for the LLC CONFIRM LINK or the
LLC ADD LINK step, fallback to TCP should be enabled. Thus
the negative return code -EAGAIN should switch to a positive timeout
reason code in these cases, and the internal CLC socket should
not have a set sk_err.
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/af_smc.c | 8 ++++----
net/smc/smc_clc.c | 9 +++++++--
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index d9b1a0e4446c..66836cfbc587 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -336,7 +336,7 @@ static int smc_clnt_conf_first_link(struct smc_sock *smc)
rc = smc_clc_wait_msg(smc, &dclc, sizeof(dclc),
SMC_CLC_DECLINE);
- return rc;
+ return rc == -EAGAIN ? SMC_CLC_DECL_TIMEOUT_CL : rc;
}
if (link->llc_confirm_rc)
@@ -364,7 +364,7 @@ static int smc_clnt_conf_first_link(struct smc_sock *smc)
rc = smc_clc_wait_msg(smc, &dclc, sizeof(dclc),
SMC_CLC_DECLINE);
- return rc;
+ return rc == -EAGAIN ? SMC_CLC_DECL_TIMEOUT_AL : rc;
}
/* send add link reject message, only one link supported for now */
@@ -966,7 +966,7 @@ static int smc_serv_conf_first_link(struct smc_sock *smc)
rc = smc_clc_wait_msg(smc, &dclc, sizeof(dclc),
SMC_CLC_DECLINE);
- return rc;
+ return rc == -EAGAIN ? SMC_CLC_DECL_TIMEOUT_CL : rc;
}
if (link->llc_confirm_resp_rc)
@@ -987,7 +987,7 @@ static int smc_serv_conf_first_link(struct smc_sock *smc)
rc = smc_clc_wait_msg(smc, &dclc, sizeof(dclc),
SMC_CLC_DECLINE);
- return rc;
+ return rc == -EAGAIN ? SMC_CLC_DECL_TIMEOUT_AL : rc;
}
smc_llc_link_active(link, net->ipv4.sysctl_tcp_keepalive_time);
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 7278ec0cfa58..62043d69e3a3 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -297,7 +297,11 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
}
if (clc_sk->sk_err) {
reason_code = -clc_sk->sk_err;
- smc->sk.sk_err = clc_sk->sk_err;
+ if (clc_sk->sk_err == EAGAIN &&
+ expected_type == SMC_CLC_DECLINE)
+ clc_sk->sk_err = 0; /* reset for fallback usage */
+ else
+ smc->sk.sk_err = clc_sk->sk_err;
goto out;
}
if (!len) { /* peer has performed orderly shutdown */
@@ -306,7 +310,8 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
goto out;
}
if (len < 0) {
- smc->sk.sk_err = -len;
+ if (len != -EAGAIN || expected_type != SMC_CLC_DECLINE)
+ smc->sk.sk_err = -len;
reason_code = len;
goto out;
}
--
2.16.4
^ permalink raw reply related
* [PATCH net-next 05/10] net/smc: no link delete for a never active link
From: Ursula Braun @ 2018-11-22 9:26 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20181122092643.26820-1-ubraun@linux.ibm.com>
If a link is terminated that has never reached the active state,
there is no need to trigger an LLC DELETE LINK.
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 4812ca30f1dc..ec7a7ed3b968 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -173,8 +173,11 @@ static void smc_lgr_free_work(struct work_struct *work)
spin_unlock_bh(&smc_lgr_list.lock);
if (!lgr->is_smcd && !lgr->terminating) {
+ struct smc_link *lnk = &lgr->lnk[SMC_SINGLE_LINK];
+
/* try to send del link msg, on error free lgr immediately */
- if (!smc_link_send_delete(&lgr->lnk[SMC_SINGLE_LINK])) {
+ if (lnk->state == SMC_LNK_ACTIVE &&
+ !smc_link_send_delete(lnk)) {
/* reschedule in case we never receive a response */
smc_lgr_schedule_free_work(lgr);
return;
--
2.16.4
^ permalink raw reply related
* [PATCH net-next 07/10] net/smc: cleanup listen worker mutex unlocking
From: Ursula Braun @ 2018-11-22 9:26 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20181122092643.26820-1-ubraun@linux.ibm.com>
For easier reading move the unlock of mutex smc_create_lgr_pending into
smc_listen_work(), i.e. into the function the mutex has been locked.
No functional change.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/af_smc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 93f7ff8f6e88..7657e249f526 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1182,7 +1182,6 @@ static int smc_listen_rdma_finish(struct smc_sock *new_smc,
return 0;
decline:
- mutex_unlock(&smc_create_lgr_pending);
smc_listen_decline(new_smc, reason_code, local_contact);
return reason_code;
}
@@ -1282,8 +1281,10 @@ static void smc_listen_work(struct work_struct *work)
/* finish worker */
if (!ism_supported) {
- if (smc_listen_rdma_finish(new_smc, &cclc, local_contact))
+ if (smc_listen_rdma_finish(new_smc, &cclc, local_contact)) {
+ mutex_unlock(&smc_create_lgr_pending);
return;
+ }
}
smc_conn_save_peer_info(new_smc, &cclc);
mutex_unlock(&smc_create_lgr_pending);
--
2.16.4
^ 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