* 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
* 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: santosh.shilimkar @ 2019-07-02 2:28 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <14c34ac2-38ed-9d51-f27d-74120ff34c54@oracle.com>
On 7/1/19 2:06 PM, Gerd Rausch wrote:
> Hi Santosh,
>
> On 01/07/2019 14.00, santosh.shilimkar@oracle.com wrote:
>>>
>> Look for command timeout in CX3 sources. 60 second is upper bound in
>> CX3. Its not standard in specs(at least not that I know) though
>> and may vary from vendor to vendor.
>>
>
> I am not seeing it. Can you point me to the right place?
>
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,
};
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.
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
There is a socket call RDS_GET_MR which needs to be synchronous
and that Avinash has actually fixed by making this MR registration
processes synchronous. Inline registration is still kept async.
RDS_GET_MR case is what actually showing the issue you saw
and the fix for that Avinash has it in production kernel.
I believe with that change, registration issue becomes non-issue
already.
And as far as invalidation concerned with proxy qp, it not longer
races with data path qp.
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.
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH 0/3, net-next, v2] net: netsec: Add XDP Support
From: David Miller @ 2019-07-02 2:27 UTC (permalink / raw)
To: ilias.apalodimas
Cc: netdev, jaswinder.singh, ard.biesheuvel, bjorn.topel,
magnus.karlsson, brouer, daniel, ast, makita.toshiaki,
jakub.kicinski, john.fastabend, maciejromanfijalkowski
In-Reply-To: <1561785805-21647-1-git-send-email-ilias.apalodimas@linaro.org>
From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Date: Sat, 29 Jun 2019 08:23:22 +0300
> This is a respin of https://www.spinics.net/lists/netdev/msg526066.html
> Since page_pool API fixes are merged into net-next we can now safely use
> it's DMA mapping capabilities.
>
> First patch changes the buffer allocation from napi/netdev_alloc_frag()
> to page_pool API. Although this will lead to slightly reduced performance
> (on raw packet drops only) we can use the API for XDP buffer recycling.
> Another side effect is a slight increase in memory usage, due to using a
> single page per packet.
>
> The second patch adds XDP support on the driver.
> There's a bunch of interesting options that come up due to the single
> Tx queue.
> Locking is needed(to avoid messing up the Tx queues since ndo_xdp_xmit
> and the normal stack can co-exist). We also need to track down the
> 'buffer type' for TX and properly free or recycle the packet depending
> on it's nature.
>
>
> Changes since RFC:
> - Bug fixes from Jesper and Maciej
> - Added page pool API to retrieve the DMA direction
>
> Changes since v1:
> - Use page_pool_free correctly if xdp_rxq_info_reg() failed
Series applied, thanks.
I realize from the discussion on patch #3 there will be follow-ups to this.
^ permalink raw reply
* Re: [PATCH net] net/tls: make sure offload also gets the keys wiped
From: David Miller @ 2019-07-02 2:23 UTC (permalink / raw)
To: jakub.kicinski
Cc: john.fastabend, netdev, oss-drivers, alexei.starovoitov, sd,
dirk.vandermerwe
In-Reply-To: <20190628231139.16842-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 28 Jun 2019 16:11:39 -0700
> Commit 86029d10af18 ("tls: zero the crypto information from tls_context
> before freeing") added memzero_explicit() calls to clear the key material
> before freeing struct tls_context, but it missed tls_device.c has its
> own way of freeing this structure. Replace the missing free.
>
> Fixes: 86029d10af18 ("tls: zero the crypto information from tls_context before freeing")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] net/tls: reject offload of TLS 1.3
From: David Miller @ 2019-07-02 2:22 UTC (permalink / raw)
To: jakub.kicinski
Cc: netdev, oss-drivers, borisp, alexei.starovoitov, dirk.vandermerwe
In-Reply-To: <20190628230759.16360-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 28 Jun 2019 16:07:59 -0700
> Neither drivers nor the tls offload code currently supports TLS
> version 1.3. Check the TLS version when installing connection
> state. TLS 1.3 will just fallback to the kernel crypto for now.
>
> Fixes: 130b392c6cd6 ("net: tls: Add tls 1.3 support")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Applied and queued up for v5.1+ -stable.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 1/1] tc-testing: added tdc tests for prio qdisc
From: David Miller @ 2019-07-02 2:21 UTC (permalink / raw)
To: mrv; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri
In-Reply-To: <1561757521-15439-1-git-send-email-mrv@mojatatu.com>
From: Roman Mashak <mrv@mojatatu.com>
Date: Fri, 28 Jun 2019 17:32:01 -0400
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/2] Fix batched event generation for mirred action
From: David Miller @ 2019-07-02 2:18 UTC (permalink / raw)
To: mrv; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri
In-Reply-To: <1561746618-29349-1-git-send-email-mrv@mojatatu.com>
From: Roman Mashak <mrv@mojatatu.com>
Date: Fri, 28 Jun 2019 14:30:16 -0400
> When adding or deleting a batch of entries, the kernel sends upto
> TCA_ACT_MAX_PRIO entries in an event to user space. However it does not
> consider that the action sizes may vary and require different skb sizes.
>
> For example :
>
> % cat tc-batch.sh
> TC="sudo /mnt/iproute2.git/tc/tc"
>
> $TC actions flush action mirred
> for i in `seq 1 $1`;
> do
> cmd="action mirred egress redirect dev lo index $i "
> args=$args$cmd
> done
> $TC actions add $args
> %
> % ./tc-batch.sh 32
> Error: Failed to fill netlink attributes while adding TC action.
> We have an error talking to the kernel
> %
>
> patch 1 adds callback in tc_action_ops of mirred action, which calculates
> the action size, and passes size to tcf_add_notify()/tcf_del_notify().
>
> patch 2 updates the TDC test suite with relevant test cases.
Series applied, 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