* [PATCH net-next] Revert "r8169: improve handling VLAN tag"
From: Heiner Kallweit @ 2019-07-02 5:59 UTC (permalink / raw)
To: Realtek linux nic maintainers, David Miller
Cc: netdev@vger.kernel.org, Al Viro
This reverts commit 759d095741721888b6ee51afa74e0a66ce65e974.
The patch was based on a misunderstanding. As Al Viro pointed out [0]
it's simply wrong on big endian. So let's revert it.
[0] https://marc.info/?t=156200975600004&r=1&w=2
Reported-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169_main.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index a73f25321..22b2f3dcb 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -1528,7 +1528,7 @@ static int rtl8169_set_features(struct net_device *dev,
static inline u32 rtl8169_tx_vlan_tag(struct sk_buff *skb)
{
return (skb_vlan_tag_present(skb)) ?
- TxVlanTag | htons(skb_vlan_tag_get(skb)) : 0x00;
+ TxVlanTag | swab16(skb_vlan_tag_get(skb)) : 0x00;
}
static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
@@ -1536,8 +1536,7 @@ static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
u32 opts2 = le32_to_cpu(desc->opts2);
if (opts2 & RxVlanTag)
- __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
- ntohs(opts2 & 0xffff));
+ __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), swab16(opts2 & 0xffff));
}
static void rtl8169_get_regs(struct net_device *dev, struct ethtool_regs *regs,
--
2.22.0
^ permalink raw reply related
* [PATCH bpf-next] libbpf: fix GCC8 warning for strncpy
From: Andrii Nakryiko @ 2019-07-02 5:46 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel
Cc: Andrii Nakryiko, Magnus Karlsson
GCC8 started emitting warning about using strncpy with number of bytes
exactly equal destination size, which is generally unsafe, as can lead
to non-zero terminated string being copied. Use IFNAMSIZ - 1 as number
of bytes to ensure name is always zero-terminated.
Cc: Magnus Karlsson <magnus.karlsson@intel.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/xsk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index bf15a80a37c2..9588e7f87d0b 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -327,7 +327,7 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
channels.cmd = ETHTOOL_GCHANNELS;
ifr.ifr_data = (void *)&channels;
- strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ);
+ strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ - 1);
err = ioctl(fd, SIOCETHTOOL, &ifr);
if (err && errno != EOPNOTSUPP) {
ret = -errno;
--
2.17.1
^ permalink raw reply related
* Re: [RFC iproute2] netns: add mounting state file for each netns
From: Alexander Aring @ 2019-07-02 5:36 UTC (permalink / raw)
To: Matteo Croce; +Cc: netdev, Nicolas Dichtel
In-Reply-To: <20190630192933.30743-1-mcroce@redhat.com>
Hi Matteo,
On Sun, Jun 30, 2019 at 09:29:33PM +0200, Matteo Croce wrote:
> When ip creates a netns, there is a small time interval between the
> placeholder file creation in NETNS_RUN_DIR and the bind mount from /proc.
>
> Add a temporary file named .mounting-$netns which gets deleted after the
> bind mount, so watching for delete event matching the .mounting-* name
> will notify watchers only after the bind mount has been done.
>
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
thanks for working on it and making my mess better!
Would be nice to have it upstream.
- Alex
^ permalink raw reply
* Re: [for-next V2 10/10] RDMA/core: Provide RDMA DIM support for ULPs
From: Sagi Grimberg @ 2019-07-02 5:36 UTC (permalink / raw)
To: Idan Burstein, Saeed Mahameed, David S. Miller, Doug Ledford,
Jason Gunthorpe
Cc: Leon Romanovsky, Or Gerlitz, Tal Gilboa, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Yamin Friedman, Max Gurtovoy
In-Reply-To: <AM5PR0501MB248327B260F97EF97CD5B80EC5E20@AM5PR0501MB2483.eurprd05.prod.outlook.com>
Hey Idan,
> " Please don't. This is a bad choice to opt it in by default."
>
> I disagree here. I'd prefer Linux to have good out of the box experience (e.g. reach 100G in 4K NVMeOF on Intel servers) with the default parameters. Especially since Yamin have shown it is beneficial / not hurting in terms of performance for variety of use cases. The whole concept of DIM is that it adapts to the workload requirements in terms of bandwidth and latency.
Well, its a Mellanox device driver after all.
But do note that by far, the vast majority of users are not saturating
100G of 4K I/O. The absolute vast majority of users are primarily
sensitive to synchronous QD=1 I/O latency, and when the workload
is much more dynamic than the synthetic 100%/50%/0% read mix.
As much as I'm a fan (IIRC I was the one giving a first pass at this),
the dim default opt-in is not only not beneficial, but potentially
harmful to the majority of users out-of-the-box experience.
Given that this is a fresh code with almost no exposure, and that was
not tested outside of Yamin running limited performance testing, I think
it would be a mistake to add it as a default opt-in, that can come as an
incremental stage.
Obviously, I cannot tell what Mellanox should/shouldn't do in its own
device driver of course, but I just wanted to emphasize that I think
this is a mistake.
> Moreover, net-dim is enabled by default, I don't see why RDMA is different.
Very different animals.
^ permalink raw reply
* Re: Reminder: 2 open syzbot bugs in vhost subsystem
From: Eric Biggers @ 2019-07-02 5:32 UTC (permalink / raw)
To: Jason Wang
Cc: kvm, virtualization, netdev, Michael S. Tsirkin, Hillf Danton,
linux-kernel, syzkaller-bugs
In-Reply-To: <e2da1124-52c3-84ff-77ce-deb017711138@redhat.com>
On Tue, Jul 02, 2019 at 01:24:43PM +0800, Jason Wang wrote:
> > --------------------------------------------------------------------------------
> > Title: INFO: task hung in vhost_init_device_iotlb
> > Last occurred: 125 days ago
> > Reported: 153 days ago
> > Branches: Mainline and others
> > Dashboard link: https://syzkaller.appspot.com/bug?id=cb1ea8daf03a5942c2ab314679148cf6e128ef58
> > Original thread: https://lkml.kernel.org/lkml/0000000000007e86fd058095533f@google.com/T/#u
> >
> > Unfortunately, this bug does not have a reproducer.
> >
> > The original thread for this bug received 2 replies; the last was 152 days ago.
> >
> > If you fix this bug, please add the following tag to the commit:
> > Reported-by: syzbot+40e28a8bd59d10ed0c42@syzkaller.appspotmail.com
> >
> > If you send any email or patch for this bug, please consider replying to the
> > original thread. For the git send-email command to use, or tips on how to reply
> > if the thread isn't in your mailbox, see the "Reply instructions" at
> > https://lkml.kernel.org/r/0000000000007e86fd058095533f@google.com
> >
>
> Can syzbot still reproduce this issue?
Apparently not, as it last occurred 125 days ago.
That doesn't necessarily mean the bug isn't still there, though.
But if you (as a person familiar with the code) think it's no longer valid or
not actionable, you can invalidate it.
- Eric
^ permalink raw reply
* Re: Reminder: 2 open syzbot bugs in vhost subsystem
From: Jason Wang @ 2019-07-02 5:24 UTC (permalink / raw)
To: Eric Biggers, kvm, virtualization, netdev, Michael S. Tsirkin,
Hillf Danton
Cc: linux-kernel, syzkaller-bugs
In-Reply-To: <20190702051707.GF23743@sol.localdomain>
On 2019/7/2 下午1:17, Eric Biggers wrote:
> [This email was generated by a script. Let me know if you have any suggestions
> to make it better, or if you want it re-generated with the latest status.]
>
> Of the currently open syzbot reports against the upstream kernel, I've manually
> marked 2 of them as possibly being bugs in the vhost subsystem. I've listed
> these reports below, sorted by an algorithm that tries to list first the reports
> most likely to be still valid, important, and actionable.
>
> Of these 2 bugs, 1 was seen in mainline in the last week.
>
> If you believe a bug is no longer valid, please close the syzbot report by
> sending a '#syz fix', '#syz dup', or '#syz invalid' command in reply to the
> original thread, as explained at https://goo.gl/tpsmEJ#status
>
> If you believe I misattributed a bug to the vhost subsystem, please let me know,
> and if possible forward the report to the correct people or mailing list.
>
> Here are the bugs:
>
> --------------------------------------------------------------------------------
> Title: memory leak in vhost_net_ioctl
> Last occurred: 0 days ago
> Reported: 26 days ago
> Branches: Mainline
> Dashboard link: https://syzkaller.appspot.com/bug?id=12ba349d7e26ccfe95317bc376e812ebbae2ee0f
> Original thread: https://lkml.kernel.org/lkml/000000000000188da1058a9c25e3@google.com/T/#u
>
> This bug has a C reproducer.
>
> The original thread for this bug has received 4 replies; the last was 17 days
> ago.
>
> If you fix this bug, please add the following tag to the commit:
> Reported-by: syzbot+0789f0c7e45efd7bb643@syzkaller.appspotmail.com
>
> If you send any email or patch for this bug, please consider replying to the
> original thread. For the git send-email command to use, or tips on how to reply
> if the thread isn't in your mailbox, see the "Reply instructions" at
> https://lkml.kernel.org/r/000000000000188da1058a9c25e3@google.com
Cc Hillf who should had a fix for this.
Hillf, would you please post a formal patch for this? (for -net)
>
> --------------------------------------------------------------------------------
> Title: INFO: task hung in vhost_init_device_iotlb
> Last occurred: 125 days ago
> Reported: 153 days ago
> Branches: Mainline and others
> Dashboard link: https://syzkaller.appspot.com/bug?id=cb1ea8daf03a5942c2ab314679148cf6e128ef58
> Original thread: https://lkml.kernel.org/lkml/0000000000007e86fd058095533f@google.com/T/#u
>
> Unfortunately, this bug does not have a reproducer.
>
> The original thread for this bug received 2 replies; the last was 152 days ago.
>
> If you fix this bug, please add the following tag to the commit:
> Reported-by: syzbot+40e28a8bd59d10ed0c42@syzkaller.appspotmail.com
>
> If you send any email or patch for this bug, please consider replying to the
> original thread. For the git send-email command to use, or tips on how to reply
> if the thread isn't in your mailbox, see the "Reply instructions" at
> https://lkml.kernel.org/r/0000000000007e86fd058095533f@google.com
>
Can syzbot still reproduce this issue?
Thanks
^ permalink raw reply
* Reminder: 2 open syzbot bugs in vhost subsystem
From: Eric Biggers @ 2019-07-02 5:17 UTC (permalink / raw)
To: kvm, virtualization, netdev, Michael S. Tsirkin, Jason Wang
Cc: linux-kernel, syzkaller-bugs
[This email was generated by a script. Let me know if you have any suggestions
to make it better, or if you want it re-generated with the latest status.]
Of the currently open syzbot reports against the upstream kernel, I've manually
marked 2 of them as possibly being bugs in the vhost subsystem. I've listed
these reports below, sorted by an algorithm that tries to list first the reports
most likely to be still valid, important, and actionable.
Of these 2 bugs, 1 was seen in mainline in the last week.
If you believe a bug is no longer valid, please close the syzbot report by
sending a '#syz fix', '#syz dup', or '#syz invalid' command in reply to the
original thread, as explained at https://goo.gl/tpsmEJ#status
If you believe I misattributed a bug to the vhost subsystem, please let me know,
and if possible forward the report to the correct people or mailing list.
Here are the bugs:
--------------------------------------------------------------------------------
Title: memory leak in vhost_net_ioctl
Last occurred: 0 days ago
Reported: 26 days ago
Branches: Mainline
Dashboard link: https://syzkaller.appspot.com/bug?id=12ba349d7e26ccfe95317bc376e812ebbae2ee0f
Original thread: https://lkml.kernel.org/lkml/000000000000188da1058a9c25e3@google.com/T/#u
This bug has a C reproducer.
The original thread for this bug has received 4 replies; the last was 17 days
ago.
If you fix this bug, please add the following tag to the commit:
Reported-by: syzbot+0789f0c7e45efd7bb643@syzkaller.appspotmail.com
If you send any email or patch for this bug, please consider replying to the
original thread. For the git send-email command to use, or tips on how to reply
if the thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/000000000000188da1058a9c25e3@google.com
--------------------------------------------------------------------------------
Title: INFO: task hung in vhost_init_device_iotlb
Last occurred: 125 days ago
Reported: 153 days ago
Branches: Mainline and others
Dashboard link: https://syzkaller.appspot.com/bug?id=cb1ea8daf03a5942c2ab314679148cf6e128ef58
Original thread: https://lkml.kernel.org/lkml/0000000000007e86fd058095533f@google.com/T/#u
Unfortunately, this bug does not have a reproducer.
The original thread for this bug received 2 replies; the last was 152 days ago.
If you fix this bug, please add the following tag to the commit:
Reported-by: syzbot+40e28a8bd59d10ed0c42@syzkaller.appspotmail.com
If you send any email or patch for this bug, please consider replying to the
original thread. For the git send-email command to use, or tips on how to reply
if the thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/0000000000007e86fd058095533f@google.com
^ permalink raw reply
* Re: [PATCH bpf-next v2] virtio_net: add XDP meta data support
From: Yuya Kusakabe @ 2019-07-02 5:15 UTC (permalink / raw)
To: Jason Wang
Cc: ast, daniel, davem, hawk, jakub.kicinski, john.fastabend, kafai,
mst, netdev, songliubraving, yhs
In-Reply-To: <e703c548-6615-c830-3b8b-ead9346a6bb5@redhat.com>
On 7/2/19 12:59 PM, Jason Wang wrote:
>
> On 2019/7/2 上午11:15, Yuya Kusakabe wrote:
>> This adds XDP meta data support to both receive_small() and
>> receive_mergeable().
>>
>> Fixes: de8f3a83b0a0 ("bpf: add meta pointer for direct access")
>> Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
>> ---
>> v2:
>> - keep copy untouched in page_to_skb().
>> - preserve the vnet header in receive_small().
>> - fix indentation.
>> ---
>> drivers/net/virtio_net.c | 39 +++++++++++++++++++++++++++------------
>> 1 file changed, 27 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 4f3de0ac8b0b..2ebabb08b824 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -371,7 +371,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>> struct receive_queue *rq,
>> struct page *page, unsigned int offset,
>> unsigned int len, unsigned int truesize,
>> - bool hdr_valid)
>> + bool hdr_valid, unsigned int metasize)
>> {
>> struct sk_buff *skb;
>> struct virtio_net_hdr_mrg_rxbuf *hdr;
>> @@ -393,7 +393,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>> else
>> hdr_padded_len = sizeof(struct padded_vnet_hdr);
>> - if (hdr_valid)
>> + if (hdr_valid && !metasize)
>> memcpy(hdr, p, hdr_len);
>> len -= hdr_len;
>> @@ -405,6 +405,11 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>> copy = skb_tailroom(skb);
>> skb_put_data(skb, p, copy);
>> + if (metasize) {
>> + __skb_pull(skb, metasize);
>> + skb_metadata_set(skb, metasize);
>> + }
>> +
>> len -= copy;
>> offset += copy;
>> @@ -644,6 +649,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
>> unsigned int delta = 0;
>> struct page *xdp_page;
>> int err;
>> + unsigned int metasize = 0;
>> len -= vi->hdr_len;
>> stats->bytes += len;
>> @@ -683,8 +689,8 @@ static struct sk_buff *receive_small(struct net_device *dev,
>> xdp.data_hard_start = buf + VIRTNET_RX_PAD + vi->hdr_len;
>> xdp.data = xdp.data_hard_start + xdp_headroom;
>> - xdp_set_data_meta_invalid(&xdp);
>> xdp.data_end = xdp.data + len;
>> + xdp.data_meta = xdp.data;
>> xdp.rxq = &rq->xdp_rxq;
>> orig_data = xdp.data;
>> act = bpf_prog_run_xdp(xdp_prog, &xdp);
>> @@ -695,9 +701,11 @@ static struct sk_buff *receive_small(struct net_device *dev,
>> /* Recalculate length in case bpf program changed it */
>> delta = orig_data - xdp.data;
>> len = xdp.data_end - xdp.data;
>> + metasize = xdp.data - xdp.data_meta;
>> break;
>> case XDP_TX:
>> stats->xdp_tx++;
>> + xdp.data_meta = xdp.data;
>> xdpf = convert_to_xdp_frame(&xdp);
>> if (unlikely(!xdpf))
>> goto err_xdp;
>> @@ -740,6 +748,9 @@ static struct sk_buff *receive_small(struct net_device *dev,
>> memcpy(skb_vnet_hdr(skb), buf, vi->hdr_len);
>> } /* keep zeroed vnet hdr since packet was changed by bpf */
>
>
> I wonder whether or not it's as simple as this. Consider bpf may adjust meta, it looks to me that the vnet header will be overwrite here? If yes, we probably need to have a device specific value then bpf can move the device metadata like vnet header for us?>
> Thanks
Yes, the vnet header is overwrite by xdp metadata. I'll fix it.
Thanks.
>
>
>> + if (metasize)
>> + skb_metadata_set(skb, metasize);
>> +
>> err:
>> return skb;
>> @@ -760,8 +771,8 @@ static struct sk_buff *receive_big(struct net_device *dev,
>> struct virtnet_rq_stats *stats)
>> {
>> struct page *page = buf;
>> - struct sk_buff *skb = page_to_skb(vi, rq, page, 0, len,
>> - PAGE_SIZE, true);
>> + struct sk_buff *skb =
>> + page_to_skb(vi, rq, page, 0, len, PAGE_SIZE, true, 0);
>> stats->bytes += len - vi->hdr_len;
>> if (unlikely(!skb))
>> @@ -793,6 +804,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>> unsigned int truesize;
>> unsigned int headroom = mergeable_ctx_to_headroom(ctx);
>> int err;
>> + unsigned int metasize = 0;
>> head_skb = NULL;
>> stats->bytes += len - vi->hdr_len;
>> @@ -839,8 +851,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>> data = page_address(xdp_page) + offset;
>> xdp.data_hard_start = data - VIRTIO_XDP_HEADROOM + vi->hdr_len;
>> xdp.data = data + vi->hdr_len;
>> - xdp_set_data_meta_invalid(&xdp);
>> xdp.data_end = xdp.data + (len - vi->hdr_len);
>> + xdp.data_meta = xdp.data;
>> xdp.rxq = &rq->xdp_rxq;
>> act = bpf_prog_run_xdp(xdp_prog, &xdp);
>> @@ -852,8 +864,9 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>> * adjustments. Note other cases do not build an
>> * skb and avoid using offset
>> */
>> - offset = xdp.data -
>> - page_address(xdp_page) - vi->hdr_len;
>> + metasize = xdp.data - xdp.data_meta;
>> + offset = xdp.data - page_address(xdp_page) -
>> + vi->hdr_len - metasize;
>> /* recalculate len if xdp.data or xdp.data_end were
>> * adjusted
>> @@ -863,14 +876,15 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>> if (unlikely(xdp_page != page)) {
>> rcu_read_unlock();
>> put_page(page);
>> - head_skb = page_to_skb(vi, rq, xdp_page,
>> - offset, len,
>> - PAGE_SIZE, false);
>> + head_skb = page_to_skb(vi, rq, xdp_page, offset,
>> + len, PAGE_SIZE, false,
>> + metasize);
>> return head_skb;
>> }
>> break;
>> case XDP_TX:
>> stats->xdp_tx++;
>> + xdp.data_meta = xdp.data;
>> xdpf = convert_to_xdp_frame(&xdp);
>> if (unlikely(!xdpf))
>> goto err_xdp;
>> @@ -921,7 +935,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>> goto err_skb;
>> }
>> - head_skb = page_to_skb(vi, rq, page, offset, len, truesize, !xdp_prog);
>> + head_skb = page_to_skb(vi, rq, page, offset, len, truesize, !xdp_prog,
>> + metasize);
>> curr_skb = head_skb;
>> if (unlikely(!curr_skb))
^ permalink raw reply
* Re: [PATCH net-next 3/7] net/rds: Wait for the FRMR_IS_FREE (or FRMR_IS_STALE) transition after posting IB_WR_LOCAL_INV
From: Gerd Rausch @ 2019-07-02 5:11 UTC (permalink / raw)
To: santosh.shilimkar, netdev; +Cc: David Miller
In-Reply-To: <79d25e7c-ad9e-f6d8-b0fe-4ce04c658e1e@oracle.com>
Hi Santosh,
On 01/07/2019 19.28, santosh.shilimkar@oracle.com wrote:
>>
> Below. All command timeouts are 60 seconds.
>
> enum {
> MLX4_CMD_TIME_CLASS_A = 60000,
> MLX4_CMD_TIME_CLASS_B = 60000,
> MLX4_CMD_TIME_CLASS_C = 60000,
> };
>
Thank you for the pointer.
> But having said that, I re-looked the code you are patching
> and thats actually only FRWR code which is purely work-request
> based so this command timeout shouldn't matter.
>
Which brings us back full circle to the question of
what the timeout ought to be?
Please keep in mind that prior to this fix,
the RDS code didn't wait at all:
It simply posted those registration (IB_WR_REG_MR)
and invalidation (IB_WR_LOCAL_INV)
work-requests, with no regards to when the firmware
would execute them.
Arguably, waiting any amount time greater than zero
for the operation to complete is better than not waiting at all.
We can change the timeout to a high value, or even make it infinite
by using "wait_event" instead of "wait_event_timeout".
For the registration work-requests there is a benefit to wait a short
amount of time only (the trade-off described in patch #1 of this series).
For de-registration work-requests, it is beneficial to wait
until they are truly done.
But: Function "rds_ib_unreg_frmr" prior and post this change
simply moves on after a failed de-registration attempt,
and releases the pages owned by the memory region.
This patch does _not_ change that behavior.
> If the work request fails, then it will lead to flush errors and
> MRs will be marked as STALE. So this wait may not be necessary
>
This wait is necessary to avoid the 2 scenarios described
in the commit-log message:
#1) Memory regions bouncing between "drop_list" and "clean_list"
as items on the "clean_list" aren't really clean until
their state transitions to "FRMR_IS_FREE".
#2) Prevent an access error as "rds_ib_post_inv" is called
just prior to de-referencing pages via "__rds_ib_teardown_mr".
And you certainly don't want those pages populated in the
HCA's memory-translation-table with full access, while
the Linux kernel 'thinks' you gave them back already
and starts re-purposing them.
> RDS_GET_MR case is what actually showing the issue you saw
> and the fix for that Avinash has it in production kernel.
Actually, no:
Socket option RDS_GET_MR wasn't even in the code-path of the
tests I performed:
It were there RDS_CMSG_RDMA_MAP / RDS_CMSG_RDMA_DEST control
messages that ended up calling '__rds_rdma_map".
>
> I believe with that change, registration issue becomes non-issue
> already.
>
Please explain how that is related to this fix-suggestion?
I submitted this patch #3 and the others in this series in order
to fix bugs in the RDS that is currently shipped with Linux.
It may very well be the case that there are other changes
that Avinash put into production kernels that would be better
suited to fix this and other problems.
But that should not eliminate the need to fix what is currently broken.
Fixing what's broken does not preclude replacing the fixed code
with newer or better versions of the same.
> And as far as invalidation concerned with proxy qp, it not longer
> races with data path qp.
>
I don't understand, please elaborate.
> May be you can try those changes if not already to see if it
> addresses the couple of cases where you ended up adding
> timeouts.
>
I don't understand, please elaborate:
a) Are you saying this issue should not be fixed?
b) Or are you suggesting to replace this fix with a different fix?
If it's the later, please point out what you have in mind.
c) ???
Thanks,
Gerd
^ permalink raw reply
* Re: [PATCH net-next 3/3] macsec: add brackets and indentation after calling macsec_decrypt
From: Andreas Steinmetz @ 2019-07-02 4:37 UTC (permalink / raw)
To: Sabrina Dubroca, Willem de Bruijn; +Cc: Network Development
In-Reply-To: <20190701132157.GA15622@bistromath.localdomain>
Ouch, I missed that when Andreas sent me that patch before. No, it is
> actually intended. If we skip macsec_decrypt(), we should still
> account for that packet in the InPktsUnchecked/InPktsDelayed
> counters. That's in Figure 10-5 in the standard.
>
> Thanks for catching this, Willem. That patch should only move the
> IS_ERR(skb) case under the block where macsec_decrypt() is called, but
> not move the call to macsec_post_decrypt().
Updated patch below.
Signed-off-by: Andreas Steinmetz <ast@domdv.de>
--- a/drivers/net/macsec.c 2019-07-02 06:31:27.550120145 +0200
+++ b/drivers/net/macsec.c 2019-07-02 06:33:38.637599529 +0200
@@ -1205,17 +1205,18 @@
/* Disabled && !changed text => skip validation */
if (hdr->tci_an & MACSEC_TCI_C ||
- secy->validate_frames != MACSEC_VALIDATE_DISABLED)
+ secy->validate_frames != MACSEC_VALIDATE_DISABLED) {
skb = macsec_decrypt(skb, dev, rx_sa, sci, secy);
- if (IS_ERR(skb)) {
- /* the decrypt callback needs the reference */
- if (PTR_ERR(skb) != -EINPROGRESS) {
- macsec_rxsa_put(rx_sa);
- macsec_rxsc_put(rx_sc);
+ if (IS_ERR(skb)) {
+ /* the decrypt callback needs the reference */
+ if (PTR_ERR(skb) != -EINPROGRESS) {
+ macsec_rxsa_put(rx_sa);
+ macsec_rxsc_put(rx_sc);
+ }
+ rcu_read_unlock();
+ return RX_HANDLER_CONSUMED;
}
- rcu_read_unlock();
- return RX_HANDLER_CONSUMED;
}
if (!macsec_post_decrypt(skb, secy, pn))
^ permalink raw reply
* RE: [PATCH net-next 1/3] devlink: Introduce PCI PF port flavour and port attribute
From: Parav Pandit @ 2019-07-02 4:26 UTC (permalink / raw)
To: Jakub Kicinski, Jiri Pirko; +Cc: netdev@vger.kernel.org, Saeed Mahameed
In-Reply-To: <20190701162650.17854185@cakuba.netronome.com>
Hi Jakub,
> -----Original Message-----
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Sent: Tuesday, July 2, 2019 4:57 AM
> To: Jiri Pirko <jiri@mellanox.com>
> Cc: Parav Pandit <parav@mellanox.com>; netdev@vger.kernel.org; Saeed
> Mahameed <saeedm@mellanox.com>
> Subject: Re: [PATCH net-next 1/3] devlink: Introduce PCI PF port flavour and
> port attribute
>
> On Mon, 1 Jul 2019 07:27:32 -0500, Parav Pandit wrote:
> > In an eswitch, PCI PF may have port which is normally represented
> > using a representor netdevice.
> > To have better visibility of eswitch port, its association with PF, a
> > representor netdevice and port number, introduce a PCI PF port flavour
> > and port attriute.
> >
> > When devlink port flavour is PCI PF, fill up PCI PF attributes of the
> > port.
> >
> > Extend port name creation using PCI PF number on best effort basis.
> > So that vendor drivers can skip defining their own scheme.
> >
> > $ devlink port show
> > pci/0000:05:00.0/0: type eth netdev eth0 flavour pcipf pfnum 0
> >
> > Acked-by: Jiri Pirko <jiri@mellanox.com>
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > ---
> > include/net/devlink.h | 11 ++++++
> > include/uapi/linux/devlink.h | 5 +++
> > net/core/devlink.c | 71 +++++++++++++++++++++++++++++-------
> > 3 files changed, 73 insertions(+), 14 deletions(-)
> >
> > diff --git a/include/net/devlink.h b/include/net/devlink.h index
> > 6625ea068d5e..8db9c0e83fb5 100644
> > --- a/include/net/devlink.h
> > +++ b/include/net/devlink.h
> > @@ -38,6 +38,10 @@ struct devlink {
> > char priv[0] __aligned(NETDEV_ALIGN); };
> >
> > +struct devlink_port_pci_pf_attrs {
>
> Why the named structure? Anonymous one should be just fine?
>
No specific reason for this patch. But named structure allows to extend it more easily with code readability.
Such as subsequently we want to add the peer_mac etc port attributes.
Named structure to store those attributes are helpful.
> > + u16 pf; /* Associated PCI PF for this port. */
> > +};
> > +
> > struct devlink_port_attrs {
> > u8 set:1,
> > split:1,
> > @@ -46,6 +50,9 @@ struct devlink_port_attrs {
> > u32 port_number; /* same value as "split group" */
> > u32 split_subport_number;
> > struct netdev_phys_item_id switch_id;
> > + union {
> > + struct devlink_port_pci_pf_attrs pci_pf;
> > + };
> > };
> >
> > struct devlink_port {
> > @@ -590,6 +597,10 @@ void devlink_port_attrs_set(struct devlink_port
> *devlink_port,
> > u32 split_subport_number,
> > const unsigned char *switch_id,
> > unsigned char switch_id_len);
> > +void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
> > + u32 port_number,
> > + const unsigned char *switch_id,
> > + unsigned char switch_id_len, u16 pf);
> > int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
> > u32 size, u16 ingress_pools_count,
> > u16 egress_pools_count, u16 ingress_tc_count, diff --
> git
> > a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h index
> > 5287b42c181f..f7323884c3fe 100644
> > --- a/include/uapi/linux/devlink.h
> > +++ b/include/uapi/linux/devlink.h
> > @@ -169,6 +169,10 @@ enum devlink_port_flavour {
> > DEVLINK_PORT_FLAVOUR_DSA, /* Distributed switch architecture
> > * interconnect port.
> > */
> > + DEVLINK_PORT_FLAVOUR_PCI_PF, /* Represents eswitch port for
> > + * the PCI PF. It is an internal
> > + * port that faces the PCI PF.
> > + */
> > };
> >
> > enum devlink_param_cmode {
> > @@ -337,6 +341,7 @@ enum devlink_attr {
> > DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE, /* u64 */
> > DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL, /* u64 */
> >
> > + DEVLINK_ATTR_PORT_PCI_PF_NUMBER, /* u16 */
> > /* add new attributes above here, update the policy in devlink.c */
> >
> > __DEVLINK_ATTR_MAX,
> > diff --git a/net/core/devlink.c b/net/core/devlink.c index
> > 89c533778135..001f9e2c96f0 100644
> > --- a/net/core/devlink.c
> > +++ b/net/core/devlink.c
> > @@ -517,6 +517,11 @@ static int devlink_nl_port_attrs_put(struct sk_buff
> *msg,
> > return -EMSGSIZE;
> > if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER, attrs-
> >port_number))
> > return -EMSGSIZE;
>
> Why would we report network port information for PF and VF port flavours?
I didn't see any immediate need to report, at the same time didn't find any reason to treat such port flavours differently than existing one.
It just gives a clear view of the device's eswitch.
Might find it useful during debugging while inspecting device internal tables..
>
> > + if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
> > + if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
> > + attrs->pci_pf.pf))
> > + return -EMSGSIZE;
> > + }
> > if (!attrs->split)
> > return 0;
> > if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP,
> > attrs->port_number))
^ permalink raw reply
* Re: [PATCH net 2/2] macsec: fix checksumming after decryption
From: Andreas Steinmetz @ 2019-07-02 4:24 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, Sabrina Dubroca
In-Reply-To: <CA+FuTSdzM3AFFrvANczVzXeRP0TVZ06K--GkmTZVAk-6SKQGxA@mail.gmail.com>
On Sun, 2019-06-30 at 21:47 -0400, Willem de Bruijn wrote:
> On Sun, Jun 30, 2019 at 4:48 PM Andreas Steinmetz <ast@domdv.de>
> wrote:
> > Fix checksumming after decryption.
> >
> > Signed-off-by: Andreas Steinmetz <ast@domdv.de>
> >
> > --- a/drivers/net/macsec.c 2019-06-30 22:14:10.250285314 +0200
> > +++ b/drivers/net/macsec.c 2019-06-30 22:15:11.931230417 +0200
> > @@ -869,6 +869,7 @@
> >
> > static void macsec_finalize_skb(struct sk_buff *skb, u8 icv_len,
> > u8 hdr_len)
> > {
> > + skb->ip_summed = CHECKSUM_NONE;
> > memmove(skb->data + hdr_len, skb->data, 2 * ETH_ALEN);
> > skb_pull(skb, hdr_len);
> > pskb_trim_unique(skb, skb->len - icv_len);
>
> Does this belong in macset_reset_skb?
Putting this in macsec_reset_skb would then miss out the "nosci:" part
of the RX path in macsec_handle_frame().
^ permalink raw reply
* Re: [PATCH bpf-next v2] virtio_net: add XDP meta data support
From: Jason Wang @ 2019-07-02 3:59 UTC (permalink / raw)
To: Yuya Kusakabe
Cc: ast, daniel, davem, hawk, jakub.kicinski, john.fastabend, kafai,
mst, netdev, songliubraving, yhs
In-Reply-To: <20190702031542.5096-1-yuya.kusakabe@gmail.com>
On 2019/7/2 上午11:15, Yuya Kusakabe wrote:
> This adds XDP meta data support to both receive_small() and
> receive_mergeable().
>
> Fixes: de8f3a83b0a0 ("bpf: add meta pointer for direct access")
> Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
> ---
> v2:
> - keep copy untouched in page_to_skb().
> - preserve the vnet header in receive_small().
> - fix indentation.
> ---
> drivers/net/virtio_net.c | 39 +++++++++++++++++++++++++++------------
> 1 file changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 4f3de0ac8b0b..2ebabb08b824 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -371,7 +371,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> struct receive_queue *rq,
> struct page *page, unsigned int offset,
> unsigned int len, unsigned int truesize,
> - bool hdr_valid)
> + bool hdr_valid, unsigned int metasize)
> {
> struct sk_buff *skb;
> struct virtio_net_hdr_mrg_rxbuf *hdr;
> @@ -393,7 +393,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> else
> hdr_padded_len = sizeof(struct padded_vnet_hdr);
>
> - if (hdr_valid)
> + if (hdr_valid && !metasize)
> memcpy(hdr, p, hdr_len);
>
> len -= hdr_len;
> @@ -405,6 +405,11 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> copy = skb_tailroom(skb);
> skb_put_data(skb, p, copy);
>
> + if (metasize) {
> + __skb_pull(skb, metasize);
> + skb_metadata_set(skb, metasize);
> + }
> +
> len -= copy;
> offset += copy;
>
> @@ -644,6 +649,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
> unsigned int delta = 0;
> struct page *xdp_page;
> int err;
> + unsigned int metasize = 0;
>
> len -= vi->hdr_len;
> stats->bytes += len;
> @@ -683,8 +689,8 @@ static struct sk_buff *receive_small(struct net_device *dev,
>
> xdp.data_hard_start = buf + VIRTNET_RX_PAD + vi->hdr_len;
> xdp.data = xdp.data_hard_start + xdp_headroom;
> - xdp_set_data_meta_invalid(&xdp);
> xdp.data_end = xdp.data + len;
> + xdp.data_meta = xdp.data;
> xdp.rxq = &rq->xdp_rxq;
> orig_data = xdp.data;
> act = bpf_prog_run_xdp(xdp_prog, &xdp);
> @@ -695,9 +701,11 @@ static struct sk_buff *receive_small(struct net_device *dev,
> /* Recalculate length in case bpf program changed it */
> delta = orig_data - xdp.data;
> len = xdp.data_end - xdp.data;
> + metasize = xdp.data - xdp.data_meta;
> break;
> case XDP_TX:
> stats->xdp_tx++;
> + xdp.data_meta = xdp.data;
> xdpf = convert_to_xdp_frame(&xdp);
> if (unlikely(!xdpf))
> goto err_xdp;
> @@ -740,6 +748,9 @@ static struct sk_buff *receive_small(struct net_device *dev,
> memcpy(skb_vnet_hdr(skb), buf, vi->hdr_len);
> } /* keep zeroed vnet hdr since packet was changed by bpf */
I wonder whether or not it's as simple as this. Consider bpf may adjust
meta, it looks to me that the vnet header will be overwrite here? If
yes, we probably need to have a device specific value then bpf can move
the device metadata like vnet header for us?
Thanks
>
> + if (metasize)
> + skb_metadata_set(skb, metasize);
> +
> err:
> return skb;
>
> @@ -760,8 +771,8 @@ static struct sk_buff *receive_big(struct net_device *dev,
> struct virtnet_rq_stats *stats)
> {
> struct page *page = buf;
> - struct sk_buff *skb = page_to_skb(vi, rq, page, 0, len,
> - PAGE_SIZE, true);
> + struct sk_buff *skb =
> + page_to_skb(vi, rq, page, 0, len, PAGE_SIZE, true, 0);
>
> stats->bytes += len - vi->hdr_len;
> if (unlikely(!skb))
> @@ -793,6 +804,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
> unsigned int truesize;
> unsigned int headroom = mergeable_ctx_to_headroom(ctx);
> int err;
> + unsigned int metasize = 0;
>
> head_skb = NULL;
> stats->bytes += len - vi->hdr_len;
> @@ -839,8 +851,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
> data = page_address(xdp_page) + offset;
> xdp.data_hard_start = data - VIRTIO_XDP_HEADROOM + vi->hdr_len;
> xdp.data = data + vi->hdr_len;
> - xdp_set_data_meta_invalid(&xdp);
> xdp.data_end = xdp.data + (len - vi->hdr_len);
> + xdp.data_meta = xdp.data;
> xdp.rxq = &rq->xdp_rxq;
>
> act = bpf_prog_run_xdp(xdp_prog, &xdp);
> @@ -852,8 +864,9 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
> * adjustments. Note other cases do not build an
> * skb and avoid using offset
> */
> - offset = xdp.data -
> - page_address(xdp_page) - vi->hdr_len;
> + metasize = xdp.data - xdp.data_meta;
> + offset = xdp.data - page_address(xdp_page) -
> + vi->hdr_len - metasize;
>
> /* recalculate len if xdp.data or xdp.data_end were
> * adjusted
> @@ -863,14 +876,15 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
> if (unlikely(xdp_page != page)) {
> rcu_read_unlock();
> put_page(page);
> - head_skb = page_to_skb(vi, rq, xdp_page,
> - offset, len,
> - PAGE_SIZE, false);
> + head_skb = page_to_skb(vi, rq, xdp_page, offset,
> + len, PAGE_SIZE, false,
> + metasize);
> return head_skb;
> }
> break;
> case XDP_TX:
> stats->xdp_tx++;
> + xdp.data_meta = xdp.data;
> xdpf = convert_to_xdp_frame(&xdp);
> if (unlikely(!xdpf))
> goto err_xdp;
> @@ -921,7 +935,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
> goto err_skb;
> }
>
> - head_skb = page_to_skb(vi, rq, page, offset, len, truesize, !xdp_prog);
> + head_skb = page_to_skb(vi, rq, page, offset, len, truesize, !xdp_prog,
> + metasize);
> curr_skb = head_skb;
>
> if (unlikely(!curr_skb))
^ permalink raw reply
* [Patch net] bonding: validate ip header before check IPPROTO_IGMP
From: Cong Wang @ 2019-07-02 3:40 UTC (permalink / raw)
To: netdev
Cc: Cong Wang, syzbot+e5be16aa39ad6e755391, Jay Vosburgh,
Veaceslav Falico, Andy Gospodarek
bond_xmit_roundrobin() checks for IGMP packets but it parses
the IP header even before checking skb->protocol.
We should validate the IP header with pskb_may_pull() before
using iph->protocol.
Reported-and-tested-by: syzbot+e5be16aa39ad6e755391@syzkaller.appspotmail.com
Fixes: a2fd940f4cff ("bonding: fix broken multicast with round-robin mode")
Cc: Jay Vosburgh <j.vosburgh@gmail.com>
Cc: Veaceslav Falico <vfalico@gmail.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
drivers/net/bonding/bond_main.c | 37 ++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 799fc38c5c34..b0aab3a0a1bf 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3866,8 +3866,8 @@ static netdev_tx_t bond_xmit_roundrobin(struct sk_buff *skb,
struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
- struct iphdr *iph = ip_hdr(skb);
struct slave *slave;
+ int slave_cnt;
u32 slave_id;
/* Start with the curr_active_slave that joined the bond as the
@@ -3876,23 +3876,32 @@ static netdev_tx_t bond_xmit_roundrobin(struct sk_buff *skb,
* send the join/membership reports. The curr_active_slave found
* will send all of this type of traffic.
*/
- if (iph->protocol == IPPROTO_IGMP && skb->protocol == htons(ETH_P_IP)) {
- slave = rcu_dereference(bond->curr_active_slave);
- if (slave)
- bond_dev_queue_xmit(bond, skb, slave->dev);
- else
- bond_xmit_slave_id(bond, skb, 0);
- } else {
- int slave_cnt = READ_ONCE(bond->slave_cnt);
+ if (skb->protocol == htons(ETH_P_IP)) {
+ int noff = skb_network_offset(skb);
+ struct iphdr *iph;
- if (likely(slave_cnt)) {
- slave_id = bond_rr_gen_slave_id(bond);
- bond_xmit_slave_id(bond, skb, slave_id % slave_cnt);
- } else {
- bond_tx_drop(bond_dev, skb);
+ if (unlikely(!pskb_may_pull(skb, noff + sizeof(*iph))))
+ goto non_igmp;
+
+ iph = ip_hdr(skb);
+ if (iph->protocol == IPPROTO_IGMP) {
+ slave = rcu_dereference(bond->curr_active_slave);
+ if (slave)
+ bond_dev_queue_xmit(bond, skb, slave->dev);
+ else
+ bond_xmit_slave_id(bond, skb, 0);
+ return NETDEV_TX_OK;
}
}
+non_igmp:
+ slave_cnt = READ_ONCE(bond->slave_cnt);
+ if (likely(slave_cnt)) {
+ slave_id = bond_rr_gen_slave_id(bond);
+ bond_xmit_slave_id(bond, skb, slave_id % slave_cnt);
+ } else {
+ bond_tx_drop(bond_dev, skb);
+ }
return NETDEV_TX_OK;
}
--
2.21.0
^ permalink raw reply related
* [PATCH] fix noderef.cocci warnings
From: kbuild test robot @ 2019-07-02 3:33 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: kbuild-all, Martin Weinelt, bridge, Roopa Prabhu, netdev
In-Reply-To: <908e9e90-70cc-7bbe-f83f-0810c9ef3925@cumulusnetworks.com>
From: kbuild test robot <lkp@intel.com>
net/bridge/br_multicast.c:999:8-14: ERROR: application of sizeof to pointer
sizeof when applied to a pointer typed expression gives the size of
the pointer
Generated by: scripts/coccinelle/misc/noderef.cocci
Fixes: 17c91348ed8b ("Use-after-free in br_multicast_rcv")
CC: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Signed-off-by: kbuild test robot <lkp@intel.com>
---
url: https://github.com/0day-ci/linux/commits/Nikolay-Aleksandrov/net-bridge-mcast-fix-possible-uses-of-stale-pointers/20190702-083354
br_multicast.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -996,7 +996,7 @@ static int br_ip6_multicast_mld2_report(
return -EINVAL;
_nsrcs = skb_header_pointer(skb, nsrcs_offset,
- sizeof(_nsrcs), &__nsrcs);
+ sizeof(*_nsrcs), &__nsrcs);
if (!_nsrcs)
return -EINVAL;
^ permalink raw reply
* Re: [PATCH TEST] net: bridge: mcast: fix possible uses of stale pointers
From: kbuild test robot @ 2019-07-02 3:33 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: kbuild-all, Martin Weinelt, bridge, Roopa Prabhu, netdev
In-Reply-To: <908e9e90-70cc-7bbe-f83f-0810c9ef3925@cumulusnetworks.com>
Hi Nikolay,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net/master]
[also build test WARNING on v5.2-rc6 next-20190625]
[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/Nikolay-Aleksandrov/net-bridge-mcast-fix-possible-uses-of-stale-pointers/20190702-083354
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
coccinelle warnings: (new ones prefixed by >>)
>> net/bridge/br_multicast.c:999:8-14: ERROR: application of sizeof to pointer
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH 0/3, net-next, v2] net: netsec: Add XDP Support
From: Ilias Apalodimas @ 2019-07-02 3:18 UTC (permalink / raw)
To: David Miller
Cc: netdev, jaswinder.singh, ard.biesheuvel, bjorn.topel,
magnus.karlsson, brouer, daniel, ast, makita.toshiaki,
jakub.kicinski, john.fastabend, maciejromanfijalkowski
In-Reply-To: <20190701.192733.26575663343081553.davem@davemloft.net>
Hi David
[...]
>
> Series applied, thanks.
>
> I realize from the discussion on patch #3 there will be follow-ups to this.
Yea, small cosmetic changes. I'll send them shortly
Thanks
/Ilias
^ permalink raw reply
* [PATCH bpf-next v2] virtio_net: add XDP meta data support
From: Yuya Kusakabe @ 2019-07-02 3:15 UTC (permalink / raw)
To: yuya.kusakabe
Cc: ast, daniel, davem, hawk, jakub.kicinski, jasowang,
john.fastabend, kafai, mst, netdev, songliubraving, yhs
In-Reply-To: <455941f9-aadb-ab70-2745-34f8fd893e89@gmail.com>
This adds XDP meta data support to both receive_small() and
receive_mergeable().
Fixes: de8f3a83b0a0 ("bpf: add meta pointer for direct access")
Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
---
v2:
- keep copy untouched in page_to_skb().
- preserve the vnet header in receive_small().
- fix indentation.
---
drivers/net/virtio_net.c | 39 +++++++++++++++++++++++++++------------
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 4f3de0ac8b0b..2ebabb08b824 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -371,7 +371,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
struct receive_queue *rq,
struct page *page, unsigned int offset,
unsigned int len, unsigned int truesize,
- bool hdr_valid)
+ bool hdr_valid, unsigned int metasize)
{
struct sk_buff *skb;
struct virtio_net_hdr_mrg_rxbuf *hdr;
@@ -393,7 +393,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
else
hdr_padded_len = sizeof(struct padded_vnet_hdr);
- if (hdr_valid)
+ if (hdr_valid && !metasize)
memcpy(hdr, p, hdr_len);
len -= hdr_len;
@@ -405,6 +405,11 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
copy = skb_tailroom(skb);
skb_put_data(skb, p, copy);
+ if (metasize) {
+ __skb_pull(skb, metasize);
+ skb_metadata_set(skb, metasize);
+ }
+
len -= copy;
offset += copy;
@@ -644,6 +649,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
unsigned int delta = 0;
struct page *xdp_page;
int err;
+ unsigned int metasize = 0;
len -= vi->hdr_len;
stats->bytes += len;
@@ -683,8 +689,8 @@ static struct sk_buff *receive_small(struct net_device *dev,
xdp.data_hard_start = buf + VIRTNET_RX_PAD + vi->hdr_len;
xdp.data = xdp.data_hard_start + xdp_headroom;
- xdp_set_data_meta_invalid(&xdp);
xdp.data_end = xdp.data + len;
+ xdp.data_meta = xdp.data;
xdp.rxq = &rq->xdp_rxq;
orig_data = xdp.data;
act = bpf_prog_run_xdp(xdp_prog, &xdp);
@@ -695,9 +701,11 @@ static struct sk_buff *receive_small(struct net_device *dev,
/* Recalculate length in case bpf program changed it */
delta = orig_data - xdp.data;
len = xdp.data_end - xdp.data;
+ metasize = xdp.data - xdp.data_meta;
break;
case XDP_TX:
stats->xdp_tx++;
+ xdp.data_meta = xdp.data;
xdpf = convert_to_xdp_frame(&xdp);
if (unlikely(!xdpf))
goto err_xdp;
@@ -740,6 +748,9 @@ static struct sk_buff *receive_small(struct net_device *dev,
memcpy(skb_vnet_hdr(skb), buf, vi->hdr_len);
} /* keep zeroed vnet hdr since packet was changed by bpf */
+ if (metasize)
+ skb_metadata_set(skb, metasize);
+
err:
return skb;
@@ -760,8 +771,8 @@ static struct sk_buff *receive_big(struct net_device *dev,
struct virtnet_rq_stats *stats)
{
struct page *page = buf;
- struct sk_buff *skb = page_to_skb(vi, rq, page, 0, len,
- PAGE_SIZE, true);
+ struct sk_buff *skb =
+ page_to_skb(vi, rq, page, 0, len, PAGE_SIZE, true, 0);
stats->bytes += len - vi->hdr_len;
if (unlikely(!skb))
@@ -793,6 +804,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
unsigned int truesize;
unsigned int headroom = mergeable_ctx_to_headroom(ctx);
int err;
+ unsigned int metasize = 0;
head_skb = NULL;
stats->bytes += len - vi->hdr_len;
@@ -839,8 +851,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
data = page_address(xdp_page) + offset;
xdp.data_hard_start = data - VIRTIO_XDP_HEADROOM + vi->hdr_len;
xdp.data = data + vi->hdr_len;
- xdp_set_data_meta_invalid(&xdp);
xdp.data_end = xdp.data + (len - vi->hdr_len);
+ xdp.data_meta = xdp.data;
xdp.rxq = &rq->xdp_rxq;
act = bpf_prog_run_xdp(xdp_prog, &xdp);
@@ -852,8 +864,9 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
* adjustments. Note other cases do not build an
* skb and avoid using offset
*/
- offset = xdp.data -
- page_address(xdp_page) - vi->hdr_len;
+ metasize = xdp.data - xdp.data_meta;
+ offset = xdp.data - page_address(xdp_page) -
+ vi->hdr_len - metasize;
/* recalculate len if xdp.data or xdp.data_end were
* adjusted
@@ -863,14 +876,15 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
if (unlikely(xdp_page != page)) {
rcu_read_unlock();
put_page(page);
- head_skb = page_to_skb(vi, rq, xdp_page,
- offset, len,
- PAGE_SIZE, false);
+ head_skb = page_to_skb(vi, rq, xdp_page, offset,
+ len, PAGE_SIZE, false,
+ metasize);
return head_skb;
}
break;
case XDP_TX:
stats->xdp_tx++;
+ xdp.data_meta = xdp.data;
xdpf = convert_to_xdp_frame(&xdp);
if (unlikely(!xdpf))
goto err_xdp;
@@ -921,7 +935,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
goto err_skb;
}
- head_skb = page_to_skb(vi, rq, page, offset, len, truesize, !xdp_prog);
+ head_skb = page_to_skb(vi, rq, page, offset, len, truesize, !xdp_prog,
+ metasize);
curr_skb = head_skb;
if (unlikely(!curr_skb))
--
2.20.1
^ permalink raw reply related
* Re: [Patch net 0/3] idr: fix overflow cases on 32-bit CPU
From: Cong Wang @ 2019-07-02 3:12 UTC (permalink / raw)
To: Matthew Wilcox
Cc: David Miller, Linux Kernel Network Developers, Davide Caratti,
Chris Mi
In-Reply-To: <20190702023730.GA1729@bombadil.infradead.org>
On Mon, Jul 1, 2019 at 7:37 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, Jul 01, 2019 at 07:16:00PM -0700, David Miller wrote:
> > From: Cong Wang <xiyou.wangcong@gmail.com>
> > Date: Fri, 28 Jun 2019 11:03:40 -0700
> >
> > > idr_get_next_ul() is problematic by design, it can't handle
> > > the following overflow case well on 32-bit CPU:
> > >
> > > u32 id = UINT_MAX;
> > > idr_alloc_u32(&id);
> > > while (idr_get_next_ul(&id) != NULL)
> > > id++;
> > >
> > > when 'id' overflows and becomes 0 after UINT_MAX, the loop
> > > goes infinite.
> > >
> > > Fix this by eliminating external users of idr_get_next_ul()
> > > and migrating them to idr_for_each_entry_continue_ul(). And
> > > add an additional parameter for these iteration macros to detect
> > > overflow properly.
> > >
> > > Please merge this through networking tree, as all the users
> > > are in networking subsystem.
> >
> > Series applied, thanks Cong.
>
> Ugh, I don't even get the weekend to reply?
>
> I think this is just a bad idea. It'd be better to apply the conversion
> patches to use XArray than fix up this crappy interface. I didn't
> reply before because I wanted to check those patches still apply and
> post them as part of the response. Now they're definitely broken and
> need to be redone.
You can always do refactoring for net-next/linux-next. It is never late
for it.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next v4 1/4] gve: Add basic driver framework for Compute Engine Virtual NIC
From: Jakub Kicinski @ 2019-07-02 2:47 UTC (permalink / raw)
To: Catherine Sullivan
Cc: netdev, Sagi Shahar, Jon Olson, Willem de Bruijn, Luigi Rizzo
In-Reply-To: <20190701225755.209250-2-csully@google.com>
On Mon, 1 Jul 2019 15:57:52 -0700, Catherine Sullivan wrote:
> Add a driver framework for the Compute Engine Virtual NIC that will be
> available in the future.
>
> At this point the only functionality is loading the driver.
>
> Signed-off-by: Catherine Sullivan <csully@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> Signed-off-by: Jon Olson <jonolson@google.com>
> Acked-by: Willem de Bruijn <willemb@google.com>
> Reviewed-by: Luigi Rizzo <lrizzo@google.com>
> +Admin Queue (AQ)
> +----------------
> +The Admin Queue is a PAGE_SIZE memory block, treated as an array of AQ
> +commands, used by the driver to issue commands to the device and set up
> +resources.The driver and the device maintain a count of how many commands
> +have been submitted and executed. To issue AQ commands, the driver must do
> +the following (with proper locking):
> +
> +1) Copy new commands into next available slots in the AQ array
> +2) Increment its counter by he number of new commands
s/he/the/
> +3) Write the counter into the GVE_ADMIN_QUEUE_DOORBELL register
> +4) Poll the ADMIN_QUEUE_EVENT_COUNTER register until it equals
> + the value written to the doorbell, or until a timeout.
> +
> +The device will update the status field in each AQ command reported as
> +executed through the ADMIN_QUEUE_EVENT_COUNTER register.
> +
> +/* This function is not threadsafe - the caller is responsible for any
> + * necessary locks.
> + */
> +int gve_adminq_execute_cmd(struct gve_priv *priv,
> + union gve_adminq_command *cmd_orig)
> +{
> + union gve_adminq_command *cmd;
> + u32 status = 0;
> + u32 prod_cnt;
> +
> + cmd = &priv->adminq[priv->adminq_prod_cnt & priv->adminq_mask];
> + priv->adminq_prod_cnt++;
> + prod_cnt = priv->adminq_prod_cnt;
> +
> + memcpy(cmd, cmd_orig, sizeof(*cmd_orig));
Eh, I guess you don't even need memory barriers around the data movement
to the DMA buffer because of the restriction to x86? :)
> + gve_adminq_kick_cmd(priv, prod_cnt);
> + if (!gve_adminq_wait_for_cmd(priv, prod_cnt)) {
> + dev_err(&priv->pdev->dev, "AQ command timed out, need to reset AQ\n");
> + return -ENOTRECOVERABLE;
> + }
> + memcpy(cmd_orig, cmd, sizeof(*cmd));
> + status = be32_to_cpu(READ_ONCE(cmd->status));
> + return gve_adminq_parse_err(&priv->pdev->dev, status);
> +}
> +int gve_adminq_describe_device(struct gve_priv *priv)
> +{
> + struct gve_device_descriptor *descriptor;
> + union gve_adminq_command cmd;
> + dma_addr_t descriptor_bus;
> + int err = 0;
> + u8 *mac;
> + u16 mtu;
> +
> + memset(&cmd, 0, sizeof(cmd));
> + descriptor = dma_alloc_coherent(&priv->pdev->dev, PAGE_SIZE,
> + &descriptor_bus, GFP_KERNEL);
> + if (!descriptor)
> + return -ENOMEM;
> + cmd.opcode = cpu_to_be32(GVE_ADMINQ_DESCRIBE_DEVICE);
> + cmd.describe_device.device_descriptor_addr =
> + cpu_to_be64(descriptor_bus);
> + cmd.describe_device.device_descriptor_version =
> + cpu_to_be32(GVE_ADMINQ_DEVICE_DESCRIPTOR_VERSION);
> + cmd.describe_device.available_length = cpu_to_be32(PAGE_SIZE);
> +
> + err = gve_adminq_execute_cmd(priv, &cmd);
> + if (err)
> + goto free_device_descriptor;
> +
> + mtu = be16_to_cpu(descriptor->mtu);
> + if (mtu < ETH_MIN_MTU) {
> + netif_err(priv, drv, priv->dev, "MTU %d below minimum MTU\n",
> + mtu);
> + err = -EINVAL;
> + goto free_device_descriptor;
> + }
> + priv->dev->max_mtu = mtu;
> + priv->num_event_counters = be16_to_cpu(descriptor->counters);
> + ether_addr_copy(priv->dev->dev_addr, descriptor->mac);
Since you check MTU you can check the address with
is_valid_ether_addr().
Also it's common practice to copy the provisioned MAC to
netdev->perm_addr as well as dev_addr for VFs.
> + mac = descriptor->mac;
> + netif_info(priv, drv, priv->dev, "MAC addr: %pM\n", mac);
> +
> +free_device_descriptor:
> + dma_free_coherent(&priv->pdev->dev, sizeof(*descriptor), descriptor,
> + descriptor_bus);
> + return err;
> +}
Oh, okay, David already applied..
^ permalink raw reply
* Re: [Patch net 0/3] idr: fix overflow cases on 32-bit CPU
From: David Miller @ 2019-07-02 2:39 UTC (permalink / raw)
To: willy; +Cc: xiyou.wangcong, netdev, dcaratti, chrism
In-Reply-To: <20190702023730.GA1729@bombadil.infradead.org>
From: Matthew Wilcox <willy@infradead.org>
Date: Mon, 1 Jul 2019 19:37:30 -0700
> On Mon, Jul 01, 2019 at 07:16:00PM -0700, David Miller wrote:
>> From: Cong Wang <xiyou.wangcong@gmail.com>
>> Date: Fri, 28 Jun 2019 11:03:40 -0700
>>
>> > idr_get_next_ul() is problematic by design, it can't handle
>> > the following overflow case well on 32-bit CPU:
>> >
>> > u32 id = UINT_MAX;
>> > idr_alloc_u32(&id);
>> > while (idr_get_next_ul(&id) != NULL)
>> > id++;
>> >
>> > when 'id' overflows and becomes 0 after UINT_MAX, the loop
>> > goes infinite.
>> >
>> > Fix this by eliminating external users of idr_get_next_ul()
>> > and migrating them to idr_for_each_entry_continue_ul(). And
>> > add an additional parameter for these iteration macros to detect
>> > overflow properly.
>> >
>> > Please merge this through networking tree, as all the users
>> > are in networking subsystem.
>>
>> Series applied, thanks Cong.
>
> Ugh, I don't even get the weekend to reply?
>
> I think this is just a bad idea. It'd be better to apply the conversion
> patches to use XArray than fix up this crappy interface. I didn't
> reply before because I wanted to check those patches still apply and
> post them as part of the response. Now they're definitely broken and
> need to be redone.
Please work this out with Cong.
I think his approach is safe for net and thus -stable than an xarray
conversion.
^ permalink raw reply
* Re: [Patch net 0/3] idr: fix overflow cases on 32-bit CPU
From: Matthew Wilcox @ 2019-07-02 2:37 UTC (permalink / raw)
To: David Miller; +Cc: xiyou.wangcong, netdev, dcaratti, chrism
In-Reply-To: <20190701.191600.1570499492484804858.davem@davemloft.net>
On Mon, Jul 01, 2019 at 07:16:00PM -0700, David Miller wrote:
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Fri, 28 Jun 2019 11:03:40 -0700
>
> > idr_get_next_ul() is problematic by design, it can't handle
> > the following overflow case well on 32-bit CPU:
> >
> > u32 id = UINT_MAX;
> > idr_alloc_u32(&id);
> > while (idr_get_next_ul(&id) != NULL)
> > id++;
> >
> > when 'id' overflows and becomes 0 after UINT_MAX, the loop
> > goes infinite.
> >
> > Fix this by eliminating external users of idr_get_next_ul()
> > and migrating them to idr_for_each_entry_continue_ul(). And
> > add an additional parameter for these iteration macros to detect
> > overflow properly.
> >
> > Please merge this through networking tree, as all the users
> > are in networking subsystem.
>
> Series applied, thanks Cong.
Ugh, I don't even get the weekend to reply?
I think this is just a bad idea. It'd be better to apply the conversion
patches to use XArray than fix up this crappy interface. I didn't
reply before because I wanted to check those patches still apply and
post them as part of the response. Now they're definitely broken and
need to be redone.
^ permalink raw reply
* Re: [PATCH net-next v4 0/4] Add gve driver
From: David Miller @ 2019-07-02 2:36 UTC (permalink / raw)
To: csully; +Cc: netdev, lkp, julia.lawall
In-Reply-To: <20190701225755.209250-1-csully@google.com>
From: Catherine Sullivan <csully@google.com>
Date: Mon, 1 Jul 2019 15:57:51 -0700
> This patch series adds the gve driver which will support the
> Compute Engine Virtual NIC that will be available in the future.
...
Series applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCHv3 next 0/3] blackhole device to invalidate dst
From: David Miller @ 2019-07-02 2:35 UTC (permalink / raw)
To: maheshb; +Cc: netdev, edumazet, michael.chan, dja, mahesh
In-Reply-To: <20190701213843.102002-1-maheshb@google.com>
From: Mahesh Bandewar <maheshb@google.com>
Date: Mon, 1 Jul 2019 14:38:43 -0700
...
> The idea here is to not alter the data-path with additional
> locks or smb()/rmb() barriers to avoid racy assignments but
> to create a new device that has really low MTU that has
> .ndo_start_xmit essentially a kfree_skb(). Make use of this
> device instead of 'lo' when marking the dst dead.
>
> First patch implements the blackhole device and second
> patch uses it in IPv4 and IPv6 stack while the third patch
> is the self test that ensures the sanity of this device.
...
Series applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH] net: ethernet: broadcom: bcm63xx_enet: Remove unneeded memset
From: David Miller @ 2019-07-02 2:29 UTC (permalink / raw)
To: hariprasad.kelam
Cc: f.fainelli, bcm-kernel-feedback-list, andrew, gregkh, yuehaibing,
tglx, mcgrof, netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <20190630142949.GA7422@hari-Inspiron-1545>
From: Hariprasad Kelam <hariprasad.kelam@gmail.com>
Date: Sun, 30 Jun 2019 19:59:49 +0530
> Remove unneeded memset as alloc_etherdev is using kvzalloc which uses
> __GFP_ZERO flag
>
> Signed-off-by: Hariprasad Kelam <hariprasad.kelam@gmail.com>
Applied to net-next, thanks.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox