* Re: Regression caused by commit 7bb05b85bc2d ("r8169: don't use MSI-X on RTL8106e")
From: Jian-Hong Pan @ 2018-09-27 9:03 UTC (permalink / raw)
To: andy.shevchenko
Cc: Kai-Heng Feng, Heiner Kallweit, Thomas Gleixner,
Linux Netdev List, Linux Kernel, Linux Upstreaming Team,
Daniel Drake, Steve Dodd
In-Reply-To: <CAHp75VcCLvb+LvE49YNWnptTMFpUNuj+Jao7udhQeLfOuHh2vg@mail.gmail.com>
Andy Shevchenko <andy.shevchenko@gmail.com> 於 2018年9月22日 週六 上午1:08寫道：
>
> On Thu, Sep 13, 2018 at 8:53 AM Jian-Hong Pan <jian-hong@endlessm.com> wrote:
> >
> > 2018-09-12 16:19 GMT+08:00 Kai-Heng Feng <kai.heng.feng@canonical.com>:
> > > at 14:32, Thomas Gleixner <tglx@linutronix.de> wrote:
> > >
> > >> On Wed, 12 Sep 2018, Kai-Heng Feng wrote:
> > >>
> > >>> There's a Dell machine with RTL8106e stops to work after S3 since the
> > >>> commit introduced. So I am wondering if it's possible to revert the
> > >>> commit and use DMI/subsystem id based quirk table?
> > >>
> > >>
> > >> Probably.
>
> Have you seen this thread:
> https://patchwork.ozlabs.org/cover/968924/
>
> and this one:
> https://patchwork.kernel.org/patch/10583229/
Ya! It is the one. And it is discussed in bugzilla
https://bugzilla.kernel.org/show_bug.cgi?id=201181
Now, the revert patch is submitted https://lkml.org/lkml/2018/9/27/224
However, still thanks for your information. :)
Regards,
Jian-Hong Pan
> ?
>
> > >
> > >
> > > Hopefully Jian-Hong can cook up a quirk table for the issue.
> >
> > Module r8169 gets nothing in the PCI BAR after system resumes which
> > makes MSI-X fail on some ASUS laptops equipped with RTL8106e chip.
> > https://www.spinics.net/lists/linux-pci/msg75598.html
> >
> > Actually, I am waiting for the patch "PCI: Reprogram bridge prefetch
> > registers on resume" being merged.
> > https://marc.info/?l=linux-pm&m=153680987814299&w=2
> >
> > It resolves the drivers which get nothing in PCI BAR after system resumes.
> >
> > After that, I can remove the falling back code of RTL8106e.
> >
> > Heiner, any comment?
> >
> > Regards,
> > Jian-Hong Pan
> >
> > >>
> > >>> It's because of commit bc976233a872 ("genirq/msi, x86/vector: Prevent
> > >>> reservation mode for non maskable MSI") cleared the reservation mode, and
> > >>> I
> > >>> can see this after S3:
> > >>>
> > >>> [ 94.872838] do_IRQ: 3.33 No irq handler for vector
> > >>
> > >>
> > >> It's not because of that commit, really. There is a interrupt sent after
> > >> resume to the wrong vector for whatever reason. The MSI vector cannot be
> > >> masked it seems in the device, but the driver should quiescen the device
> > >> to
> > >> a point where it does not send interrupts.
> > >
> > >
> > > Understood.
> > >
> > >>
> > >>> If the device uses MSI-X instead of MSI, the issue doesn't happen because
> > >>> of
> > >>> reservation mode.
> > >>
> > >>
> > >> Reservation mode has absolutely nothing to do with that. What prevents the
> > >> issue is the fact that MSI-X can be masked by the IRQ core.
> > >
> > >
> > > So in this case I think keep the device using MSI-X is a better route, it's
> > > MSI-X capable anyway.
> > >
> > >>
> > >>> Is it something should be handled by x86 BIOS? Because I don't see this
> > >>> issue
> > >>> when I use Suspend-to-Idle, which doesn't use BIOS to do suspend.
> > >>
> > >>
> > >> Suspend to idle works completely different and I don't see the BIOS at
> > >> fault here. it's more an issue of MSI not being maskable on that device,
> > >> which can't be fixed in BIOS or it's some half quiescened state which is
> > >> used when suspending and that's a pure driver issue.
> > >
> > >
> > > Understood.
> > > Thanks for all the info!
> > >
> > > Kai-Heng
> > >
> > >>
> > >> Thanks,
> > >>
> > >> tglx
> > >
> > >
> > >
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* Re: [PATCH net-next] can: xilinx: fix return type of ndo_start_xmit function
From: Marc Kleine-Budde @ 2018-09-27 8:58 UTC (permalink / raw)
To: YueHaibing, davem, wg, michal.simek
Cc: linux-kernel, netdev, linux-can, linux-arm-kernel
In-Reply-To: <20180926103229.13204-1-yuehaibing@huawei.com>
[-- Attachment #1.1: Type: text/plain, Size: 694 bytes --]
On 09/26/2018 12:32 PM, YueHaibing wrote:
> The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
> which is a typedef for an enum type, so make sure the implementation in
> this driver has returns 'netdev_tx_t' value, and change the function
> return type to netdev_tx_t.
>
> Found by coccinelle.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
applied to can-next.
Tnx,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] hv_netvsc: Make sure out channel is fully opened on send
From: Mohammed Gamal @ 2018-09-27 8:57 UTC (permalink / raw)
To: Haiyang Zhang, Stephen Hemminger, netdev@vger.kernel.org
Cc: otubo@redhat.com, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, vkuznets
In-Reply-To: <BN6PR21MB01612C60B5421AF2BB09742CCA150@BN6PR21MB0161.namprd21.prod.outlook.com>
On Wed, 2018-09-26 at 17:13 +0000, Haiyang Zhang wrote:
> > -----Original Message-----
> > From: Mohammed Gamal <mgamal@redhat.com>
> > Sent: Wednesday, September 26, 2018 12:34 PM
> > To: Stephen Hemminger <sthemmin@microsoft.com>; netdev@vger.kernel.
> > org
> > Cc: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> > <haiyangz@microsoft.com>; vkuznets <vkuznets@redhat.com>;
> > otubo@redhat.com; cavery <cavery@redhat.com>; linux-
> > kernel@vger.kernel.org; devel@linuxdriverproject.org; Mohammed
> > Gamal
> > <mgamal@redhat.com>
> > Subject: [PATCH] hv_netvsc: Make sure out channel is fully opened
> > on send
> >
> > Dring high network traffic changes to network interface parameters
> > such as
> > number of channels or MTU can cause a kernel panic with a NULL
> > pointer
> > dereference. This is due to netvsc_device_remove() being called and
> > deallocating the channel ring buffers, which can then be accessed
> > by
> > netvsc_send_pkt() before they're allocated on calling
> > netvsc_device_add()
> >
> > The patch fixes this problem by checking the channel state and
> > returning
> > ENODEV if not yet opened. We also move the call to
> > hv_ringbuf_avail_percent()
> > which may access the uninitialized ring buffer.
> >
> > Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
> > ---
> > drivers/net/hyperv/netvsc.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/hyperv/netvsc.c
> > b/drivers/net/hyperv/netvsc.c index
> > fe01e14..75f1b31 100644
> > --- a/drivers/net/hyperv/netvsc.c
> > +++ b/drivers/net/hyperv/netvsc.c
> > @@ -825,7 +825,12 @@ static inline int netvsc_send_pkt(
> > struct netdev_queue *txq = netdev_get_tx_queue(ndev,
> > packet->q_idx);
> > u64 req_id;
> > int ret;
> > - u32 ring_avail =
> > hv_get_avail_to_write_percent(&out_channel-
> > > outbound);
> >
> > + u32 ring_avail;
> > +
> > + if (out_channel->state != CHANNEL_OPENED_STATE)
> > + return -ENODEV;
> > +
> > + ring_avail = hv_get_avail_to_write_percent(&out_channel-
> > >outbound);
>
> When you reproducing the NULL ptr panic, does your kernel include the
> following patch?
> hv_netvsc: common detach logic
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/c
> ommit/?id=7b2ee50c0cd513a176a26a71f2989facdd75bfea
>
Yes it is included. And the commit did reduce the occurrence of this
race condition, but it still nevertheless occurs albeit rarely.
> We call netif_tx_disable(ndev) and netif_device_detach(ndev) before
> doing the changes
> on MTU or #channels. So there should be no call to start_xmit() when
> channel is not ready.
>
> If you see the check for CHANNEL_OPENED_STATE is still necessary on
> upstream kernel (including
> the patch " common detach logic "), we should debug further on the
> code and find out the
> root cause.
>
> Thanks,
> - Haiyang
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply
* Re: [PATCH] netlink: add policy attribute range validation
From: Michal Kubecek @ 2018-09-27 8:48 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1538035929.14416.21.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
On Thu, Sep 27, 2018 at 10:12:09AM +0200, Johannes Berg wrote:
> On Thu, 2018-09-27 at 09:16 +0200, Michal Kubecek wrote:
>
> > The overloading still feels a bit complicated. Perhaps we could rather
> > use validation_data in the natural way, i.e. as a pointer to validation
> > data. That would be a struct (maybe array) of two values of the
> > corresponding type. It would mean a bit more data and a bit more writing
> > but it would be IMHO more straightforward.
>
> I considered that, but I didn't really like it either. The memory
> wasting isn't *that* bad (even if we go to s64 that'd only be ~20x16
> bytes for nl80211, eating up 320 out of the 550 saved, but still); I'm
> more worried about making this really hard to actually *do*.
>
> Consider
>
> policy[] = {
> ...
> [NL80211_ATTR_WIPHY_RETRY_SHORT] =
> NLA_POLICY_RANGE(NLA_U8, 1, 255),
> ...
> };
>
> vs.
>
> static const struct netlink_policy_range retry_range = {
> .min = 1,
> .max = 255,
> };
We could still use helper macros so this part could become
DEFINE_NLA_U8_RANGE(retry_range, 1, 255);
or
DEFINE_NLA_RANGE(retry_range, u8, 1, 255);
>
> policy[] = {
> ...
> [NL80211_ATTR_WIPHY_RETRY_SHORT] = {
> .type = NLA_U8,
> .validation_data = &retry_range,
> },
> ...
> };
And this could be also shortened using a macro.
It would still be longer but not that much.
> That's significantly more to type, to the point where I'd seriously
> consider doing this only for attributes that are used and checked in
> many places - it doesn't feel like a big win over manual range-checking.
>
> But I want it to be a win over manual range-checking so it gets used
> more because it's more efficient, less prone to getting messed up if
> multiple places use the same attribute and validates attributes even if
> they're ignored by an operation.
>
>
> I'd also say that we're certainly no strangers to union/overloading, so
> I don't feel like this is a big argument. One doesn't even really have
> to be *aware* of it for the most part: if it were a struct instead of a
> union, it'd actually have the same effect since the .type field
> indicates which part gets used. That it's overloaded in a union is
> basically just a space saving measure, I don't think it makes the
> reasoning much more complex?
I didn't mean it as a serious objection, rather a note that the gain may
not be worth the additional complexity. But if you want to follow in the
direction you indicated later (in particular, allowing different
interpretations of validation_data for the same type), overloading does
indeed make more sense.
Michal Kubecek
^ permalink raw reply
* [PATCH net] vhost-vsock: fix use after free
From: Jason Wang @ 2018-09-27 8:43 UTC (permalink / raw)
To: stefanha, mst; +Cc: kvm, virtualization, netdev, linux-kernel, Jason Wang
The access of vsock is not protected by vhost_vsock_lock. This may
lead use after free since vhost_vsock_dev_release() may free the
pointer at the same time.
Fix this by holding the lock during the acess.
Reported-by: syzbot+e3e074963495f92a89ed@syzkaller.appspotmail.com
Fixes: 16320f363ae1 ("vhost-vsock: add pkt cancel capability")
Fixes: 433fc58e6bf2 ("VSOCK: Introduce vhost_vsock.ko")
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
- The patch is needed for -stable.
---
drivers/vhost/vsock.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 34bc3ab40c6d..7d0b292867fd 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -210,21 +210,27 @@ vhost_transport_send_pkt(struct virtio_vsock_pkt *pkt)
struct vhost_vsock *vsock;
int len = pkt->len;
+ spin_lock_bh(&vhost_vsock_lock);
+
/* Find the vhost_vsock according to guest context id */
- vsock = vhost_vsock_get(le64_to_cpu(pkt->hdr.dst_cid));
+ vsock = __vhost_vsock_get(le64_to_cpu(pkt->hdr.dst_cid));
if (!vsock) {
virtio_transport_free_pkt(pkt);
+ spin_unlock_bh(&vhost_vsock_lock);
return -ENODEV;
}
if (pkt->reply)
atomic_inc(&vsock->queued_replies);
- spin_lock_bh(&vsock->send_pkt_list_lock);
+ spin_lock(&vsock->send_pkt_list_lock);
list_add_tail(&pkt->list, &vsock->send_pkt_list);
- spin_unlock_bh(&vsock->send_pkt_list_lock);
+ spin_unlock(&vsock->send_pkt_list_lock);
vhost_work_queue(&vsock->dev, &vsock->send_pkt_work);
+
+ spin_unlock_bh(&vhost_vsock_lock);
+
return len;
}
@@ -236,18 +242,22 @@ vhost_transport_cancel_pkt(struct vsock_sock *vsk)
int cnt = 0;
LIST_HEAD(freeme);
+ spin_lock_bh(&vhost_vsock_lock);
+
/* Find the vhost_vsock according to guest context id */
- vsock = vhost_vsock_get(vsk->remote_addr.svm_cid);
- if (!vsock)
+ vsock = __vhost_vsock_get(vsk->remote_addr.svm_cid);
+ if (!vsock) {
+ spin_unlock_bh(&vhost_vsock_lock);
return -ENODEV;
+ }
- spin_lock_bh(&vsock->send_pkt_list_lock);
+ spin_lock(&vsock->send_pkt_list_lock);
list_for_each_entry_safe(pkt, n, &vsock->send_pkt_list, list) {
if (pkt->vsk != vsk)
continue;
list_move(&pkt->list, &freeme);
}
- spin_unlock_bh(&vsock->send_pkt_list_lock);
+ spin_unlock(&vsock->send_pkt_list_lock);
list_for_each_entry_safe(pkt, n, &freeme, list) {
if (pkt->reply)
@@ -265,6 +275,8 @@ vhost_transport_cancel_pkt(struct vsock_sock *vsk)
vhost_poll_queue(&tx_vq->poll);
}
+ spin_unlock_bh(&vhost_vsock_lock);
+
return 0;
}
--
2.17.1
^ permalink raw reply related
* Could your company tell us if you can supply us
From: DEBENHAMS PLC @ 2018-09-27 0:05 UTC (permalink / raw)
Hi
Situated in (United Kingdom) DEBENHAMS RETAIL PLC is a retailer shop
furnished by European products.We are looking to buy ( Fashion
items,Perfumes, Lighters, Tiles Marbles, Wooden Floors, Smart-phones,
Tablets,Laptops, TVs, Air conditioner etc...) and find new partnership
with companies dealing with different products so can you please send us
your Catalog or your website to learn more about your products or prices
list by email and if we can make some order with you and start long-term
partnership.Could you also tell us if you can supply us, more
information about the possibility to become one of your regular
customers?
Our Payment Policy is payment by swift within 30 days net in Europe.
Waiting for your feedback.
Best regards.
Richard Jones
Trading Director, Global Sourcing
DEBENHAMS RETAIL PLC
Direct line : +44 7413339961
Fax : +44 02082534105
www.debenhams.com
www.debenhamsplus.com
^ permalink raw reply
* Re: [PATCH bpf-next 0/5] Introduce libbpf_attach_type_by_name
From: Jakub Kicinski @ 2018-09-27 2:06 UTC (permalink / raw)
To: Andrey Ignatov
Cc: netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
Kernel Team
In-Reply-To: <20180926235413.GA38328@rdna-mbp.dhcp.thefacebook.com>
On Wed, 26 Sep 2018 23:54:17 +0000, Andrey Ignatov wrote:
> Jakub Kicinski <jakub.kicinski@netronome.com> [Wed, 2018-09-26 16:20 -0700]:
> > On Wed, 26 Sep 2018 15:24:52 -0700, Andrey Ignatov wrote:
> > > This patch set introduces libbpf_attach_type_by_name function in libbpf to
> > > identify attach type by section name.
> > >
> > > This is useful to avoid writing same logic over and over again in user
> > > space applications that leverage libbpf.
> > >
> > > Patch 1 has more details on the new function and problem being solved.
> > > Patches 2 and 3 add support for new section names.
> > > Patch 4 uses new function in a selftest.
> > > Patch 5 adds selftest for libbpf_{prog,attach}_type_by_name.
> > >
> > > As a side note there are a lot of inconsistencies now between names used by
> > > libbpf and bpftool (e.g. cgroup/skb vs cgroup_skb, cgroup_device and device
> > > vs cgroup/dev, sockops vs sock_ops, etc). This patch set does not address
> > > it but it tries not to make it harder to address it in the future.
> >
> > I was wondering a few times whether I should point it out to people
> > during review, but thought it would be nit picking. Maybe we should be
> > more strict.
> >
> > Your series LGTM!
>
> Thanks for review!
>
> IMO having it consistent would be great, e.g. one writes a program with
> section name X and bpftool shows/accepts it in exactly same way in all
> its sub-commands (w/o maybe custom suffix added by program writer).
>
> But I doubt that keeping a few places in sync manually will work long
> term since it's easy to miss such a thing.
>
> What do you think of having one source of truth in libbpf so that a
> string for prog_type or attach_type is defined once and all other places
> (e.g. bpftool prog show, bpftool cgroup show) use only corresponding
> enum-s to get those strings, but don't introduce any new strings?
>
> Keeping already existing names in a backward compatible way is a pain
> though.
One source of truth would be nice IMO. The backward compat ties our
hands a tiny bit, but we could do a fallback strategy, where bpftool
checks if it has a name defined for a type, and if it doesn't (for new
types) it will ask libbpf.
There is another place actually where names are hard coded - for program
load command we use the types as they appear in libbpf already. And
that requires putting them in help, man page and bash completions, too.
> Another thing, I was wondering, is if there is a way to bypass strings
> completely (at least in libbpf, since bpftool still has to print
> human-readable names) and keep actual bpf_prog_type and bpf_attach_type
> as metadata for a program in ELF file. Maybe some compiler magic ..
Possibly. I'm far from an ELF expert, but I think attaching arbitrary
metadata to sections may be harder, name is already there and is free
form.
^ permalink raw reply
* Re: [PATCH] netlink: add policy attribute range validation
From: Johannes Berg @ 2018-09-27 8:12 UTC (permalink / raw)
To: Michal Kubecek
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20180927071621.GF30601-OEaqT8BN2ewCVLCxKZUutA@public.gmane.org>
On Thu, 2018-09-27 at 09:16 +0200, Michal Kubecek wrote:
> The overloading still feels a bit complicated. Perhaps we could rather
> use validation_data in the natural way, i.e. as a pointer to validation
> data. That would be a struct (maybe array) of two values of the
> corresponding type. It would mean a bit more data and a bit more writing
> but it would be IMHO more straightforward.
I considered that, but I didn't really like it either. The memory
wasting isn't *that* bad (even if we go to s64 that'd only be ~20x16
bytes for nl80211, eating up 320 out of the 550 saved, but still); I'm
more worried about making this really hard to actually *do*.
Consider
policy[] = {
...
[NL80211_ATTR_WIPHY_RETRY_SHORT] =
NLA_POLICY_RANGE(NLA_U8, 1, 255),
...
};
vs.
static const struct netlink_policy_range retry_range = {
.min = 1,
.max = 255,
};
policy[] = {
...
[NL80211_ATTR_WIPHY_RETRY_SHORT] = {
.type = NLA_U8,
.validation_data = &retry_range,
},
...
};
That's significantly more to type, to the point where I'd seriously
consider doing this only for attributes that are used and checked in
many places - it doesn't feel like a big win over manual range-checking.
But I want it to be a win over manual range-checking so it gets used
more because it's more efficient, less prone to getting messed up if
multiple places use the same attribute and validates attributes even if
they're ignored by an operation.
I'd also say that we're certainly no strangers to union/overloading, so
I don't feel like this is a big argument. One doesn't even really have
to be *aware* of it for the most part: if it were a struct instead of a
union, it'd actually have the same effect since the .type field
indicates which part gets used. That it's overloaded in a union is
basically just a space saving measure, I don't think it makes the
reasoning much more complex?
That said, given that I also later sent that RFC patch for a further
validation function pointer (which is useful e.g. for ensuring certain
binary attributes are well-formed), I think it might in fact be better
to split .type field (it really only needs to be u8, we have ~20) and
adding a "validation" enum to the policy:
enum netlink_policy_validation {
/* default */
NLA_VALIDATE_NONE,
/* valid for NLA_{U,S}{8,16,32,64} */
NLA_VALIDATE_MIN,
NLA_VALIDATE_MAX,
NLA_VALIDATE_RANGE,
/* valid for any type other than NLA_BITFIELD32/NLA_REJECT */
NLA_VALIDATE_FUNCTION,
};
Combining that with macros like the ones I wrote in my previous message
in this thread:
#define __NLA_ENSURE(condition) (sizeof(char[1 - 2*!(condition)]) - 1)
#define NLA_ENSURE_INT_TYPE(tp) \
(__NLA_ENSURE(tp == NLA_S8 || tp == NLA_U8 || \
tp == NLA_S16 || tp == NLA_U16 || \
tp == NLA_S32 || tp == NLA_U32 || \
tp == NLA_S64 || tp == NLA_U64) + tp)
#define NLA_ENSURE_NO_VALIDATION_PTR(tp) \
(__NLA_ENSURE(tp != NLA_BITFIELD32 && \
tp != NLA_REJECT && \
tp != NLA_NESTED && \
tp != NLA_NESTED_ARRAY) + tp)
#define NLA_POLICY_RANGE(tp, _min, _max) {
.type = NLA_ENSURE_INT_TYPE(tp),
.validate_type = NLA_VALIDATE_RANGE,
.min = _min,
.max = _max,
}
#define NLA_POLICY_MIN(tp, _min) {
.type = NLA_ENSURE_INT_TYPE(tp),
.validate_type = NLA_VALIDATE_MIN,
.min = _min,
}
#define NLA_POLICY_MAX(tp, _max) {
.type = NLA_ENSURE_INT_TYPE(tp),
.validate_type = NLA_VALIDATE_MAX,
.max = _max,
}
#define NLA_POLICY_FN(tp, fn) {
.type = NLA_ENSURE_NO_VALIDATION_PTR(tp),
.validate_type = NLA_VALIDATE_FUNCTION,
.validate = fn,
}
This would even give us the flexibility to extend the validation type
further in the future, to actually have what you suggested where the
validation_data is a pointer and the valid range is given in a struct
pointed to, to allow larger ranges than s16.
johannes
^ permalink raw reply
* Re: KASAN: use-after-free Read in tcf_block_find
From: Dmitry Vyukov @ 2018-09-27 8:10 UTC (permalink / raw)
To: Cong Wang
Cc: Eric Dumazet, syzbot+37b8770e6d5a8220a039, David Miller,
Jamal Hadi Salim, Jiri Pirko, LKML,
Linux Kernel Network Developers, syzkaller-bugs
In-Reply-To: <CACT4Y+YbfOYDHJjeK-88wQDg2oSF3fqsaWBhkDwnDr+KUsLnDA@mail.gmail.com>
On Thu, Sep 27, 2018 at 10:06 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Wed, Sep 26, 2018 at 11:55 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> >> Hello,
>>> >>
>>> >> syzbot found the following crash on:
>>> >>
>>> >> HEAD commit: 4b1bd6976945 net: phy: marvell: Fix build.
>>> >> git tree: net-next
>>> >> console output: https://syzkaller.appspot.com/x/log.txt?x=16f763fa400000
>>> >> kernel config: https://syzkaller.appspot.com/x/.config?x=443816db871edd66
>>> >> dashboard link: https://syzkaller.appspot.com/bug?extid=37b8770e6d5a8220a039
>>> >> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>>> >> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17a5614e400000
>>> >> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=141a532a400000
>>> >>
>>> >> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>> >> Reported-by: syzbot+37b8770e6d5a8220a039@syzkaller.appspotmail.com
>>> >>
>>> >> IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
>>> >> 8021q: adding VLAN 0 to HW filter on device team0
>>> >> ==================================================================
>>> >> BUG: KASAN: use-after-free in tcf_block_find+0x9d1/0xb90
>>> >> net/sched/cls_api.c:646
>>> >> Read of size 4 at addr ffff8801cc126978 by task syz-executor002/5646
>>> >>
>>> >> CPU: 1 PID: 5646 Comm: syz-executor002 Not tainted 4.19.0-rc5+ #232
>>> >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>>> >> Google 01/01/2011
>>> >> Call Trace:
>>> >> __dump_stack lib/dump_stack.c:77 [inline]
>>> >> dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
>>> >> print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
>>> >> kasan_report_error mm/kasan/report.c:354 [inline]
>>> >> kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
>>> >> __asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:432
>>> >> tcf_block_find+0x9d1/0xb90 net/sched/cls_api.c:646
>>> >
>>> > Hmm. looks like missing a rcu_dereference() here:
>>> >
>>> > if (!*parent) {
>>> > *q = dev->qdisc;
>>> > *parent = (*q)->handle;
>>> >
>>>
>>> We hold RTNL here.
>>>
>>> Have we already done the changes allowing dev->qdisc being changed
>>> without RTNL being required ?
>>
>> That transition is not completed yet, but holding RTNL doesn't help
>> any more after we now free qdisc in rcu callback.
>>
>> More interestingly, rcu read lock is already held in this context,
>> so it seems we still have to use rcu_deference() to read dev->qdisc
>> and of course mark it with __rcu too.
>
> On hardware level rcu_deference() only affects Alpha. On software
> level, it can affect all archs due to e.g. a re-read of the variable.
> Do we see the potential for compilation that would lead to the UAF
> here? A missed rcu_deference() would not be the first thing that I
> would suspect for UAF on x86. It's more likely some bolder bug.
Would a stack trace for call_rcu be helpful here? I have this idea for
a long time, but never get around to implementing it:
https://bugzilla.kernel.org/show_bug.cgi?id=198437
Also FWIW I recently used the following hack for another net bug. It
made that other bug involving call_rcu way more likely to fire. Maybe
it will be helpful here too.
diff --git a/net/core/dst.c b/net/core/dst.c
index 81ccf20e28265..591a8d0aca545 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -187,8 +187,16 @@ void dst_release(struct dst_entry *dst)
if (unlikely(newrefcnt < 0))
net_warn_ratelimited("%s: dst:%p refcnt:%d\n",
__func__, dst, newrefcnt);
- if (!newrefcnt)
- call_rcu(&dst->rcu_head, dst_destroy_rcu);
+ if (!newrefcnt) {
+ if (lock_is_held(&rcu_bh_lock_map) ||
+ lock_is_held(&rcu_lock_map) ||
+ lock_is_held(&rcu_sched_lock_map)) {
+ call_rcu(&dst->rcu_head, dst_destroy_rcu);
+ } else {
+ synchronize_rcu();
+ dst_destroy_rcu(&dst->rcu_head);
+ }
+ }
}
}
^ permalink raw reply related
* Re: KASAN: use-after-free Read in tcf_block_find
From: Dmitry Vyukov @ 2018-09-27 8:06 UTC (permalink / raw)
To: Cong Wang
Cc: Eric Dumazet, syzbot+37b8770e6d5a8220a039, David Miller,
Jamal Hadi Salim, Jiri Pirko, LKML,
Linux Kernel Network Developers, syzkaller-bugs
In-Reply-To: <CAM_iQpX4sKSPFXgEjU-be=he=91+=3Bm9KvP3CMciA9zi2b-6g@mail.gmail.com>
On Wed, Sep 26, 2018 at 11:55 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> >> Hello,
>> >>
>> >> syzbot found the following crash on:
>> >>
>> >> HEAD commit: 4b1bd6976945 net: phy: marvell: Fix build.
>> >> git tree: net-next
>> >> console output: https://syzkaller.appspot.com/x/log.txt?x=16f763fa400000
>> >> kernel config: https://syzkaller.appspot.com/x/.config?x=443816db871edd66
>> >> dashboard link: https://syzkaller.appspot.com/bug?extid=37b8770e6d5a8220a039
>> >> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>> >> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17a5614e400000
>> >> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=141a532a400000
>> >>
>> >> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> >> Reported-by: syzbot+37b8770e6d5a8220a039@syzkaller.appspotmail.com
>> >>
>> >> IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
>> >> 8021q: adding VLAN 0 to HW filter on device team0
>> >> ==================================================================
>> >> BUG: KASAN: use-after-free in tcf_block_find+0x9d1/0xb90
>> >> net/sched/cls_api.c:646
>> >> Read of size 4 at addr ffff8801cc126978 by task syz-executor002/5646
>> >>
>> >> CPU: 1 PID: 5646 Comm: syz-executor002 Not tainted 4.19.0-rc5+ #232
>> >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>> >> Google 01/01/2011
>> >> Call Trace:
>> >> __dump_stack lib/dump_stack.c:77 [inline]
>> >> dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
>> >> print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
>> >> kasan_report_error mm/kasan/report.c:354 [inline]
>> >> kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
>> >> __asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:432
>> >> tcf_block_find+0x9d1/0xb90 net/sched/cls_api.c:646
>> >
>> > Hmm. looks like missing a rcu_dereference() here:
>> >
>> > if (!*parent) {
>> > *q = dev->qdisc;
>> > *parent = (*q)->handle;
>> >
>>
>> We hold RTNL here.
>>
>> Have we already done the changes allowing dev->qdisc being changed
>> without RTNL being required ?
>
> That transition is not completed yet, but holding RTNL doesn't help
> any more after we now free qdisc in rcu callback.
>
> More interestingly, rcu read lock is already held in this context,
> so it seems we still have to use rcu_deference() to read dev->qdisc
> and of course mark it with __rcu too.
On hardware level rcu_deference() only affects Alpha. On software
level, it can affect all archs due to e.g. a re-read of the variable.
Do we see the potential for compilation that would lead to the UAF
here? A missed rcu_deference() would not be the first thing that I
would suspect for UAF on x86. It's more likely some bolder bug.
^ permalink raw reply
* nfs+tcp boot failures on linuxnext-20180924 from switch tcp_clock_ns to CLOCK_TAI
From: Leonard Crestez @ 2018-09-27 1:15 UTC (permalink / raw)
To: sfr@canb.auug.org.au, davem@davemloft.net, edumazet@google.com
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-next@vger.kernel.org
Hello,
It seems that after commit 72b0094f9182 ("tcp: switch tcp_clock_ns() to
CLOCK_TAI base") some systems that use nfs over tcp fail to boot. The
last line printed in the log is from systemd:
[ 7.232579] systemd[1]: System time before build time, advancing clock.
Superficially it looks like very large clock discontinuities now break
TCP. Maybe boottime could avoid such issues?
I didn't find similar reports anywhere else. The machines I’m seeing
this are all 32bit arm imx (this shouldn’t matter) and it seems their
RTC isn’t properly setup so they boot with realtime set to unix zero,
then a ~50 years jump happens when systemd starts up. This is the
likely trigger for this issue.
--
Regards,
Leonard
^ permalink raw reply
* Reminder,
From: Juliet Muhammad @ 2018-09-27 0:50 UTC (permalink / raw)
To: Recipients
Hello
Please i still await your response regarding my previous email.
^ permalink raw reply
* Reminder,
From: Juliet Muhammad @ 2018-09-27 0:45 UTC (permalink / raw)
To: Recipients
Hello
Please i still await your response regarding my previous email.
^ permalink raw reply
* Re: [PATCH bpf-next] flow_dissector: lookup netns by skb->sk if skb->dev is NULL
From: Eric Dumazet @ 2018-09-27 0:49 UTC (permalink / raw)
To: Daniel Borkmann, Willem de Bruijn, netdev; +Cc: ast, davem, Willem de Bruijn
In-Reply-To: <1cc82769-d4c4-2976-d341-0537d2d2eb24@iogearbox.net>
On 09/25/2018 08:38 AM, Daniel Borkmann wrote:
> On 09/24/2018 10:49 PM, Willem de Bruijn wrote:
>> From: Willem de Bruijn <willemb@google.com>
>>
>> BPF flow dissectors are configured per network namespace.
>> __skb_flow_dissect looks up the netns through dev_net(skb->dev).
>>
>> In some dissector paths skb->dev is NULL, such as for Unix sockets.
>> In these cases fall back to looking up the netns by socket.
>>
>> Analyzing the codepaths leading to __skb_flow_dissect I did not find
>> a case where both skb->dev and skb->sk are NULL. Warn and fall back to
>> standard flow dissector if one is found.
>>
>> Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
>> Reported-by: Eric Dumazet <edumazet@google.com>
>> Signed-off-by: Willem de Bruijn <willemb@google.com>
>
> Applied to bpf-next, thanks Willem!
>
Sadly lib/test_bpf should still cause crashes, because populate_skb()
populates an skb attached to a fake device, for which net pointer is NULL.
^ permalink raw reply
* [PATCH net] net/ipv6: Remove extra call to ip6_convert_metrics for multipath case
From: dsahern @ 2018-09-27 0:35 UTC (permalink / raw)
To: netdev; +Cc: David Ahern
From: David Ahern <dsahern@gmail.com>
The change to move metrics from the dst to rt6_info moved the call
to ip6_convert_metrics from ip6_route_add to ip6_route_info_create. In
doing so it makes the call in ip6_route_info_append redundant and
actually leaks the metrics installed as part of the ip6_route_info_create.
Remove the now unnecessary call.
Fixes: d4ead6b34b67f ("net/ipv6: move metrics from dst to rt6_info")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv6/route.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index d28f83e01593..570f158253b9 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4293,11 +4293,6 @@ static int ip6_route_info_append(struct net *net,
if (!nh)
return -ENOMEM;
nh->fib6_info = rt;
- err = ip6_convert_metrics(net, rt, r_cfg);
- if (err) {
- kfree(nh);
- return err;
- }
memcpy(&nh->r_cfg, r_cfg, sizeof(*r_cfg));
list_add_tail(&nh->next, rt6_nh_list);
--
2.11.0
^ permalink raw reply related
* Reminder,
From: Juliet Muhammad @ 2018-09-27 0:16 UTC (permalink / raw)
To: Recipients
Hello
Please i still await your response regarding my previous email.
^ permalink raw reply
* Re: WARN_ON in TLP causing RT throttling
From: Eric Dumazet @ 2018-09-27 0:09 UTC (permalink / raw)
To: stranche, eric.dumazet; +Cc: soheil@google.com
In-Reply-To: <7aa9932a59aad7a21c7f8a8146dd0542@codeaurora.org>
On 09/26/2018 04:46 PM, stranche@codeaurora.org wrote:
> Hi Eric,
>
> Someone recently reported a crash to us on the 4.14.62 kernel where excessive
> WARNING prints were spamming the logs and causing watchdog bites. The kernel
> does have the following commit by Soheil:
> bffd168c3fc5 "tcp: clear tp->packets_out when purging write queue"
>
> Before this bug we see over 1 second of continuous WARN_ON prints from
> tcp_send_loss_probe() like so:
>
> 7795.530450: <2> tcp_send_loss_probe+0x194/0x1b8
> 7795.534833: <2> tcp_write_timer_handler+0xf8/0x1c4
> 7795.539492: <2> tcp_write_timer+0x4c/0x74
> 7795.543348: <2> call_timer_fn+0xc0/0x1b4
> 7795.547113: <2> run_timer_softirq+0x248/0x81c
>
> Specifically, the prints come from the following check:
>
> /* Retransmit last segment. */
> if (WARN_ON(!skb))
> goto rearm_timer;
>
> Since skb is always NULL, we know there's nothing on the write queue or the
> retransmit queue, so we just keep resetting the timer, waiting for more data
> to be queued. However, we were able to determine that the TCP socket is in the
> TCP_FIN_WAIT1 state, so we will no longer be sending any data and these queues
> remain empty.
>
> Would it be appropriate to stop resetting the TLP timer if we detect that the
> connection is starting to close and we have no more data to send the probe with,
> or is there some way that this scenario should already be handled?
>
> Unfortunately, we don't have a reproducer for this crash.
>
Something is fishy.
If there is no skb in the queues, then tp->packets_out should be 0,
therefore tcp_rearm_rto() should simply call inet_csk_clear_xmit_timer(sk, ICSK_TIME_RETRANS);
I have never seen this report before.
^ permalink raw reply
* Re: [PATCH bpf-next 0/5] Introduce libbpf_attach_type_by_name
From: Andrey Ignatov @ 2018-09-26 23:54 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
Kernel Team
In-Reply-To: <20180926162007.37a4b873@cakuba.netronome.com>
Jakub Kicinski <jakub.kicinski@netronome.com> [Wed, 2018-09-26 16:20 -0700]:
> On Wed, 26 Sep 2018 15:24:52 -0700, Andrey Ignatov wrote:
> > This patch set introduces libbpf_attach_type_by_name function in libbpf to
> > identify attach type by section name.
> >
> > This is useful to avoid writing same logic over and over again in user
> > space applications that leverage libbpf.
> >
> > Patch 1 has more details on the new function and problem being solved.
> > Patches 2 and 3 add support for new section names.
> > Patch 4 uses new function in a selftest.
> > Patch 5 adds selftest for libbpf_{prog,attach}_type_by_name.
> >
> > As a side note there are a lot of inconsistencies now between names used by
> > libbpf and bpftool (e.g. cgroup/skb vs cgroup_skb, cgroup_device and device
> > vs cgroup/dev, sockops vs sock_ops, etc). This patch set does not address
> > it but it tries not to make it harder to address it in the future.
>
> I was wondering a few times whether I should point it out to people
> during review, but thought it would be nit picking. Maybe we should be
> more strict.
>
> Your series LGTM!
Thanks for review!
IMO having it consistent would be great, e.g. one writes a program with
section name X and bpftool shows/accepts it in exactly same way in all
its sub-commands (w/o maybe custom suffix added by program writer).
But I doubt that keeping a few places in sync manually will work long
term since it's easy to miss such a thing.
What do you think of having one source of truth in libbpf so that a
string for prog_type or attach_type is defined once and all other places
(e.g. bpftool prog show, bpftool cgroup show) use only corresponding
enum-s to get those strings, but don't introduce any new strings?
Keeping already existing names in a backward compatible way is a pain
though.
Another thing, I was wondering, is if there is a way to bypass strings
completely (at least in libbpf, since bpftool still has to print
human-readable names) and keep actual bpf_prog_type and bpf_attach_type
as metadata for a program in ELF file. Maybe some compiler magic ..
--
Andrey Ignatov
^ permalink raw reply
* WARN_ON in TLP causing RT throttling
From: stranche @ 2018-09-26 23:46 UTC (permalink / raw)
To: eric.dumazet; +Cc: soheil@google.com
Hi Eric,
Someone recently reported a crash to us on the 4.14.62 kernel where
excessive
WARNING prints were spamming the logs and causing watchdog bites. The
kernel
does have the following commit by Soheil:
bffd168c3fc5 "tcp: clear tp->packets_out when purging write queue"
Before this bug we see over 1 second of continuous WARN_ON prints from
tcp_send_loss_probe() like so:
7795.530450: <2> tcp_send_loss_probe+0x194/0x1b8
7795.534833: <2> tcp_write_timer_handler+0xf8/0x1c4
7795.539492: <2> tcp_write_timer+0x4c/0x74
7795.543348: <2> call_timer_fn+0xc0/0x1b4
7795.547113: <2> run_timer_softirq+0x248/0x81c
Specifically, the prints come from the following check:
/* Retransmit last segment. */
if (WARN_ON(!skb))
goto rearm_timer;
Since skb is always NULL, we know there's nothing on the write queue or
the
retransmit queue, so we just keep resetting the timer, waiting for more
data
to be queued. However, we were able to determine that the TCP socket is
in the
TCP_FIN_WAIT1 state, so we will no longer be sending any data and these
queues
remain empty.
Would it be appropriate to stop resetting the TLP timer if we detect
that the
connection is starting to close and we have no more data to send the
probe with,
or is there some way that this scenario should already be handled?
Unfortunately, we don't have a reproducer for this crash.
Thanks,
Sean
^ permalink raw reply
* Re: bpf: Massive skbuff_head_cache memory leak?
From: John Johansen @ 2018-09-26 23:35 UTC (permalink / raw)
To: Daniel Borkmann, Tetsuo Handa, Alexei Starovoitov
Cc: Network Development, David S. Miller, Dmitry Vyukov,
Andrew Morton, Michal Hocko
In-Reply-To: <037d9772-63fe-203a-edb7-4af65016f6ff@iogearbox.net>
On 09/26/2018 02:22 PM, Daniel Borkmann wrote:
> On 09/26/2018 11:09 PM, Tetsuo Handa wrote:
>> Hello, Alexei and Daniel.
>>
>> Can you show us how to run testcases you are testing?
>
> Sorry for the delay; currently quite backlogged but will definitely take a look
> at these reports. Regarding your question: majority of test cases are in the
> kernel tree under selftests, see tools/testing/selftests/bpf/ .
>
Its unlikely to be apparmor. I went through the reports and saw nothing that
would indicate apparmor involvement, but the primary reason is what is being tested
in upstream apparmor atm.
The current upstream code does nothing directly with skbuffs. Its
possible that the audit code paths (kernel audit does grab skbuffs)
could, but there are only a couple cases that would be triggered in
the current fuzzing so this seems to be an unlikely source for such a
large leak.
>> On 2018/09/22 22:25, Tetsuo Handa wrote:
>>> Hello.
>>>
>>> syzbot is reporting many lockup problems on bpf.git / bpf-next.git / net.git / net-next.git trees.
>>>
>>> INFO: rcu detected stall in br_multicast_port_group_expired (2)
>>> https://syzkaller.appspot.com/bug?id=15c7ad8cf35a07059e8a697a22527e11d294bc94
>>>
>>> INFO: rcu detected stall in tun_chr_close
>>> https://syzkaller.appspot.com/bug?id=6c50618bde03e5a2eefdd0269cf9739c5ebb8270
>>>
>>> INFO: rcu detected stall in discover_timer
>>> https://syzkaller.appspot.com/bug?id=55da031ddb910e58ab9c6853a5784efd94f03b54
>>>
>>> INFO: rcu detected stall in ret_from_fork (2)
>>> https://syzkaller.appspot.com/bug?id=c83129a6683b44b39f5b8864a1325893c9218363
>>>
>>> INFO: rcu detected stall in addrconf_rs_timer
>>> https://syzkaller.appspot.com/bug?id=21c029af65f81488edbc07a10ed20792444711b6
>>>
>>> INFO: rcu detected stall in kthread (2)
>>> https://syzkaller.appspot.com/bug?id=6accd1ed11c31110fed1982f6ad38cc9676477d2
>>>
>>> INFO: rcu detected stall in ext4_filemap_fault
>>> https://syzkaller.appspot.com/bug?id=817e38d20e9ee53390ac361bf0fd2007eaf188af
>>>
>>> INFO: rcu detected stall in run_timer_softirq (2)
>>> https://syzkaller.appspot.com/bug?id=f5a230a3ff7822f8d39fddf8485931bd06ae47fe
>>>
>>> INFO: rcu detected stall in bpf_prog_ADDR
>>> https://syzkaller.appspot.com/bug?id=fb4911fd0e861171cc55124e209f810a0dd68744
>>>
>>> INFO: rcu detected stall in __run_timers (2)
>>> https://syzkaller.appspot.com/bug?id=65416569ddc8d2feb8f19066aa761f5a47f7451a
>>>
>>> The cause of lockup seems to be flood of printk() messages from memory allocation
>>> failures, and one of out_of_memory() messages indicates that skbuff_head_cache
>>> usage is huge enough to suspect in-kernel memory leaks.
>>>
>>> [ 1554.547011] skbuff_head_cache 1847887KB 1847887KB
>>>
>>> Unfortunately, we cannot find from logs what syzbot is trying to do
>>> because constant printk() messages is flooding away syzkaller messages.
>>> Can you try running your testcases with kmemleak enabled?
>>>
>>
>> On 2018/09/27 2:35, Dmitry Vyukov wrote:
>>> I also started suspecting Apparmor. We switched to Apparmor on Aug 30:
>>> https://groups.google.com/d/msg/syzkaller-bugs/o73lO4KGh0w/j9pcH2tSBAAJ
>>> Now the instances that use SELinux and Smack explicitly contain that
>>> in the name, but the rest are Apparmor.
>>> Aug 30 roughly matches these assorted "task hung" reports. Perhaps
>>> some Apparmor hook leaks a reference to skbs?
>>
>> Maybe. They have CONFIG_DEFAULT_SECURITY="apparmor". But I'm wondering why
>> this problem is not occurring on linux-next.git when this problem is occurring
>> on bpf.git / bpf-next.git / net.git / net-next.git trees. Is syzbot running
>> different testcases depending on which git tree is targeted?
>>
>
this is another reason that it is doubtful that its apparmor.
^ permalink raw reply
* Re: [PATCH bpf-next 0/5] Introduce libbpf_attach_type_by_name
From: Jakub Kicinski @ 2018-09-26 23:20 UTC (permalink / raw)
To: Andrey Ignatov; +Cc: netdev, ast, daniel, kernel-team
In-Reply-To: <cover.1538000102.git.rdna@fb.com>
On Wed, 26 Sep 2018 15:24:52 -0700, Andrey Ignatov wrote:
> This patch set introduces libbpf_attach_type_by_name function in libbpf to
> identify attach type by section name.
>
> This is useful to avoid writing same logic over and over again in user
> space applications that leverage libbpf.
>
> Patch 1 has more details on the new function and problem being solved.
> Patches 2 and 3 add support for new section names.
> Patch 4 uses new function in a selftest.
> Patch 5 adds selftest for libbpf_{prog,attach}_type_by_name.
>
> As a side note there are a lot of inconsistencies now between names used by
> libbpf and bpftool (e.g. cgroup/skb vs cgroup_skb, cgroup_device and device
> vs cgroup/dev, sockops vs sock_ops, etc). This patch set does not address
> it but it tries not to make it harder to address it in the future.
I was wondering a few times whether I should point it out to people
during review, but thought it would be nit picking. Maybe we should be
more strict.
Your series LGTM!
^ permalink raw reply
* Re: [PATCH v2 2/7] include: add setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64 in linux/setbits.h
From: LABBE Corentin @ 2018-09-27 5:35 UTC (permalink / raw)
To: Christophe LEROY
Cc: Gilles.Muller, Julia.Lawall, agust, airlied, alexandre.torgue,
alistair, benh, carlo, davem, galak, joabreu, khilman,
maxime.ripard, michal.lkml, mpe, mporter, narmstrong,
nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb, wens,
netdev, linux-kernel, dri-devel, linux-ide, linux-amlogic,
linuxppc-dev, cocci, linux-arm-kernel
In-Reply-To: <4a63152f-9eca-f7d9-8fe6-59caaab33666@c-s.fr>
On Tue, Sep 25, 2018 at 07:05:00AM +0200, Christophe LEROY wrote:
>
>
> Le 24/09/2018 à 21:04, Corentin Labbe a écrit :
> > This patch adds setbits32/clrbits32/clrsetbits32 and
> > setbits64/clrbits64/clrsetbits64 in linux/setbits.h header.
>
> Fix the patch subject and description.
>
> >
> > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> > ---
> > include/linux/setbits.h | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 88 insertions(+)
> > create mode 100644 include/linux/setbits.h
> >
> > diff --git a/include/linux/setbits.h b/include/linux/setbits.h
> > new file mode 100644
> > index 000000000000..6e7e257134ae
> > --- /dev/null
> > +++ b/include/linux/setbits.h
> > @@ -0,0 +1,88 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __LINUX_SETBITS_H
> > +#define __LINUX_SETBITS_H
> > +
> > +#include <linux/io.h>
> > +
> > +#define __setbits(readfunction, writefunction, addr, set) \
> > + writefunction((readfunction(addr) | (set)), addr)
>
> You don't need so long names for parameters in a 2 lines macro (See
> Linux Kernel Codying style §4 Naming).
>
> A single line macro would be feasible with only 3 chars names:
>
> #define __setbits(rfn, wfn, addr, set) wfn((rfn(addr) | (set)), addr)
>
Thanks I will fix all reported problem.
> > +#define __clrbits(readfunction, writefunction, addr, mask) \
> > + writefunction((readfunction(addr) & ~(mask)), addr)
> > +#define __clrsetbits(readfunction, writefunction, addr, mask, set) \
> > + writefunction(((readfunction(addr) & ~(mask)) | (set)), addr)
> > +#define __setclrbits(readfunction, writefunction, addr, mask, set) \
> > + writefunction(((readfunction(addr) | (set)) & ~(mask)), addr)
> > +
> > +#ifndef setbits_le32
> > +#define setbits_le32(addr, set) __setbits(readl, writel, addr, set)
> > +#endif
> > +#ifndef setbits_le32_relaxed
> > +#define setbits_le32_relaxed(addr, set) __setbits(readl_relaxed, writel_relaxed, \
> > + addr, set)
> > +#endif
> > +
> > +#ifndef clrbits_le32
> > +#define clrbits_le32(addr, mask) __clrbits(readl, writel, addr, mask)
> > +#endif
> > +#ifndef clrbits_le32_relaxed
> > +#define clrbits_le32_relaxed(addr, mask) __clrbits(readl_relaxed, writel_relaxed, \
> > + addr, mask)
> > +#endif
> > +
> > +#ifndef clrsetbits_le32
> > +#define clrsetbits_le32(addr, mask, set) __clrsetbits(readl, writel, addr, mask, set)
> > +#endif
> > +#ifndef clrsetbits_le32_relaxed
> > +#define clrsetbits_le32_relaxed(addr, mask, set) __clrsetbits(readl_relaxed, \
> > + writel_relaxed, \
> > + addr, mask, set)
> > +#endif
> > +
> > +#ifndef setclrbits_le32
> > +#define setclrbits_le32(addr, mask, set) __setclrbits(readl, writel, addr, mask, set)
> > +#endif
> > +#ifndef setclrbits_le32_relaxed
> > +#define setclrbits_le32_relaxed(addr, mask, set) __setclrbits(readl_relaxed, \
> > + writel_relaxed, \
> > + addr, mask, set)
> > +#endif
> > +
> > +/* We cannot use CONFIG_64BIT as some x86 drivers use non-atomicwriteq() */
> > +#if defined(writeq) && defined(readq)
>
> Take care. At least Alpha Arch defines it as a static inline without
> redefining it as a #define. (see arch/alpha/kernel/io.c)
In fact, it does in arch/alpha/include/asm/io.h along with a gentle comment.
But fixing their comment will be another interesting patch serie.
Regards
Corentin Labbe
^ permalink raw reply
* Re: [PATCH v2 1/7] powerpc: rename setbits32/clrbits32 to setbits32_be/clrbits32_be
From: LABBE Corentin @ 2018-09-27 5:30 UTC (permalink / raw)
To: Christophe LEROY
Cc: Gilles.Muller, Julia.Lawall, agust, airlied, alexandre.torgue,
alistair, benh, carlo, davem, galak, joabreu, khilman,
maxime.ripard, michal.lkml, mpe, mporter, narmstrong,
nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb, wens,
netdev, linux-kernel, dri-devel, linux-ide, linux-amlogic,
linuxppc-dev, cocci, linux-arm-kernel
In-Reply-To: <eeaeff10-dcc9-ab50-5d54-4d727f12d584@c-s.fr>
On Tue, Sep 25, 2018 at 06:56:23AM +0200, Christophe LEROY wrote:
> Fix the patch title.
>
>
> Le 24/09/2018 à 21:04, Corentin Labbe a écrit :
> > Since setbits32/clrbits32 work on be32, it's better to remove ambiguity on
> > the used data type.
> >
> > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> > ---
> > arch/powerpc/include/asm/fsl_lbc.h | 2 +-
> > arch/powerpc/include/asm/io.h | 5 +-
> > arch/powerpc/platforms/44x/canyonlands.c | 4 +-
> > arch/powerpc/platforms/4xx/gpio.c | 28 ++++-----
> > arch/powerpc/platforms/512x/pdm360ng.c | 6 +-
> > arch/powerpc/platforms/52xx/mpc52xx_common.c | 6 +-
> > arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 10 ++--
> > arch/powerpc/platforms/82xx/ep8248e.c | 2 +-
> > arch/powerpc/platforms/82xx/km82xx.c | 6 +-
> > arch/powerpc/platforms/82xx/mpc8272_ads.c | 10 ++--
> > arch/powerpc/platforms/82xx/pq2.c | 2 +-
> > arch/powerpc/platforms/82xx/pq2ads-pci-pic.c | 4 +-
> > arch/powerpc/platforms/82xx/pq2fads.c | 10 ++--
> > arch/powerpc/platforms/83xx/km83xx.c | 6 +-
> > arch/powerpc/platforms/83xx/mpc836x_mds.c | 2 +-
> > arch/powerpc/platforms/85xx/mpc85xx_mds.c | 2 +-
> > arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c | 4 +-
> > arch/powerpc/platforms/85xx/mpc85xx_rdb.c | 2 +-
> > arch/powerpc/platforms/85xx/p1022_ds.c | 4 +-
> > arch/powerpc/platforms/85xx/p1022_rdk.c | 4 +-
> > arch/powerpc/platforms/85xx/t1042rdb_diu.c | 4 +-
> > arch/powerpc/platforms/85xx/twr_p102x.c | 2 +-
> > arch/powerpc/platforms/86xx/mpc8610_hpcd.c | 4 +-
> > arch/powerpc/platforms/8xx/adder875.c | 2 +-
> > arch/powerpc/platforms/8xx/m8xx_setup.c | 4 +-
> > arch/powerpc/platforms/8xx/mpc86xads_setup.c | 4 +-
> > arch/powerpc/platforms/8xx/mpc885ads_setup.c | 28 ++++-----
> > arch/powerpc/platforms/embedded6xx/flipper-pic.c | 6 +-
> > arch/powerpc/platforms/embedded6xx/hlwd-pic.c | 8 +--
> > arch/powerpc/platforms/embedded6xx/wii.c | 10 ++--
> > arch/powerpc/sysdev/cpm1.c | 26 ++++-----
> > arch/powerpc/sysdev/cpm2.c | 16 ++---
> > arch/powerpc/sysdev/cpm_common.c | 4 +-
> > arch/powerpc/sysdev/fsl_85xx_l2ctlr.c | 8 +--
> > arch/powerpc/sysdev/fsl_lbc.c | 2 +-
> > arch/powerpc/sysdev/fsl_pci.c | 8 +--
> > arch/powerpc/sysdev/fsl_pmc.c | 2 +-
> > arch/powerpc/sysdev/fsl_rcpm.c | 74 ++++++++++++------------
> > arch/powerpc/sysdev/fsl_rio.c | 4 +-
> > arch/powerpc/sysdev/fsl_rmu.c | 8 +--
> > arch/powerpc/sysdev/mpic_timer.c | 12 ++--
> > 41 files changed, 178 insertions(+), 177 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/fsl_lbc.h b/arch/powerpc/include/asm/fsl_lbc.h
> > index c7240a024b96..4d6a56b48a28 100644
> > --- a/arch/powerpc/include/asm/fsl_lbc.h
> > +++ b/arch/powerpc/include/asm/fsl_lbc.h
> > @@ -276,7 +276,7 @@ static inline void fsl_upm_start_pattern(struct fsl_upm *upm, u8 pat_offset)
> > */
> > static inline void fsl_upm_end_pattern(struct fsl_upm *upm)
> > {
> > - clrbits32(upm->mxmr, MxMR_OP_RP);
> > + clrbits_be32(upm->mxmr, MxMR_OP_RP);
> >
> > while (in_be32(upm->mxmr) & MxMR_OP_RP)
> > cpu_relax();
> > diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
> > index e0331e754568..57486a1b9992 100644
> > --- a/arch/powerpc/include/asm/io.h
> > +++ b/arch/powerpc/include/asm/io.h
> > @@ -873,8 +873,8 @@ static inline void * bus_to_virt(unsigned long address)
> > #endif /* CONFIG_PPC32 */
> >
> > /* access ports */
> > -#define setbits32(_addr, _v) out_be32((_addr), in_be32(_addr) | (_v))
> > -#define clrbits32(_addr, _v) out_be32((_addr), in_be32(_addr) & ~(_v))
> > +#define setbits_be32(_addr, _v) out_be32((_addr), in_be32(_addr) | (_v))
> > +#define clrbits_be32(_addr, _v) out_be32((_addr), in_be32(_addr) & ~(_v))
> >
> > #define setbits16(_addr, _v) out_be16((_addr), in_be16(_addr) | (_v))
> > #define clrbits16(_addr, _v) out_be16((_addr), in_be16(_addr) & ~(_v))
> > @@ -904,6 +904,7 @@ static inline void * bus_to_virt(unsigned long address)
> > #define clrsetbits_le16(addr, clear, set) clrsetbits(le16, addr, clear, set)
> >
> > #define clrsetbits_8(addr, clear, set) clrsetbits(8, addr, clear, set)
> > +#define clrsetbits_be32(addr, clear, set) clrsetbits(be32, addr, clear, set)
>
> This one already exists a few lines above.
>
> >
> > #endif /* __KERNEL__ */
> >
> > diff --git a/arch/powerpc/platforms/44x/canyonlands.c b/arch/powerpc/platforms/44x/canyonlands.c
> > index 157f4ce46386..6aeb4ca64d09 100644
> > --- a/arch/powerpc/platforms/44x/canyonlands.c
> > +++ b/arch/powerpc/platforms/44x/canyonlands.c
> > @@ -113,8 +113,8 @@ static int __init ppc460ex_canyonlands_fixup(void)
> > * USB2HStop and gpio19 will be USB2DStop. For more details refer to
> > * table 34-7 of PPC460EX user manual.
> > */
> > - setbits32((vaddr + GPIO0_OSRH), 0x42000000);
> > - setbits32((vaddr + GPIO0_TSRH), 0x42000000);
> > + setbits_be32((vaddr + GPIO0_OSRH), 0x42000000);
> > + setbits_be32((vaddr + GPIO0_TSRH), 0x42000000);
> > err_gpio:
> > iounmap(vaddr);
> > err_bcsr:
> > diff --git a/arch/powerpc/platforms/4xx/gpio.c b/arch/powerpc/platforms/4xx/gpio.c
> > index 2238e369cde4..8436da0617fd 100644
> > --- a/arch/powerpc/platforms/4xx/gpio.c
> > +++ b/arch/powerpc/platforms/4xx/gpio.c
> > @@ -82,9 +82,9 @@ __ppc4xx_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> > struct ppc4xx_gpio __iomem *regs = mm_gc->regs;
> >
> > if (val)
> > - setbits32(®s->or, GPIO_MASK(gpio));
> > + setbits_be32(®s->or, GPIO_MASK(gpio));
> > else
> > - clrbits32(®s->or, GPIO_MASK(gpio));
> > + clrbits_be32(®s->or, GPIO_MASK(gpio));
> > }
> >
> > static void
> > @@ -112,18 +112,18 @@ static int ppc4xx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> > spin_lock_irqsave(&chip->lock, flags);
> >
> > /* Disable open-drain function */
> > - clrbits32(®s->odr, GPIO_MASK(gpio));
> > + clrbits_be32(®s->odr, GPIO_MASK(gpio));
> >
> > /* Float the pin */
> > - clrbits32(®s->tcr, GPIO_MASK(gpio));
> > + clrbits_be32(®s->tcr, GPIO_MASK(gpio));
> >
> > /* Bits 0-15 use TSRL/OSRL, bits 16-31 use TSRH/OSRH */
> > if (gpio < 16) {
> > - clrbits32(®s->osrl, GPIO_MASK2(gpio));
> > - clrbits32(®s->tsrl, GPIO_MASK2(gpio));
> > + clrbits_be32(®s->osrl, GPIO_MASK2(gpio));
> > + clrbits_be32(®s->tsrl, GPIO_MASK2(gpio));
> > } else {
> > - clrbits32(®s->osrh, GPIO_MASK2(gpio));
> > - clrbits32(®s->tsrh, GPIO_MASK2(gpio));
> > + clrbits_be32(®s->osrh, GPIO_MASK2(gpio));
> > + clrbits_be32(®s->tsrh, GPIO_MASK2(gpio));
> > }
> >
> > spin_unlock_irqrestore(&chip->lock, flags);
> > @@ -145,18 +145,18 @@ ppc4xx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> > __ppc4xx_gpio_set(gc, gpio, val);
> >
> > /* Disable open-drain function */
> > - clrbits32(®s->odr, GPIO_MASK(gpio));
> > + clrbits_be32(®s->odr, GPIO_MASK(gpio));
> >
> > /* Drive the pin */
> > - setbits32(®s->tcr, GPIO_MASK(gpio));
> > + setbits_be32(®s->tcr, GPIO_MASK(gpio));
> >
> > /* Bits 0-15 use TSRL, bits 16-31 use TSRH */
> > if (gpio < 16) {
> > - clrbits32(®s->osrl, GPIO_MASK2(gpio));
> > - clrbits32(®s->tsrl, GPIO_MASK2(gpio));
> > + clrbits_be32(®s->osrl, GPIO_MASK2(gpio));
> > + clrbits_be32(®s->tsrl, GPIO_MASK2(gpio));
> > } else {
> > - clrbits32(®s->osrh, GPIO_MASK2(gpio));
> > - clrbits32(®s->tsrh, GPIO_MASK2(gpio));
> > + clrbits_be32(®s->osrh, GPIO_MASK2(gpio));
> > + clrbits_be32(®s->tsrh, GPIO_MASK2(gpio));
> > }
> >
> > spin_unlock_irqrestore(&chip->lock, flags);
> > diff --git a/arch/powerpc/platforms/512x/pdm360ng.c b/arch/powerpc/platforms/512x/pdm360ng.c
> > index dc81f05e0bce..06b95795267a 100644
> > --- a/arch/powerpc/platforms/512x/pdm360ng.c
> > +++ b/arch/powerpc/platforms/512x/pdm360ng.c
> > @@ -38,7 +38,7 @@ static int pdm360ng_get_pendown_state(void)
> >
> > reg = in_be32(pdm360ng_gpio_base + 0xc);
> > if (reg & 0x40)
> > - setbits32(pdm360ng_gpio_base + 0xc, 0x40);
> > + setbits_be32(pdm360ng_gpio_base + 0xc, 0x40);
> >
> > reg = in_be32(pdm360ng_gpio_base + 0x8);
> >
> > @@ -69,8 +69,8 @@ static int __init pdm360ng_penirq_init(void)
> > return -ENODEV;
> > }
> > out_be32(pdm360ng_gpio_base + 0xc, 0xffffffff);
> > - setbits32(pdm360ng_gpio_base + 0x18, 0x2000);
> > - setbits32(pdm360ng_gpio_base + 0x10, 0x40);
> > + setbits_be32(pdm360ng_gpio_base + 0x18, 0x2000);
> > + setbits_be32(pdm360ng_gpio_base + 0x10, 0x40);
> >
> > return 0;
> > }
> > diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c b/arch/powerpc/platforms/52xx/mpc52xx_common.c
> > index 565e3a83dc9e..edfe619d67bf 100644
> > --- a/arch/powerpc/platforms/52xx/mpc52xx_common.c
> > +++ b/arch/powerpc/platforms/52xx/mpc52xx_common.c
> > @@ -314,13 +314,13 @@ int mpc5200_psc_ac97_gpio_reset(int psc_number)
> >
> > /* enable gpio pins for output */
> > setbits8(&wkup_gpio->wkup_gpioe, reset);
> > - setbits32(&simple_gpio->simple_gpioe, sync | out);
> > + setbits_be32(&simple_gpio->simple_gpioe, sync | out);
> >
> > setbits8(&wkup_gpio->wkup_ddr, reset);
> > - setbits32(&simple_gpio->simple_ddr, sync | out);
> > + setbits_be32(&simple_gpio->simple_ddr, sync | out);
> >
> > /* Assert cold reset */
> > - clrbits32(&simple_gpio->simple_dvo, sync | out);
> > + clrbits_be32(&simple_gpio->simple_dvo, sync | out);
> > clrbits8(&wkup_gpio->wkup_dvo, reset);
> >
> > /* wait for 1 us */
> > diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> > index 17cf249b18ee..e9f4dec06077 100644
> > --- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> > +++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> > @@ -142,7 +142,7 @@ static void mpc52xx_gpt_irq_unmask(struct irq_data *d)
> > unsigned long flags;
> >
> > raw_spin_lock_irqsave(&gpt->lock, flags);
> > - setbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
> > + setbits_be32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
> > raw_spin_unlock_irqrestore(&gpt->lock, flags);
> > }
> >
> > @@ -152,7 +152,7 @@ static void mpc52xx_gpt_irq_mask(struct irq_data *d)
> > unsigned long flags;
> >
> > raw_spin_lock_irqsave(&gpt->lock, flags);
> > - clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
> > + clrbits_be32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
> > raw_spin_unlock_irqrestore(&gpt->lock, flags);
> > }
> >
> > @@ -308,7 +308,7 @@ static int mpc52xx_gpt_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> > dev_dbg(gpt->dev, "%s: gpio:%d\n", __func__, gpio);
> >
> > raw_spin_lock_irqsave(&gpt->lock, flags);
> > - clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_GPIO_MASK);
> > + clrbits_be32(&gpt->regs->mode, MPC52xx_GPT_MODE_GPIO_MASK);
> > raw_spin_unlock_irqrestore(&gpt->lock, flags);
> >
> > return 0;
> > @@ -482,7 +482,7 @@ int mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt)
> > return -EBUSY;
> > }
> >
> > - clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_COUNTER_ENABLE);
> > + clrbits_be32(&gpt->regs->mode, MPC52xx_GPT_MODE_COUNTER_ENABLE);
> > raw_spin_unlock_irqrestore(&gpt->lock, flags);
> > return 0;
> > }
> > @@ -639,7 +639,7 @@ static int mpc52xx_wdt_release(struct inode *inode, struct file *file)
> > unsigned long flags;
> >
> > raw_spin_lock_irqsave(&gpt_wdt->lock, flags);
> > - clrbits32(&gpt_wdt->regs->mode,
> > + clrbits_be32(&gpt_wdt->regs->mode,
> > MPC52xx_GPT_MODE_COUNTER_ENABLE | MPC52xx_GPT_MODE_WDT_EN);
>
> The alignment needs to be fixed here (and all other places). The second
> line should start under the &
> Eventually use checkpatch to locate all places that need to be fixed.
> (checkpatch may even fix it for you)
>
Thanks, I will fix all reported problem
^ permalink raw reply
* [PATCH net-next] nfp: warn on experimental TLV types
From: Jakub Kicinski @ 2018-09-26 22:35 UTC (permalink / raw)
To: davem; +Cc: netdev, oss-drivers, Jakub Kicinski
Reserve two TLV types for feature development, and warn in the driver
if they ever leak into production.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.c | 7 +++++++
drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h | 8 ++++++++
2 files changed, 15 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.c
index 1f9149bb2ae6..2190836eaa1d 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.c
@@ -113,6 +113,13 @@ int nfp_net_tlv_caps_parse(struct device *dev, u8 __iomem *ctrl_mem,
caps->mbox_len = length;
}
break;
+ case NFP_NET_CFG_TLV_TYPE_EXPERIMENTAL0:
+ case NFP_NET_CFG_TLV_TYPE_EXPERIMENTAL1:
+ dev_warn(dev,
+ "experimental TLV type:%u offset:%u len:%u\n",
+ FIELD_GET(NFP_NET_CFG_TLV_HEADER_TYPE, hdr),
+ offset, length);
+ break;
default:
if (!FIELD_GET(NFP_NET_CFG_TLV_HEADER_REQUIRED, hdr))
break;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h b/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h
index 44d3ea75d043..a51490747689 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h
@@ -489,12 +489,20 @@
* %NFP_NET_CFG_TLV_TYPE_MBOX:
* Variable, mailbox area. Overwrites the default location which is
* %NFP_NET_CFG_MBOX_BASE and length %NFP_NET_CFG_MBOX_VAL_MAX_SZ.
+ *
+ * %NFP_NET_CFG_TLV_TYPE_EXPERIMENTAL0:
+ * %NFP_NET_CFG_TLV_TYPE_EXPERIMENTAL1:
+ * Variable, experimental IDs. IDs designated for internal development and
+ * experiments before a stable TLV ID has been allocated to a feature. Should
+ * never be present in production firmware.
*/
#define NFP_NET_CFG_TLV_TYPE_UNKNOWN 0
#define NFP_NET_CFG_TLV_TYPE_RESERVED 1
#define NFP_NET_CFG_TLV_TYPE_END 2
#define NFP_NET_CFG_TLV_TYPE_ME_FREQ 3
#define NFP_NET_CFG_TLV_TYPE_MBOX 4
+#define NFP_NET_CFG_TLV_TYPE_EXPERIMENTAL0 5
+#define NFP_NET_CFG_TLV_TYPE_EXPERIMENTAL1 6
struct device;
--
2.17.1
^ permalink raw reply related
* Re: [PATCHv2 bpf-next 08/11] selftests/bpf: Add tests for reference tracking
From: Joe Stringer @ 2018-09-26 22:31 UTC (permalink / raw)
To: daniel
Cc: Joe Stringer, ast, netdev, john fastabend, tgraf,
Martin KaFai Lau, Nitin Hande, mauricio.vasquez
In-Reply-To: <3bdb6449-41ce-ef1a-b845-c55f6d44775d@iogearbox.net>
On Mon, 24 Sep 2018 at 05:22, Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 09/21/2018 07:10 PM, Joe Stringer wrote:
> > reference tracking: leak potential reference
> > reference tracking: leak potential reference on stack
> > reference tracking: leak potential reference on stack 2
> > reference tracking: zero potential reference
> > reference tracking: copy and zero potential references
> > reference tracking: release reference without check
> > reference tracking: release reference
> > reference tracking: release reference twice
> > reference tracking: release reference twice inside branch
> > reference tracking: alloc, check, free in one subbranch
> > reference tracking: alloc, check, free in both subbranches
> > reference tracking in call: free reference in subprog
> > reference tracking in call: free reference in subprog and outside
> > reference tracking in call: alloc & leak reference in subprog
> > reference tracking in call: alloc in subprog, release outside
> > reference tracking in call: sk_ptr leak into caller stack
> > reference tracking in call: sk_ptr spill into caller stack
> >
> > Signed-off-by: Joe Stringer <joe@wand.net.nz>
> > Acked-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> > tools/testing/selftests/bpf/test_verifier.c | 359 ++++++++++++++++++++
> > 1 file changed, 359 insertions(+)
>
> I think this here needs to have some more test cases that we current do not track but
> should in order to have better coverage. At minimum what comes to mind additionally:
>
> - verifier interaction with LD_ABS, LD_IND
> - verifier interaction with tail calls (e.g. try to leak socket, socket_or_null, etc,
> but should also have a positive test where we drop ref before tail call to show it
> works in combination)
> - Try to mangle a socket and socket_or_null pointer with ALU ops and pass it to helper
> - Try to access the socket data fields after we released its reference
> - Access socket member fields in general (I think not present right now)
> - Use direct packet access in combination with lookup helper (it's enabled
> via pkt_access = true in the helper, so we should also test for it here to make
> sure future changes don't break it)
Great suggestions, I think that the LD_ABS/LD_IND ones are good
candidates for the assembly-level tests here. For the remainder, I
plan to add them to the C cases. That'll be easier to review and
easier to understand if they ever get broken.
Regarding the direct packet access with lookup, that's already in this
series in the C verifier tests patch.
^ 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