* Re: [patch net-next v2 1/3] net: devlink: allow to change namespaces
From: David Ahern @ 2019-08-02 15:45 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Jakub Kicinski, netdev, davem, sthemmin, mlxsw
In-Reply-To: <20190802074838.GC2203@nanopsycho>
On 8/2/19 1:48 AM, Jiri Pirko wrote:
> Wed, Jul 31, 2019 at 09:58:10PM CEST, dsahern@gmail.com wrote:
>> On 7/31/19 1:46 PM, David Ahern wrote:
>>> On 7/31/19 1:45 PM, Jiri Pirko wrote:
>>>>> check. e.g., what happens if a resource controller has been configured
>>>>> for the devlink instance and it is moved to a namespace whose existing
>>>>> config exceeds those limits?
>>>>
>>>> It's moved with all the values. The whole instance is moved.
>>>>
>>>
>>> The values are moved, but the FIB in a namespace could already contain
>>> more routes than the devlink instance allows.
>>>
>>
>>From a quick test your recent refactoring to netdevsim broke the
>> resource controller. It was, and is intended to be, per network namespace.
>
> unifying devlink instances with network namespace in netdevsim was
> really odd. Netdevsim is also a device, like any other. With other
> devices, you do not do this so I don't see why to do this with netdevsim.
>
> Now you create netdevsim instance in sysfs, there is proper bus probe
> mechanism done, there is a devlink instance created for this device,
> there are netdevices and devlink ports created. Same as for the real
> hardware.
>
> Honestly, creating a devlink instance per-network namespace
> automagically, no relation to netdevsim devices, that is simply wrong.
> There should be always 1:1 relationshin between a device and devlink
> instance.
>
Jiri: prior to your recent change netdevsim had a fib resource
controller per network namespace. Please return that behavior or revert
the change.
^ permalink raw reply
* Re: [PATCH iproute2-next] ip tunnel: add json output
From: Stephen Hemminger @ 2019-08-02 15:49 UTC (permalink / raw)
To: Andrea Claudi; +Cc: linux-netdev, David Ahern
In-Reply-To: <CAPpH65wSxeXGQc+r+7W_LRZR=vjnL2bXfub1U10vt-Gni67+kQ@mail.gmail.com>
On Fri, 2 Aug 2019 13:14:15 +0200
Andrea Claudi <aclaudi@redhat.com> wrote:
> On Thu, Aug 1, 2019 at 5:16 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Thu, 1 Aug 2019 12:12:58 +0200
> > Andrea Claudi <aclaudi@redhat.com> wrote:
> >
> > > Add json support on iptunnel and ip6tunnel.
> > > The plain text output format should remain the same.
> > >
> > > Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
> > > ---
> > > ip/ip6tunnel.c | 82 +++++++++++++++++++++++++++++++--------------
> > > ip/iptunnel.c | 90 +++++++++++++++++++++++++++++++++-----------------
> > > ip/tunnel.c | 42 +++++++++++++++++------
> > > 3 files changed, 148 insertions(+), 66 deletions(-)
> > >
> > > diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
> > > index d7684a673fdc4..f2b9710c1320f 100644
> > > --- a/ip/ip6tunnel.c
> > > +++ b/ip/ip6tunnel.c
> > > @@ -71,57 +71,90 @@ static void usage(void)
> > > static void print_tunnel(const void *t)
> > > {
> > > const struct ip6_tnl_parm2 *p = t;
> > > - char s1[1024];
> > > - char s2[1024];
> > > + SPRINT_BUF(b1);
> > >
> > > /* Do not use format_host() for local addr,
> > > * symbolic name will not be useful.
> > > */
> > > - printf("%s: %s/ipv6 remote %s local %s",
> > > - p->name,
> > > - tnl_strproto(p->proto),
> > > - format_host_r(AF_INET6, 16, &p->raddr, s1, sizeof(s1)),
> > > - rt_addr_n2a_r(AF_INET6, 16, &p->laddr, s2, sizeof(s2)));
> > > + open_json_object(NULL);
> > > + print_string(PRINT_ANY, "ifname", "%s: ", p->name);
> >
> > Print this using color for interface name?
>
> Thanks for the suggestion, I can do the same also for remote and local
> addresses?
>
> >
> >
> > > + snprintf(b1, sizeof(b1), "%s/ipv6", tnl_strproto(p->proto));
> > > + print_string(PRINT_ANY, "mode", "%s ", b1);
> > > + print_string(PRINT_ANY,
> > > + "remote",
> > > + "remote %s ",
> > > + format_host_r(AF_INET6, 16, &p->raddr, b1, sizeof(b1)));
> > > + print_string(PRINT_ANY,
> > > + "local",
> > > + "local %s",
> > > + rt_addr_n2a_r(AF_INET6, 16, &p->laddr, b1, sizeof(b1)));
> > > +
> > > if (p->link) {
> > > const char *n = ll_index_to_name(p->link);
> > >
> > > if (n)
> > > - printf(" dev %s", n);
> > > + print_string(PRINT_ANY, "link", " dev %s", n);
> > > }
> > >
> > > if (p->flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
> > > - printf(" encaplimit none");
> > > + print_bool(PRINT_ANY,
> > > + "ip6_tnl_f_ign_encap_limit",
> > > + " encaplimit none",
> > > + true);
> >
> > For flags like this, print_null is more typical JSON than a boolean
> > value. Null is better for presence flag. Bool is better if both true and
> > false are printed.
>
> Using print_null json JSON output becomes:
>
> {
> "ifname": "gre2",
> "mode": "gre/ipv6",
> "remote": "fd::1",
> "local": "::",
> "ip6_tnl_f_ign_encap_limit": null,
> "hoplimit": 64,
> "tclass": "0x00",
> "flowlabel": "0x00000",
> "flowinfo": "0x00000000",
> "flags": []
> }
>
> Which seems a bit confusing to me (what does the "null" value means?).
> Using print_null we also introduce an inconsistency with the JSON
> output of ip/link_gre6.c and ip/link_ip6tnl.c.
> I would prefer to use print_bool and print out
> ip6_tnl_f_ign_encap_limit both when true and false, patching both
> files above to do the same. WDYT?
JSON has several ways to represent the same type of flag value.
Since JSON always has key/value. Null is used to indicate something is present and
in that case the value is unnecessary, which is what the null field was meant for.
^ permalink raw reply
* [PATCH][net-next] ice: fix potential infinite loop
From: Colin King @ 2019-08-02 15:52 UTC (permalink / raw)
To: Jeff Kirsher, David S . Miller, intel-wired-lan, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The loop counter of a for-loop is a u8 however this is being compared
to an int upper bound and this can lead to an infinite loop if the
upper bound is greater than 255 since the loop counter will wrap back
to zero. Fix this potential issue by making the loop counter an int.
Addresses-Coverity: ("Infinite loop")
Fixes: c7aeb4d1b9bf ("ice: Disable VFs until reset is completed")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index c26e6a102dac..088543d50095 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -488,7 +488,7 @@ static void
ice_prepare_for_reset(struct ice_pf *pf)
{
struct ice_hw *hw = &pf->hw;
- u8 i;
+ int i;
/* already prepared for reset */
if (test_bit(__ICE_PREPARED_FOR_RESET, pf->state))
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next 00/15] net: Add functional tests for L3 and L4
From: David Ahern @ 2019-08-02 15:59 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: David Ahern, David S. Miller, Network Development
In-Reply-To: <CAADnVQLLKziv+3oOEijh=woyBZ7KsxoJ8=BB9ax+XJT9wxTuYQ@mail.gmail.com>
On 8/2/19 9:14 AM, Alexei Starovoitov wrote:
> On Thu, Aug 1, 2019 at 9:04 PM David Ahern <dsahern@gmail.com> wrote:
>> ...
>>
>>>
>>> with -v I see:
>>> COMMAND: ip netns exec ns-A ping -c1 -w1 -I 172.16.2.1 172.16.1.2
>>> ping: unknown iface 172.16.2.1
>>> TEST: ping out, address bind - ns-B IP [FAIL]
>>
>> With ping from iputils-ping -I can be an address or a device.
>
> the ping, I have installed, supports -I.
> The issue is somewhere else. Ideas?
>
make sure ping supports the overloading of -I (both dev and address).
check your kernel version. No results guaranteed on kernel prior to 5.3
This is Fedora 29 with 5.1 kernel
TEST: ping out - ns-B IP [ OK ]
TEST: ping out, device bind - ns-B IP [ OK ]
TEST: ping out, address bind - ns-B IP [ OK ]
TEST: ping out - ns-B loopback IP [ OK ]
TEST: ping out, device bind - ns-B loopback IP [ OK ]
TEST: ping out, address bind - ns-B loopback IP [ OK ]
TEST: ping in - ns-A IP [ OK ]
TEST: ping in - ns-A loopback IP [ OK ]
TEST: ping local - ns-A IP [ OK ]
TEST: ping local - ns-A loopback IP [ OK ]
TEST: ping local - loopback [ OK ]
TEST: ping local, device bind - ns-A IP [ OK ]
TEST: ping local, device bind - ns-A loopback IP [ OK ]
TEST: ping local, device bind - loopback [ OK ]
TEST: ping out, blocked by rule - ns-B loopback IP [ OK ]
TEST: ping in, blocked by rule - ns-A loopback IP [ OK ]
TEST: ping out, blocked by route - ns-B loopback IP [ OK ]
TEST: ping in, blocked by route - ns-A loopback IP [ OK ]
TEST: ping out, unreachable default route - ns-B loopback IP [ OK ]
Tests are known to work on Debian stretch and buster. Appears to work on
Fedora 29.
^ permalink raw reply
* Re: [PATCH net-next 00/15] net: Add functional tests for L3 and L4
From: David Ahern @ 2019-08-02 16:00 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: David Ahern, David S. Miller, Network Development
In-Reply-To: <CAADnVQJ7GUX=EWP0RWxpe71cGx2cTMyKHsA+6RRX0P05FPMg3w@mail.gmail.com>
On 8/2/19 9:15 AM, Alexei Starovoitov wrote:
> On Thu, Aug 1, 2019 at 9:11 PM David Ahern <dsahern@gmail.com> wrote:
>>
>> On 8/1/19 6:19 PM, Alexei Starovoitov wrote:
>>> Do you really need 'sleep 1' everywhere?
>>> It makes them so slow to run...
>>> What happens if you just remove it ? Tests will fail? Why?
>>
>> yes, the sleep 1 is needed. A server process is getting launched into
>> the background. It's status is not relevant; the client's is the key.
>> The server needs time to initialize. And, yes I tried forking and it
>> gets hung up with bash and capturing the output for verbose mode.
>
> That means that the tests are not suitable for CI servers
> where cpus are pegged to 100% and multiple tests run in parallel.
> The test will be flaky :(
>
once the tests exist, they can be improved - by me or anyone else that
has the time and interest.
^ permalink raw reply
* RE: [PATCH 20/34] xen: convert put_page() to put_user_page*()
From: Weiny, Ira @ 2019-08-02 16:09 UTC (permalink / raw)
To: Juergen Gross, John Hubbard, john.hubbard@gmail.com,
Andrew Morton
Cc: devel@driverdev.osuosl.org, Dave Chinner, Christoph Hellwig,
Williams, Dan J, x86@kernel.org, linux-mm@kvack.org, Dave Hansen,
amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
intel-gfx@lists.freedesktop.org,
linux-arm-kernel@lists.infradead.org,
linux-rpi-kernel@lists.infradead.org, devel@lists.orangefs.org,
xen-devel@lists.xenproject.org, Boris Ostrovsky,
rds-devel@oss.oracle.com, Jérôme Glisse, Jan Kara,
ceph-devel@vger.kernel.org, kvm@vger.kernel.org,
linux-block@vger.kernel.org, linux-crypto@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-fsdevel@vger.kernel.org, LKML,
linux-media@vger.kernel.org, linux-nfs@vger.kernel.org,
linux-rdma@vger.kernel.org, linux-xfs@vger.kernel.org,
netdev@vger.kernel.org, sparclinux@vger.kernel.org,
Jason Gunthorpe
In-Reply-To: <d4931311-db01-e8c3-0f8c-d64685dc2143@suse.com>
>
> On 02.08.19 07:48, John Hubbard wrote:
> > On 8/1/19 9:36 PM, Juergen Gross wrote:
> >> On 02.08.19 04:19, john.hubbard@gmail.com wrote:
> >>> From: John Hubbard <jhubbard@nvidia.com>
> > ...
> >>> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index
> >>> 2f5ce7230a43..29e461dbee2d 100644
> >>> --- a/drivers/xen/privcmd.c
> >>> +++ b/drivers/xen/privcmd.c
> >>> @@ -611,15 +611,10 @@ static int lock_pages(
> >>> static void unlock_pages(struct page *pages[], unsigned int
> >>> nr_pages)
> >>> {
> >>> - unsigned int i;
> >>> -
> >>> if (!pages)
> >>> return;
> >>> - for (i = 0; i < nr_pages; i++) {
> >>> - if (pages[i])
> >>> - put_page(pages[i]);
> >>> - }
> >>> + put_user_pages(pages, nr_pages);
> >>
> >> You are not handling the case where pages[i] is NULL here. Or am I
> >> missing a pending patch to put_user_pages() here?
> >>
> >
> > Hi Juergen,
> >
> > You are correct--this no longer handles the cases where pages[i] is
> > NULL. It's intentional, though possibly wrong. :)
> >
> > I see that I should have added my standard blurb to this commit
> > description. I missed this one, but some of the other patches have it.
> > It makes the following, possibly incorrect claim:
> >
> > "This changes the release code slightly, because each page slot in the
> > page_list[] array is no longer checked for NULL. However, that check
> > was wrong anyway, because the get_user_pages() pattern of usage here
> > never allowed for NULL entries within a range of pinned pages."
> >
> > The way I've seen these page arrays used with get_user_pages(), things
> > are either done single page, or with a contiguous range. So unless I'm
> > missing a case where someone is either
> >
> > a) releasing individual pages within a range (and thus likely messing
> > up their count of pages they have), or
> >
> > b) allocating two gup ranges within the same pages[] array, with a gap
> > between the allocations,
> >
> > ...then it should be correct. If so, then I'll add the above blurb to
> > this patch's commit description.
> >
> > If that's not the case (both here, and in 3 or 4 other patches in this
> > series, then as you said, I should add NULL checks to put_user_pages()
> > and put_user_pages_dirty_lock().
>
> In this case it is not correct, but can easily be handled. The NULL case can
> occur only in an error case with the pages array filled partially or not at all.
>
> I'd prefer something like the attached patch here.
I'm not an expert in this code and have not looked at it carefully but that patch does seem to be the better fix than forcing NULL checks on everyone.
Ira
^ permalink raw reply
* Re: [PATCH] net/mlx4_core: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 16:10 UTC (permalink / raw)
Cc: Tariq Toukan, David S . Miller, Netdev, linux-rdma, LKML
In-Reply-To: <20190802121020.1181-1-hslester96@gmail.com>
Chuhong Yuan <hslester96@gmail.com> 于2019年8月2日周五 下午8:10写道：
>
> refcount_t is better for reference counters since its
> implementation can prevent overflows.
> So convert atomic_t ref counters to refcount_t.
>
> Also convert refcount from 0-based to 1-based.
>
It seems that directly converting refcount from 0-based
to 1-based is infeasible.
I am sorry for this mistake.
Regards,
Chuhong
> Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> ---
> .../ethernet/mellanox/mlx4/resource_tracker.c | 90 +++++++++----------
> 1 file changed, 45 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> index 4356f3a58002..d7e26935fd76 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> @@ -114,7 +114,7 @@ struct res_qp {
> struct list_head mcg_list;
> spinlock_t mcg_spl;
> int local_qpn;
> - atomic_t ref_count;
> + refcount_t ref_count;
> u32 qpc_flags;
> /* saved qp params before VST enforcement in order to restore on VGT */
> u8 sched_queue;
> @@ -143,7 +143,7 @@ static inline const char *mtt_states_str(enum res_mtt_states state)
> struct res_mtt {
> struct res_common com;
> int order;
> - atomic_t ref_count;
> + refcount_t ref_count;
> };
>
> enum res_mpt_states {
> @@ -179,7 +179,7 @@ enum res_cq_states {
> struct res_cq {
> struct res_common com;
> struct res_mtt *mtt;
> - atomic_t ref_count;
> + refcount_t ref_count;
> };
>
> enum res_srq_states {
> @@ -192,7 +192,7 @@ struct res_srq {
> struct res_common com;
> struct res_mtt *mtt;
> struct res_cq *cq;
> - atomic_t ref_count;
> + refcount_t ref_count;
> };
>
> enum res_counter_states {
> @@ -1050,7 +1050,7 @@ static struct res_common *alloc_qp_tr(int id)
> ret->local_qpn = id;
> INIT_LIST_HEAD(&ret->mcg_list);
> spin_lock_init(&ret->mcg_spl);
> - atomic_set(&ret->ref_count, 0);
> + refcount_set(&ret->ref_count, 1);
>
> return &ret->com;
> }
> @@ -1066,7 +1066,7 @@ static struct res_common *alloc_mtt_tr(int id, int order)
> ret->com.res_id = id;
> ret->order = order;
> ret->com.state = RES_MTT_ALLOCATED;
> - atomic_set(&ret->ref_count, 0);
> + refcount_set(&ret->ref_count, 1);
>
> return &ret->com;
> }
> @@ -1110,7 +1110,7 @@ static struct res_common *alloc_cq_tr(int id)
>
> ret->com.res_id = id;
> ret->com.state = RES_CQ_ALLOCATED;
> - atomic_set(&ret->ref_count, 0);
> + refcount_set(&ret->ref_count, 1);
>
> return &ret->com;
> }
> @@ -1125,7 +1125,7 @@ static struct res_common *alloc_srq_tr(int id)
>
> ret->com.res_id = id;
> ret->com.state = RES_SRQ_ALLOCATED;
> - atomic_set(&ret->ref_count, 0);
> + refcount_set(&ret->ref_count, 1);
>
> return &ret->com;
> }
> @@ -1325,10 +1325,10 @@ static int add_res_range(struct mlx4_dev *dev, int slave, u64 base, int count,
>
> static int remove_qp_ok(struct res_qp *res)
> {
> - if (res->com.state == RES_QP_BUSY || atomic_read(&res->ref_count) ||
> + if (res->com.state == RES_QP_BUSY || refcount_read(&res->ref_count) != 1 ||
> !list_empty(&res->mcg_list)) {
> pr_err("resource tracker: fail to remove qp, state %d, ref_count %d\n",
> - res->com.state, atomic_read(&res->ref_count));
> + res->com.state, refcount_read(&res->ref_count));
> return -EBUSY;
> } else if (res->com.state != RES_QP_RESERVED) {
> return -EPERM;
> @@ -1340,11 +1340,11 @@ static int remove_qp_ok(struct res_qp *res)
> static int remove_mtt_ok(struct res_mtt *res, int order)
> {
> if (res->com.state == RES_MTT_BUSY ||
> - atomic_read(&res->ref_count)) {
> + refcount_read(&res->ref_count) != 1) {
> pr_devel("%s-%d: state %s, ref_count %d\n",
> __func__, __LINE__,
> mtt_states_str(res->com.state),
> - atomic_read(&res->ref_count));
> + refcount_read(&res->ref_count));
> return -EBUSY;
> } else if (res->com.state != RES_MTT_ALLOCATED)
> return -EPERM;
> @@ -1675,7 +1675,7 @@ static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
> } else if (state == RES_CQ_ALLOCATED) {
> if (r->com.state != RES_CQ_HW)
> err = -EINVAL;
> - else if (atomic_read(&r->ref_count))
> + else if (refcount_read(&r->ref_count) != 1)
> err = -EBUSY;
> else
> err = 0;
> @@ -1715,7 +1715,7 @@ static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
> } else if (state == RES_SRQ_ALLOCATED) {
> if (r->com.state != RES_SRQ_HW)
> err = -EINVAL;
> - else if (atomic_read(&r->ref_count))
> + else if (refcount_read(&r->ref_count) != 1)
> err = -EBUSY;
> } else if (state != RES_SRQ_HW || r->com.state != RES_SRQ_ALLOCATED) {
> err = -EINVAL;
> @@ -2808,7 +2808,7 @@ int mlx4_SW2HW_MPT_wrapper(struct mlx4_dev *dev, int slave,
> goto ex_put;
>
> if (!phys) {
> - atomic_inc(&mtt->ref_count);
> + refcount_inc(&mtt->ref_count);
> put_res(dev, slave, mtt->com.res_id, RES_MTT);
> }
>
> @@ -2845,7 +2845,7 @@ int mlx4_HW2SW_MPT_wrapper(struct mlx4_dev *dev, int slave,
> goto ex_abort;
>
> if (mpt->mtt)
> - atomic_dec(&mpt->mtt->ref_count);
> + refcount_dec(&mpt->mtt->ref_count);
>
> res_end_move(dev, slave, RES_MPT, id);
> return 0;
> @@ -3007,18 +3007,18 @@ int mlx4_RST2INIT_QP_wrapper(struct mlx4_dev *dev, int slave,
> err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
> if (err)
> goto ex_put_srq;
> - atomic_inc(&mtt->ref_count);
> + refcount_inc(&mtt->ref_count);
> qp->mtt = mtt;
> - atomic_inc(&rcq->ref_count);
> + refcount_inc(&rcq->ref_count);
> qp->rcq = rcq;
> - atomic_inc(&scq->ref_count);
> + refcount_inc(&scq->ref_count);
> qp->scq = scq;
>
> if (scqn != rcqn)
> put_res(dev, slave, scqn, RES_CQ);
>
> if (use_srq) {
> - atomic_inc(&srq->ref_count);
> + refcount_inc(&srq->ref_count);
> put_res(dev, slave, srqn, RES_SRQ);
> qp->srq = srq;
> }
> @@ -3113,7 +3113,7 @@ int mlx4_SW2HW_EQ_wrapper(struct mlx4_dev *dev, int slave,
> if (err)
> goto out_put;
>
> - atomic_inc(&mtt->ref_count);
> + refcount_inc(&mtt->ref_count);
> eq->mtt = mtt;
> put_res(dev, slave, mtt->com.res_id, RES_MTT);
> res_end_move(dev, slave, RES_EQ, res_id);
> @@ -3310,7 +3310,7 @@ int mlx4_HW2SW_EQ_wrapper(struct mlx4_dev *dev, int slave,
> if (err)
> goto ex_put;
>
> - atomic_dec(&eq->mtt->ref_count);
> + refcount_dec(&eq->mtt->ref_count);
> put_res(dev, slave, eq->mtt->com.res_id, RES_MTT);
> res_end_move(dev, slave, RES_EQ, res_id);
> rem_res_range(dev, slave, res_id, 1, RES_EQ, 0);
> @@ -3445,7 +3445,7 @@ int mlx4_SW2HW_CQ_wrapper(struct mlx4_dev *dev, int slave,
> err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
> if (err)
> goto out_put;
> - atomic_inc(&mtt->ref_count);
> + refcount_inc(&mtt->ref_count);
> cq->mtt = mtt;
> put_res(dev, slave, mtt->com.res_id, RES_MTT);
> res_end_move(dev, slave, RES_CQ, cqn);
> @@ -3474,7 +3474,7 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev, int slave,
> err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
> if (err)
> goto out_move;
> - atomic_dec(&cq->mtt->ref_count);
> + refcount_dec(&cq->mtt->ref_count);
> res_end_move(dev, slave, RES_CQ, cqn);
> return 0;
>
> @@ -3539,9 +3539,9 @@ static int handle_resize(struct mlx4_dev *dev, int slave,
> err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
> if (err)
> goto ex_put1;
> - atomic_dec(&orig_mtt->ref_count);
> + refcount_dec(&orig_mtt->ref_count);
> put_res(dev, slave, orig_mtt->com.res_id, RES_MTT);
> - atomic_inc(&mtt->ref_count);
> + refcount_inc(&mtt->ref_count);
> cq->mtt = mtt;
> put_res(dev, slave, mtt->com.res_id, RES_MTT);
> return 0;
> @@ -3627,7 +3627,7 @@ int mlx4_SW2HW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
> if (err)
> goto ex_put_mtt;
>
> - atomic_inc(&mtt->ref_count);
> + refcount_inc(&mtt->ref_count);
> srq->mtt = mtt;
> put_res(dev, slave, mtt->com.res_id, RES_MTT);
> res_end_move(dev, slave, RES_SRQ, srqn);
> @@ -3657,9 +3657,9 @@ int mlx4_HW2SW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
> err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
> if (err)
> goto ex_abort;
> - atomic_dec(&srq->mtt->ref_count);
> + refcount_dec(&srq->mtt->ref_count);
> if (srq->cq)
> - atomic_dec(&srq->cq->ref_count);
> + refcount_dec(&srq->cq->ref_count);
> res_end_move(dev, slave, RES_SRQ, srqn);
>
> return 0;
> @@ -3988,11 +3988,11 @@ int mlx4_2RST_QP_wrapper(struct mlx4_dev *dev, int slave,
> if (err)
> goto ex_abort;
>
> - atomic_dec(&qp->mtt->ref_count);
> - atomic_dec(&qp->rcq->ref_count);
> - atomic_dec(&qp->scq->ref_count);
> + refcount_dec(&qp->mtt->ref_count);
> + refcount_dec(&qp->rcq->ref_count);
> + refcount_dec(&qp->scq->ref_count);
> if (qp->srq)
> - atomic_dec(&qp->srq->ref_count);
> + refcount_dec(&qp->srq->ref_count);
> res_end_move(dev, slave, RES_QP, qpn);
> return 0;
>
> @@ -4456,7 +4456,7 @@ int mlx4_QP_FLOW_STEERING_ATTACH_wrapper(struct mlx4_dev *dev, int slave,
> if (mlx4_is_bonded(dev))
> mlx4_do_mirror_rule(dev, rrule);
>
> - atomic_inc(&rqp->ref_count);
> + refcount_inc(&rqp->ref_count);
>
> err_put_rule:
> put_res(dev, slave, vhcr->out_param, RES_FS_RULE);
> @@ -4540,7 +4540,7 @@ int mlx4_QP_FLOW_STEERING_DETACH_wrapper(struct mlx4_dev *dev, int slave,
> MLX4_QP_FLOW_STEERING_DETACH, MLX4_CMD_TIME_CLASS_A,
> MLX4_CMD_NATIVE);
> if (!err)
> - atomic_dec(&rqp->ref_count);
> + refcount_dec(&rqp->ref_count);
> out:
> put_res(dev, slave, qpn, RES_QP);
> return err;
> @@ -4702,11 +4702,11 @@ static void rem_slave_qps(struct mlx4_dev *dev, int slave)
> if (err)
> mlx4_dbg(dev, "rem_slave_qps: failed to move slave %d qpn %d to reset\n",
> slave, qp->local_qpn);
> - atomic_dec(&qp->rcq->ref_count);
> - atomic_dec(&qp->scq->ref_count);
> - atomic_dec(&qp->mtt->ref_count);
> + refcount_dec(&qp->rcq->ref_count);
> + refcount_dec(&qp->scq->ref_count);
> + refcount_dec(&qp->mtt->ref_count);
> if (qp->srq)
> - atomic_dec(&qp->srq->ref_count);
> + refcount_dec(&qp->srq->ref_count);
> state = RES_QP_MAPPED;
> break;
> default:
> @@ -4768,9 +4768,9 @@ static void rem_slave_srqs(struct mlx4_dev *dev, int slave)
> mlx4_dbg(dev, "rem_slave_srqs: failed to move slave %d srq %d to SW ownership\n",
> slave, srqn);
>
> - atomic_dec(&srq->mtt->ref_count);
> + refcount_dec(&srq->mtt->ref_count);
> if (srq->cq)
> - atomic_dec(&srq->cq->ref_count);
> + refcount_dec(&srq->cq->ref_count);
> state = RES_SRQ_ALLOCATED;
> break;
>
> @@ -4805,7 +4805,7 @@ static void rem_slave_cqs(struct mlx4_dev *dev, int slave)
> spin_lock_irq(mlx4_tlock(dev));
> list_for_each_entry_safe(cq, tmp, cq_list, com.list) {
> spin_unlock_irq(mlx4_tlock(dev));
> - if (cq->com.owner == slave && !atomic_read(&cq->ref_count)) {
> + if (cq->com.owner == slave && refcount_read(&cq->ref_count) == 1) {
> cqn = cq->com.res_id;
> state = cq->com.from_state;
> while (state != 0) {
> @@ -4832,7 +4832,7 @@ static void rem_slave_cqs(struct mlx4_dev *dev, int slave)
> if (err)
> mlx4_dbg(dev, "rem_slave_cqs: failed to move slave %d cq %d to SW ownership\n",
> slave, cqn);
> - atomic_dec(&cq->mtt->ref_count);
> + refcount_dec(&cq->mtt->ref_count);
> state = RES_CQ_ALLOCATED;
> break;
>
> @@ -4900,7 +4900,7 @@ static void rem_slave_mrs(struct mlx4_dev *dev, int slave)
> mlx4_dbg(dev, "rem_slave_mrs: failed to move slave %d mpt %d to SW ownership\n",
> slave, mptn);
> if (mpt->mtt)
> - atomic_dec(&mpt->mtt->ref_count);
> + refcount_dec(&mpt->mtt->ref_count);
> state = RES_MPT_MAPPED;
> break;
> default:
> @@ -5144,7 +5144,7 @@ static void rem_slave_eqs(struct mlx4_dev *dev, int slave)
> if (err)
> mlx4_dbg(dev, "rem_slave_eqs: failed to move slave %d eqs %d to SW ownership\n",
> slave, eqn & 0x3ff);
> - atomic_dec(&eq->mtt->ref_count);
> + refcount_dec(&eq->mtt->ref_count);
> state = RES_EQ_RESERVED;
> break;
>
> --
> 2.20.1
>
^ permalink raw reply
* Re: [PATCH] niu: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 16:16 UTC (permalink / raw)
Cc: David S . Miller, Netdev, LKML
In-Reply-To: <20190802125720.22363-1-hslester96@gmail.com>
Chuhong Yuan <hslester96@gmail.com> 于2019年8月2日周五 下午8:57写道：
>
> refcount_t is better for reference counters since its
> implementation can prevent overflows.
> So convert atomic_t ref counters to refcount_t.
>
> Also convert refcount from 0-based to 1-based.
It seems that directly converting refcount from 0-based to
1-based is infeasible.
I am sorry for this mistake.
Regards,
Chuhong
>
> Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> ---
> drivers/net/ethernet/sun/niu.c | 6 +++---
> drivers/net/ethernet/sun/niu.h | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c
> index 0bc5863bffeb..5bf096e51db7 100644
> --- a/drivers/net/ethernet/sun/niu.c
> +++ b/drivers/net/ethernet/sun/niu.c
> @@ -9464,7 +9464,7 @@ static struct niu_parent *niu_new_parent(struct niu *np,
> memcpy(&p->id, id, sizeof(*id));
> p->plat_type = ptype;
> INIT_LIST_HEAD(&p->list);
> - atomic_set(&p->refcnt, 0);
> + refcount_set(&p->refcnt, 1);
> list_add(&p->list, &niu_parent_list);
> spin_lock_init(&p->lock);
>
> @@ -9524,7 +9524,7 @@ static struct niu_parent *niu_get_parent(struct niu *np,
> port_name);
> if (!err) {
> p->ports[port] = np;
> - atomic_inc(&p->refcnt);
> + refcount_inc(&p->refcnt);
> }
> }
> mutex_unlock(&niu_parent_lock);
> @@ -9552,7 +9552,7 @@ static void niu_put_parent(struct niu *np)
> p->ports[port] = NULL;
> np->parent = NULL;
>
> - if (atomic_dec_and_test(&p->refcnt)) {
> + if (refcount_dec_and_test(&p->refcnt)) {
> list_del(&p->list);
> platform_device_unregister(p->plat_dev);
> }
> diff --git a/drivers/net/ethernet/sun/niu.h b/drivers/net/ethernet/sun/niu.h
> index 04c215f91fc0..755e6dd4c903 100644
> --- a/drivers/net/ethernet/sun/niu.h
> +++ b/drivers/net/ethernet/sun/niu.h
> @@ -3071,7 +3071,7 @@ struct niu_parent {
>
> struct niu *ports[NIU_MAX_PORTS];
>
> - atomic_t refcnt;
> + refcount_t refcnt;
> struct list_head list;
>
> spinlock_t lock;
> --
> 2.20.1
>
^ permalink raw reply
* Re: [patch 1/1] drivers/net/ethernet/marvell/mvmdio.c: Fix non OF case
From: Arnaud Patard @ 2019-08-02 16:21 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20190802144353.GG2099@lunn.ch>
Andrew Lunn <andrew@lunn.ch> writes:
Hi,
> On Fri, Aug 02, 2019 at 10:32:40AM +0200, Arnaud Patard wrote:
>> Orion5.x systems are still using machine files and not device-tree.
>> Commit 96cb4342382290c9 ("net: mvmdio: allow up to three clocks to be
>> specified for orion-mdio") has replaced devm_clk_get() with of_clk_get(),
>> leading to a oops at boot and not working network, as reported in
>> https://lists.debian.org/debian-arm/2019/07/msg00088.html and possibly in
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908712.
>>
>> Link: https://lists.debian.org/debian-arm/2019/07/msg00088.html
>> Fixes: 96cb4342382290c9 ("net: mvmdio: allow up to three clocks to be specified for orion-mdio")
>> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
>> Index: linux-next/drivers/net/ethernet/marvell/mvmdio.c
>> ===================================================================
>> --- linux-next.orig/drivers/net/ethernet/marvell/mvmdio.c
>> +++ linux-next/drivers/net/ethernet/marvell/mvmdio.c
>> @@ -319,20 +319,33 @@ static int orion_mdio_probe(struct platf
>>
>> init_waitqueue_head(&dev->smi_busy_wait);
>>
>> - for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
>> - dev->clk[i] = of_clk_get(pdev->dev.of_node, i);
>> - if (PTR_ERR(dev->clk[i]) == -EPROBE_DEFER) {
>> + if (pdev->dev.of_node) {
>> + for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
>> + dev->clk[i] = of_clk_get(pdev->dev.of_node, i);
>> + if (PTR_ERR(dev->clk[i]) == -EPROBE_DEFER) {
>> + ret = -EPROBE_DEFER;
>> + goto out_clk;
>> + }
>> + if (IS_ERR(dev->clk[i]))
>> + break;
>> + clk_prepare_enable(dev->clk[i]);
>> + }
>> +
>> + if (!IS_ERR(of_clk_get(pdev->dev.of_node,
>> + ARRAY_SIZE(dev->clk))))
>> + dev_warn(&pdev->dev,
>> + "unsupported number of clocks, limiting to the first "
>> + __stringify(ARRAY_SIZE(dev->clk)) "\n");
>> + } else {
>> + dev->clk[0] = clk_get(&pdev->dev, NULL);
>> + if (PTR_ERR(dev->clk[0]) == -EPROBE_DEFER) {
>> ret = -EPROBE_DEFER;
>> goto out_clk;
>> }
>
> Hi Arnaud
>
> It is a long time since i looked at Orion5x. Is this else clause even
> needed? If my memory is right, i don't think it needs to enable tclk?
> It was kirkwood which first added gateable clocks. And all kirkwood
> boards are not DT.
I was not sure if the else clause was needed or not so I only restored
similar behaviour to what was there before the commit 96cb4342382290c9.
By looking at the commit logs, the commit adding the clock support
3d604da1e9547c09 (net: mvmdio: get and enable optional clock) doesn't
mention the SoC names. So, I've no idea if it's needed or not.
Arnaud
^ permalink raw reply
* Re: linux-next: Tree for Aug 2 (staging/octeon/)
From: Randy Dunlap @ 2019-08-02 16:22 UTC (permalink / raw)
To: Stephen Rothwell, Linux Next Mailing List
Cc: Linux Kernel Mailing List, devel@driverdev.osuosl.org,
Matthew Wilcox, netdev@vger.kernel.org, Nathan Chancellor
In-Reply-To: <20190802155223.41b0be6e@canb.auug.org.au>
On 8/1/19 10:52 PM, Stephen Rothwell wrote:
> Hi all,
>
> Changes since 20190801:
>
on x86_64:
when CONFIG_OF is not set/enabled.
WARNING: unmet direct dependencies detected for MDIO_OCTEON
Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=y] && MDIO_BUS [=y] && 64BIT [=y] && HAS_IOMEM [=y] && OF_MDIO [=n]
Selected by [y]:
- OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC || COMPILE_TEST [=y]) && NETDEVICES [=y]
ld: drivers/net/phy/mdio-octeon.o: in function `octeon_mdiobus_probe':
mdio-octeon.c:(.text+0x31c): undefined reference to `cavium_mdiobus_read'
ld: mdio-octeon.c:(.text+0x35a): undefined reference to `cavium_mdiobus_write'
OCTEON_ETHERNET should not select MDIO_OCTEON when OF_MDIO is not set/enabled.
--
~Randy
^ permalink raw reply
* [RFC] net: dsa: mv88e6xxx: ptp: improve phc2sys precision for mv88e6xxx switch in combination with imx6-fec
From: Hubert Feurstein @ 2019-08-02 16:32 UTC (permalink / raw)
To: Richard Cochran, Andrew Lunn, netdev, linux-kernel
Cc: Hubert Feurstein, Vivien Didelot, Florian Fainelli,
David S. Miller, Vladimir Oltean
With this patch the phc2sys synchronisation precision improved to +/-500ns.
Signed-off-by: Hubert Feurstein <h.feurstein@gmail.com>
---
This patch should only be the base for a discussion about improving precision of
phc2sys (from the linuxptp package) in combination with a mv88e6xxx switch and
imx6-fec.
When I started my work on PTP at the beginning of this week, I was positively
supprised about the sync precision of ptp4l. After adding support for the
MV88E6220 I was able to synchronize two of our boards within +/- 10 nanoseconds.
Remebering that the PTP system in the MV88E6220 is clocked with 100MHz, this is
I think the best what can be expected. Big thanks to Richard and the other
developers who made this possible.
But then I tried to synchornize the PTP clock with the system clock by using
phc2sys (phc2sys -rr -amq -l6) and I quickly was very disapointed about the
precision.
Min: -17829 ns
Max: 21694 ns
StdDev: 8520 ns
Count: 127
So I tried to find the reason for this. And as you probably already know, the
reason for that is the MDIO latency, which is terrible (up to 800 usecs).
As a next step, I tried to to implement the gettimex64 callback (see: "[PATCH]
net: dsa: mv88e6xxx: extend PTP gettime function to read system clock"). With
this in place (and a patched linuxptp-master version which really uses the
PTP_SYS_OFFSET_EXTENDED-ioctl), I got the following results:
Min: -12144 ns
Max: 10891 ns
StdDev: 4046,71 ns
Count: 112
So, things improved, but this is still unacceptable. It was still not possible
to compensate the MDIO latency issue.
According to my understanding, the timestamps (by using
ptp_read_system_{pre|post}ts) have to be captured at a place where we have an
constant offset related to the PHC in the switch. The only point where these
timestamps can be captured is the mdio_write callback in the imx_fec. Because,
reading the PHC timestamp will result in the follwing MDIO accesses:
(several) reads of the AVB_CMD register (to poll for the busy-flag)
write AVB_CMD (AVBOp=110b Read with post-incerement of PHC timestamp)
read AVB_DATA (PTP Global Time [15:0])
read AVB_DATA (PTP Global Time [31:16])
With this sequence in mind, the Marvell switch has to snapshot the PHC
timestamp at the write-AVB_CMD in order to be able to get sane values later by
reading AVB_DATA. So the best place to capture the system timestamps is this
one and only write operation directly in the imx_fec. By using the patch below
(without the changes to the system clock resolution) I got the following
results:
Min: -464 ns
Max: 525 ns
StdDev: 210,31 ns
Count: 401
I would say that is a huge improvement.
I realized, that the system clock (at least on the imx6) has a resolution of
333ns. So I tried to speed up this clock by using the PER-clock instead of
OSC_PER. This gave me 15ns resolution. The results were:
Min: -476 ns
Max: 439 ns
StdDev: 176,52 ns
Count: 630
So, things got improved again a little bit (at least the StdDev).
According to my understanding, this is almost the best which is possible,
because there is one more clock which influences the results. This is the MDIO
bus clock, which is just 2.5MHz (or 400ns). So, I would say that +/- 400ns
jitter is caused only by the MDIO bus clock, since the changes in imx_fec should
not introduce any unpredictable latencies.
My question to the experienced kernel developers is, how can this be implemented
in a more generic way? Because this hack only works under these circumstances,
and of course can never be part of the mainline kernel.
I am not 100% sure that all my statements or assumptions are correct, so feel
free to correct me.
Hubert
drivers/clocksource/timer-imx-gpt.c | 9 ++++++++-
drivers/net/dsa/mv88e6xxx/chip.h | 2 ++
drivers/net/dsa/mv88e6xxx/ptp.c | 11 +++++++----
drivers/net/dsa/mv88e6xxx/smi.c | 2 +-
drivers/net/ethernet/freescale/fec_main.c | 24 ++++++++++++++++++-----
drivers/net/phy/mdio_bus.c | 16 +++++++++++++++
include/linux/mdio.h | 2 ++
include/linux/phy.h | 1 +
8 files changed, 56 insertions(+), 11 deletions(-)
diff --git a/drivers/clocksource/timer-imx-gpt.c b/drivers/clocksource/timer-imx-gpt.c
index 706c0d0ff56c..84695a2d8ff7 100644
--- a/drivers/clocksource/timer-imx-gpt.c
+++ b/drivers/clocksource/timer-imx-gpt.c
@@ -471,8 +471,15 @@ static int __init mxc_timer_init_dt(struct device_node *np, enum imx_gpt_type t
/* Try osc_per first, and fall back to per otherwise */
imxtm->clk_per = of_clk_get_by_name(np, "osc_per");
- if (IS_ERR(imxtm->clk_per))
+
+ /* Force PER clock to be used, so we get 15ns instead of 333ns */
+ //if (IS_ERR(imxtm->clk_per)) {
+ if (1) {
imxtm->clk_per = of_clk_get_by_name(np, "per");
+ pr_info("==> Using PER clock\n");
+ } else {
+ pr_info("==> Using OSC_PER clock\n");
+ }
imxtm->type = type;
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 01963ee94c50..9e14dc406415 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -277,6 +277,8 @@ struct mv88e6xxx_chip {
struct ptp_clock_info ptp_clock_info;
struct delayed_work tai_event_work;
struct ptp_pin_desc pin_config[MV88E6XXX_MAX_GPIO];
+ struct ptp_system_timestamp *ptp_sts;
+
u16 trig_config;
u16 evcap_config;
u16 enable_count;
diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c
index 073cbd0bb91b..cf6e52ee9e0a 100644
--- a/drivers/net/dsa/mv88e6xxx/ptp.c
+++ b/drivers/net/dsa/mv88e6xxx/ptp.c
@@ -235,14 +235,17 @@ static int mv88e6xxx_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
return 0;
}
-static int mv88e6xxx_ptp_gettime(struct ptp_clock_info *ptp,
- struct timespec64 *ts)
+static int mv88e6xxx_ptp_gettimex(struct ptp_clock_info *ptp,
+ struct timespec64 *ts,
+ struct ptp_system_timestamp *sts)
{
struct mv88e6xxx_chip *chip = ptp_to_chip(ptp);
u64 ns;
mv88e6xxx_reg_lock(chip);
+ chip->ptp_sts = sts;
ns = timecounter_read(&chip->tstamp_tc);
+ chip->ptp_sts = NULL;
mv88e6xxx_reg_unlock(chip);
*ts = ns_to_timespec64(ns);
@@ -426,7 +429,7 @@ static void mv88e6xxx_ptp_overflow_check(struct work_struct *work)
struct mv88e6xxx_chip *chip = dw_overflow_to_chip(dw);
struct timespec64 ts;
- mv88e6xxx_ptp_gettime(&chip->ptp_clock_info, &ts);
+ mv88e6xxx_ptp_gettimex(&chip->ptp_clock_info, &ts, NULL);
schedule_delayed_work(&chip->overflow_work,
MV88E6XXX_TAI_OVERFLOW_PERIOD);
@@ -472,7 +475,7 @@ int mv88e6xxx_ptp_setup(struct mv88e6xxx_chip *chip)
chip->ptp_clock_info.max_adj = MV88E6XXX_MAX_ADJ_PPB;
chip->ptp_clock_info.adjfine = mv88e6xxx_ptp_adjfine;
chip->ptp_clock_info.adjtime = mv88e6xxx_ptp_adjtime;
- chip->ptp_clock_info.gettime64 = mv88e6xxx_ptp_gettime;
+ chip->ptp_clock_info.gettimex64 = mv88e6xxx_ptp_gettimex;
chip->ptp_clock_info.settime64 = mv88e6xxx_ptp_settime;
chip->ptp_clock_info.enable = ptp_ops->ptp_enable;
chip->ptp_clock_info.verify = ptp_ops->ptp_verify;
diff --git a/drivers/net/dsa/mv88e6xxx/smi.c b/drivers/net/dsa/mv88e6xxx/smi.c
index 5fc78a063843..801fd4abba5a 100644
--- a/drivers/net/dsa/mv88e6xxx/smi.c
+++ b/drivers/net/dsa/mv88e6xxx/smi.c
@@ -45,7 +45,7 @@ static int mv88e6xxx_smi_direct_write(struct mv88e6xxx_chip *chip,
{
int ret;
- ret = mdiobus_write_nested(chip->bus, dev, reg, data);
+ ret = mdiobus_write_nested_ptp(chip->bus, dev, reg, data, chip->ptp_sts);
if (ret < 0)
return ret;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 2f6057e7335d..20f589dc5b8b 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1814,11 +1814,25 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
reinit_completion(&fep->mdio_done);
- /* start a write op */
- writel(FEC_MMFR_ST | FEC_MMFR_OP_WRITE |
- FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
- FEC_MMFR_TA | FEC_MMFR_DATA(value),
- fep->hwp + FEC_MII_DATA);
+ if (bus->ptp_sts) {
+ unsigned long flags = 0;
+ local_irq_save(flags);
+ __iowmb();
+ /* >Take the timestamp *after* the memory barrier */
+ ptp_read_system_prets(bus->ptp_sts);
+ writel_relaxed(FEC_MMFR_ST | FEC_MMFR_OP_WRITE |
+ FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
+ FEC_MMFR_TA | FEC_MMFR_DATA(value),
+ fep->hwp + FEC_MII_DATA);
+ ptp_read_system_postts(bus->ptp_sts);
+ local_irq_restore(flags);
+ } else {
+ /* start a write op */
+ writel(FEC_MMFR_ST | FEC_MMFR_OP_WRITE |
+ FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
+ FEC_MMFR_TA | FEC_MMFR_DATA(value),
+ fep->hwp + FEC_MII_DATA);
+ }
/* wait for end of transfer */
time_left = wait_for_completion_timeout(&fep->mdio_done,
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index bd04fe762056..f076487db11f 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -672,6 +672,22 @@ int mdiobus_write_nested(struct mii_bus *bus, int addr, u32 regnum, u16 val)
}
EXPORT_SYMBOL(mdiobus_write_nested);
+int mdiobus_write_nested_ptp(struct mii_bus *bus, int addr, u32 regnum, u16 val, struct ptp_system_timestamp *ptp_sts)
+{
+ int err;
+
+ BUG_ON(in_interrupt());
+
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+ bus->ptp_sts = ptp_sts;
+ err = __mdiobus_write(bus, addr, regnum, val);
+ bus->ptp_sts = NULL;
+ mutex_unlock(&bus->mdio_lock);
+
+ return err;
+}
+EXPORT_SYMBOL(mdiobus_write_nested_ptp);
+
/**
* mdiobus_write - Convenience function for writing a given MII mgmt register
* @bus: the mii_bus struct
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index e8242ad88c81..7f9767babdc3 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -9,6 +9,7 @@
#include <uapi/linux/mdio.h>
#include <linux/mod_devicetable.h>
+struct ptp_system_timestamp;
struct gpio_desc;
struct mii_bus;
@@ -310,6 +311,7 @@ int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
int mdiobus_read_nested(struct mii_bus *bus, int addr, u32 regnum);
int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
int mdiobus_write_nested(struct mii_bus *bus, int addr, u32 regnum, u16 val);
+int mdiobus_write_nested_ptp(struct mii_bus *bus, int addr, u32 regnum, u16 val, struct ptp_system_timestamp *ptp_sts);
int mdiobus_register_device(struct mdio_device *mdiodev);
int mdiobus_unregister_device(struct mdio_device *mdiodev);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 462b90b73f93..fd4e33654863 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -252,6 +252,7 @@ struct mii_bus {
int reset_delay_us;
/* RESET GPIO descriptor pointer */
struct gpio_desc *reset_gpiod;
+ struct ptp_system_timestamp *ptp_sts;
};
#define to_mii_bus(d) container_of(d, struct mii_bus, dev)
--
2.22.0
^ permalink raw reply related
* Re: [PATCH v4 2/4] net: phy: Add function to retrieve LED configuration from the DT
From: Andrew Lunn @ 2019-08-02 16:38 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190801190759.28201-3-mka@chromium.org>
On Thu, Aug 01, 2019 at 12:07:57PM -0700, Matthias Kaehlcke wrote:
> Add a phylib function for retrieving PHY LED configuration that
> is specified in the device tree using the generic binding. LEDs
> can be configured to be 'on' for a certain link speed or to blink
> when there is TX/RX activity.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v4:
> - patch added to the series
> ---
> drivers/net/phy/phy_device.c | 50 ++++++++++++++++++++++++++++++++++++
> include/linux/phy.h | 15 +++++++++++
> 2 files changed, 65 insertions(+)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 6b5cb87f3866..b4b48de45712 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -2188,6 +2188,56 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv)
> return phydrv->config_intr && phydrv->ack_interrupt;
> }
>
> +int of_get_phy_led_cfg(struct phy_device *phydev, int led,
> + struct phy_led_config *cfg)
> +{
> + struct device_node *np, *child;
> + const char *trigger;
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_OF_MDIO))
> + return -ENOENT;
> +
> + np = of_find_node_by_name(phydev->mdio.dev.of_node, "leds");
> + if (!np)
> + return -ENOENT;
> +
> + for_each_child_of_node(np, child) {
> + u32 val;
> +
> + if (!of_property_read_u32(child, "reg", &val)) {
> + if (val == (u32)led)
> + break;
> + }
> + }
Hi Matthias
This is leaking references to np and child. In the past we have not
cared about this too much, but we are now getting patches adding the
missing releases. So it would be good to fix this.
Andrew
^ permalink raw reply
* [PATCH v2] dpaa_eth: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 16:47 UTC (permalink / raw)
Cc: Madalin Bucur, David S . Miller, netdev, linux-kernel,
Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
Changes in v2:
- Add #include in dpaa_eth.h.
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 6 +++---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index f38c3fa7d705..2df6e745cb3f 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -485,7 +485,7 @@ static struct dpaa_bp *dpaa_bpid2pool(int bpid)
static bool dpaa_bpid2pool_use(int bpid)
{
if (dpaa_bpid2pool(bpid)) {
- atomic_inc(&dpaa_bp_array[bpid]->refs);
+ refcount_inc(&dpaa_bp_array[bpid]->refs);
return true;
}
@@ -496,7 +496,7 @@ static bool dpaa_bpid2pool_use(int bpid)
static void dpaa_bpid2pool_map(int bpid, struct dpaa_bp *dpaa_bp)
{
dpaa_bp_array[bpid] = dpaa_bp;
- atomic_set(&dpaa_bp->refs, 1);
+ refcount_set(&dpaa_bp->refs, 1);
}
static int dpaa_bp_alloc_pool(struct dpaa_bp *dpaa_bp)
@@ -584,7 +584,7 @@ static void dpaa_bp_free(struct dpaa_bp *dpaa_bp)
if (!bp)
return;
- if (!atomic_dec_and_test(&bp->refs))
+ if (!refcount_dec_and_test(&bp->refs))
return;
if (bp->free_buf_cb)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
index af320f83c742..f7e59e8db075 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
@@ -32,6 +32,7 @@
#define __DPAA_H
#include <linux/netdevice.h>
+#include <linux/refcount.h>
#include <soc/fsl/qman.h>
#include <soc/fsl/bman.h>
@@ -99,7 +100,7 @@ struct dpaa_bp {
int (*seed_cb)(struct dpaa_bp *);
/* bpool can be emptied before freeing by this cb */
void (*free_buf_cb)(const struct dpaa_bp *, struct bm_buffer *);
- atomic_t refs;
+ refcount_t refs;
};
struct dpaa_rx_errors {
--
2.20.1
^ permalink raw reply related
* [PATCH v2] mkiss: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 16:48 UTC (permalink / raw)
Cc: David S . Miller, netdev, linux-kernel, Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
Changes in v2:
- Add #include.
drivers/net/hamradio/mkiss.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/hamradio/mkiss.c b/drivers/net/hamradio/mkiss.c
index 442018ccd65e..c5bfa19ddb93 100644
--- a/drivers/net/hamradio/mkiss.c
+++ b/drivers/net/hamradio/mkiss.c
@@ -25,6 +25,7 @@
#include <linux/skbuff.h>
#include <linux/if_arp.h>
#include <linux/jiffies.h>
+#include <linux/refcount.h>
#include <net/ax25.h>
@@ -70,7 +71,7 @@ struct mkiss {
#define CRC_MODE_FLEX_TEST 3
#define CRC_MODE_SMACK_TEST 4
- atomic_t refcnt;
+ refcount_t refcnt;
struct completion dead;
};
@@ -668,7 +669,7 @@ static struct mkiss *mkiss_get(struct tty_struct *tty)
read_lock(&disc_data_lock);
ax = tty->disc_data;
if (ax)
- atomic_inc(&ax->refcnt);
+ refcount_inc(&ax->refcnt);
read_unlock(&disc_data_lock);
return ax;
@@ -676,7 +677,7 @@ static struct mkiss *mkiss_get(struct tty_struct *tty)
static void mkiss_put(struct mkiss *ax)
{
- if (atomic_dec_and_test(&ax->refcnt))
+ if (refcount_dec_and_test(&ax->refcnt))
complete(&ax->dead);
}
@@ -704,7 +705,7 @@ static int mkiss_open(struct tty_struct *tty)
ax->dev = dev;
spin_lock_init(&ax->buflock);
- atomic_set(&ax->refcnt, 1);
+ refcount_set(&ax->refcnt, 1);
init_completion(&ax->dead);
ax->tty = tty;
@@ -784,7 +785,7 @@ static void mkiss_close(struct tty_struct *tty)
* We have now ensured that nobody can start using ap from now on, but
* we have to wait for all existing users to finish.
*/
- if (!atomic_dec_and_test(&ax->refcnt))
+ if (!refcount_dec_and_test(&ax->refcnt))
wait_for_completion(&ax->dead);
/*
* Halt the transmit queue so that a new transmit cannot scribble
--
2.20.1
^ permalink raw reply related
* [PATCH v2] net/mlx5e: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 16:48 UTC (permalink / raw)
Cc: Saeed Mahameed, Leon Romanovsky, David S . Miller, netdev,
linux-rdma, linux-kernel, Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
Changes in v2:
- Add #include.
drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c
index b9d4f4e19ff9..148b55c3db7a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c
@@ -32,6 +32,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/refcount.h>
#include <linux/mlx5/driver.h>
#include <net/vxlan.h>
#include "mlx5_core.h"
@@ -48,7 +49,7 @@ struct mlx5_vxlan {
struct mlx5_vxlan_port {
struct hlist_node hlist;
- atomic_t refcount;
+ refcount_t refcount;
u16 udp_port;
};
@@ -113,7 +114,7 @@ int mlx5_vxlan_add_port(struct mlx5_vxlan *vxlan, u16 port)
vxlanp = mlx5_vxlan_lookup_port(vxlan, port);
if (vxlanp) {
- atomic_inc(&vxlanp->refcount);
+ refcount_inc(&vxlanp->refcount);
return 0;
}
@@ -137,7 +138,7 @@ int mlx5_vxlan_add_port(struct mlx5_vxlan *vxlan, u16 port)
}
vxlanp->udp_port = port;
- atomic_set(&vxlanp->refcount, 1);
+ refcount_set(&vxlanp->refcount, 1);
spin_lock_bh(&vxlan->lock);
hash_add(vxlan->htable, &vxlanp->hlist, port);
@@ -170,7 +171,7 @@ int mlx5_vxlan_del_port(struct mlx5_vxlan *vxlan, u16 port)
goto out_unlock;
}
- if (atomic_dec_and_test(&vxlanp->refcount)) {
+ if (refcount_dec_and_test(&vxlanp->refcount)) {
hash_del(&vxlanp->hlist);
remove = true;
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v4 1/4] dt-bindings: net: phy: Add subnode for LED configuration
From: Andrew Lunn @ 2019-08-02 16:57 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190801190759.28201-2-mka@chromium.org>
On Thu, Aug 01, 2019 at 12:07:56PM -0700, Matthias Kaehlcke wrote:
> The LED behavior of some Ethernet PHYs is configurable. Add an
> optional 'leds' subnode with a child node for each LED to be
> configured. The binding aims to be compatible with the common
> LED binding (see devicetree/bindings/leds/common.txt).
>
> A LED can be configured to be 'on' when a link with a certain speed
> is active, or to blink on RX/TX activity. For the configuration to
> be effective it needs to be supported by the hardware and the
> corresponding PHY driver.
>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v4:
> - patch added to the series
> ---
> .../devicetree/bindings/net/ethernet-phy.yaml | 47 +++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index f70f18ff821f..81c5aacc89a5 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -153,6 +153,38 @@ properties:
> Delay after the reset was deasserted in microseconds. If
> this property is missing the delay will be skipped.
>
> +patternProperties:
> + "^leds$":
> + type: object
> + description:
> + Subnode with configuration of the PHY LEDs.
> +
> + patternProperties:
> + "^led@[0-9]+$":
> + type: object
> + description:
> + Subnode with the configuration of a single PHY LED.
> +
> + properties:
> + reg:
> + description:
> + The ID number of the LED, typically corresponds to a hardware ID.
> + $ref: "/schemas/types.yaml#/definitions/uint32"
> +
> + linux,default-trigger:
> + description:
> + This parameter, if present, is a string specifying the trigger
> + assigned to the LED. Supported triggers are:
> + "phy_link_10m_active" - LED will be on when a 10Mb/s link is active
> + "phy_link_100m_active" - LED will be on when a 100Mb/s link is active
> + "phy_link_1g_active" - LED will be on when a 1Gb/s link is active
> + "phy_link_10g_active" - LED will be on when a 10Gb/s link is active
> + "phy_activity" - LED will blink when data is received or transmitted
Matthias
We should think a bit more about these names.
I can see in future needing 1G link, but it blinks off when there is
active traffic? So phy_link_1g_active could be confusing, and very similar to
phy_link_1g_activity? So maybe
> + "phy_link_10m" - LED will be solid on when a 10Mb/s link is active
> + "phy_link_100m" - LED will be solid on when a 100Mb/s link is active
> + "phy_link_1g" - LED will be solid on when a 1Gb/s link is active
etc.
And then in the future we can have
"phy_link_1g_activity' - LED will be on when 1Gbp/s
link is active and blink off
with activity.
What other use cases do we have? I don't want to support everything,
but we should be able to represent the most common modes without the
names getting too confusing.
Andrew
^ permalink raw reply
* Re: [net-next 2/9] i40e: make visible changed vf mac on host
From: Shannon Nelson @ 2019-08-02 17:00 UTC (permalink / raw)
To: Loktionov, Aleksandr, Kirsher, Jeffrey T, davem@davemloft.net
Cc: netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com,
Bowers, AndrewX
In-Reply-To: <0EF347314CF65544BA015993979A29CD74513DCB@irsmsx105.ger.corp.intel.com>
On 8/2/19 1:14 AM, Loktionov, Aleksandr wrote:
> Good day Nelson
Please don't top post. The custom on this mailing list is to answer
inline in order to be sure we're answering in context. As it is, I
believe you missed answering one of my questions.
> In 99% cases VF has _only one_ unicast mac anyway, and the last MAC has been chosen because of VF mac address change algo - it marks unicast filter for deletion and appends a new unicast filter to the list.
> The implementation has been chosen because of simplicity /* Just 3 more lines to solve the issue */, from one point it may look wasteful for some 1% of VF corner cases.
> But from another point of view, more complicated code will affect 99% normal cases. Modern cpu are sensitive to cache thrash by code size and pipeline stalls by conditionals
Yes, absolutely. So it follows that (a) we don't want to leave things
in a loop if not necessary to repeat them, (b) we'd like to keep loops
small as possible, (c) we want to keep our spin_lock sections small, and
(d) we don't want to do things that later don't matter if an error
happens when writing to the firmware. So it seems to me that you should
move that copy line from the loop and outside of the spin_lock, and put
it after the call sync the filters. Perhaps track the good mac index
with "good_mac = i" at the end of the loop code and use that later to
know which mac to copy into the vf struct.
I also noticed that you're checking the mac addresses for validity, but
only before copying it to the local vf struct. If you need to check the
addresses, shouldn't you check them before you add them to the vf's
filter list so you don't try to sync bad addresses to the FW?
If the sync to the FW fails, you send the error status to the VF but you
still have this new address copied into the vf struct. I think the copy
line should be after the FW sync, and only if the sync succeeds.
sln
>
> Alex
>
> -----Original Message-----
> From: Shannon Nelson [mailto:snelson@pensando.io]
> Sent: Friday, August 2, 2019 2:11 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; davem@davemloft.net
> Cc: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com; Bowers, AndrewX <andrewx.bowers@intel.com>
> Subject: Re: [net-next 2/9] i40e: make visible changed vf mac on host
>
> On 8/1/19 1:51 PM, Jeff Kirsher wrote:
>> From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>>
>> This patch makes changed VM mac address visible on host via ip link
>> show command. This problem is fixed by copying last unicast mac filter
>> to vf->default_lan_addr.addr. Without this patch if VF MAC was not set
>> from host side and if you run ip link show $pf, on host side you'd
>> always see a zero MAC, not the real VF MAC that VF assigned to itself.
>>
>> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>> drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>> b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>> index 02b09a8ad54c..21f7ac514d1f 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>> @@ -2629,6 +2629,9 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>> } else {
>> vf->num_mac++;
>> }
>> + if (is_valid_ether_addr(al->list[i].addr))
>> + ether_addr_copy(vf->default_lan_addr.addr,
>> + al->list[i].addr);
>> }
>> }
>> spin_unlock_bh(&vsi->mac_filter_hash_lock);
> Since this copy is done inside the for-loop, it looks like you are copying every address in the list, not just the last one. This seems wasteful and unnecessary.
>
> Since it is possible, altho' unlikely, that the filter sync that happens a little later could fail, might it be better to do the copy after you know that the sync has succeeded?
>
> Why is the last mac chosen for display rather than the first? Is there anything special about the last mac as opposed to the first mac?
>
> sln
>
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH][net-next] ice: fix potential infinite loop
From: Allan, Bruce W @ 2019-08-02 17:07 UTC (permalink / raw)
To: Colin King, Kirsher, Jeffrey T, David S . Miller,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org
Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190802155217.16996-1-colin.king@canonical.com>
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of Colin King
> Sent: Friday, August 02, 2019 8:52 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; David S . Miller
> <davem@davemloft.net>; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org
> Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH][net-next] ice: fix potential infinite loop
>
> From: Colin Ian King <colin.king@canonical.com>
>
> The loop counter of a for-loop is a u8 however this is being compared
> to an int upper bound and this can lead to an infinite loop if the
> upper bound is greater than 255 since the loop counter will wrap back
> to zero. Fix this potential issue by making the loop counter an int.
>
> Addresses-Coverity: ("Infinite loop")
Actually, num_alloc_vfs should probably be a u16 instead of an int since num_alloc_vfs cannot exceed 256.
Which Coverity scan reported this and what options are used in the analysis?
> Fixes: c7aeb4d1b9bf ("ice: Disable VFs until reset is completed")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index c26e6a102dac..088543d50095 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -488,7 +488,7 @@ static void
> ice_prepare_for_reset(struct ice_pf *pf)
> {
> struct ice_hw *hw = &pf->hw;
> - u8 i;
> + int i;
>
> /* already prepared for reset */
> if (test_bit(__ICE_PREPARED_FOR_RESET, pf->state))
> --
> 2.20.1
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply
* Re: [net v1 PATCH 4/4] net: fix bpf_xdp_adjust_head regression for generic-XDP
From: Jakub Kicinski @ 2019-08-02 17:08 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, David S. Miller, xdp-newbies, Daniel Borkmann,
brandon.cazander, Alexei Starovoitov, Willem de Bruijn
In-Reply-To: <20190802095350.7242399b@carbon>
On Fri, 2 Aug 2019 09:53:54 +0200, Jesper Dangaard Brouer wrote:
> On Thu, 1 Aug 2019 17:44:06 -0700
> Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
>
> > On Thu, 01 Aug 2019 20:00:31 +0200, Jesper Dangaard Brouer wrote:
> > > When generic-XDP was moved to a later processing step by commit
> > > 458bf2f224f0 ("net: core: support XDP generic on stacked devices.")
> > > a regression was introduced when using bpf_xdp_adjust_head.
> > >
> > > The issue is that after this commit the skb->network_header is now
> > > changed prior to calling generic XDP and not after. Thus, if the header
> > > is changed by XDP (via bpf_xdp_adjust_head), then skb->network_header
> > > also need to be updated again. Fix by calling skb_reset_network_header().
> > >
> > > Fixes: 458bf2f224f0 ("net: core: support XDP generic on stacked devices.")
> > > Reported-by: Brandon Cazander <brandon.cazander@multapplied.net>
> > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> >
> > Out of curiosity what was your conclusion regarding resetting the
> > transport header as well?
>
> Well, I don't know... I need some review, from e.g. Stephen that
> changed this... I've added code snippets below signature to helper
> reviewers (also helps understand below paragraph).
>
> I think, we perhaps should call skb_reset_transport_header(), as we
> change skb->data (via either __skb_pull() or __skb_push()), *BUT* I'm
> not sure it is needed/required, as someone/something afterwards still
> need to call skb_set_transport_header(), which also calls
> skb_reset_transport_header() anyway.
Perhaps you've seen this, but just in case - this is the last commit
that touched the transport header setting in __netif_receive_skb(),
and it sounds like it matters mostly for qdisc accounting?
commit fda55eca5a33f33ffcd4192c6b2d75179714a52c
Author: Eric Dumazet <edumazet@google.com>
Date: Mon Jan 7 09:28:21 2013 +0000
net: introduce skb_transport_header_was_set()
We have skb_mac_header_was_set() helper to tell if mac_header
was set on a skb. We would like the same for transport_header.
__netif_receive_skb() doesn't reset the transport header if already
set by GRO layer.
Note that network stacks usually reset the transport header anyway,
after pulling the network header, so this change only allows
a followup patch to have more precise qdisc pkt_len computation
for GSO packets at ingress side.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH][net-next] ice: fix potential infinite loop
From: Colin Ian King @ 2019-08-02 17:12 UTC (permalink / raw)
To: Allan, Bruce W, Kirsher, Jeffrey T, David S . Miller,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org
Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <804857E1F29AAC47BF68C404FC60A18401096DB0DF@ORSMSX122.amr.corp.intel.com>
On 02/08/2019 18:07, Allan, Bruce W wrote:
>> -----Original Message-----
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
>> Of Colin King
>> Sent: Friday, August 02, 2019 8:52 AM
>> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; David S . Miller
>> <davem@davemloft.net>; intel-wired-lan@lists.osuosl.org;
>> netdev@vger.kernel.org
>> Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: [Intel-wired-lan] [PATCH][net-next] ice: fix potential infinite loop
>>
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The loop counter of a for-loop is a u8 however this is being compared
>> to an int upper bound and this can lead to an infinite loop if the
>> upper bound is greater than 255 since the loop counter will wrap back
>> to zero. Fix this potential issue by making the loop counter an int.
>>
>> Addresses-Coverity: ("Infinite loop")
>
> Actually, num_alloc_vfs should probably be a u16 instead of an int since num_alloc_vfs cannot exceed 256.
>
> Which Coverity scan reported this and what options are used in the analysis?
One that I run in a private coverity scan with scan analysis cranked up
high on linux-next, so the report is not public.
Colin
>
>> Fixes: c7aeb4d1b9bf ("ice: Disable VFs until reset is completed")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
>> b/drivers/net/ethernet/intel/ice/ice_main.c
>> index c26e6a102dac..088543d50095 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>> @@ -488,7 +488,7 @@ static void
>> ice_prepare_for_reset(struct ice_pf *pf)
>> {
>> struct ice_hw *hw = &pf->hw;
>> - u8 i;
>> + int i;
>>
>> /* already prepared for reset */
>> if (test_bit(__ICE_PREPARED_FOR_RESET, pf->state))
>> --
>> 2.20.1
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply
* [PATCH bpf-next 0/3] selftests/bpf: switch test_progs back to stdio
From: Stanislav Fomichev @ 2019-08-02 17:17 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
I was looking into converting test_sockops* to test_progs framework
and that requires using cgroup_helpers.c which rely on stdio/stderr.
Let's use open_memstream to override stdout into buffer during
subtests instead of custom test_{v,}printf wrappers. That lets
us continue to use stdio in the subtests and dump it on failure
if required.
That would also fix bpf_find_map which currently uses printf to
signal failure (missed during test_printf conversion).
Cc: Andrii Nakryiko <andriin@fb.com>
Stanislav Fomichev (3):
selftests/bpf: test_progs: switch to open_memstream
selftests/bpf: test_progs: test__printf -> printf
selftests/bpf: test_progs: drop extra trailing tab
.../bpf/prog_tests/bpf_verif_scale.c | 4 +-
.../selftests/bpf/prog_tests/l4lb_all.c | 2 +-
.../selftests/bpf/prog_tests/map_lock.c | 10 +-
.../selftests/bpf/prog_tests/send_signal.c | 4 +-
.../selftests/bpf/prog_tests/spinlock.c | 2 +-
.../bpf/prog_tests/stacktrace_build_id.c | 4 +-
.../bpf/prog_tests/stacktrace_build_id_nmi.c | 4 +-
.../selftests/bpf/prog_tests/xdp_noinline.c | 4 +-
tools/testing/selftests/bpf/test_progs.c | 116 +++++++-----------
tools/testing/selftests/bpf/test_progs.h | 12 +-
10 files changed, 68 insertions(+), 94 deletions(-)
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply
* [PATCH bpf-next 1/3] selftests/bpf: test_progs: switch to open_memstream
From: Stanislav Fomichev @ 2019-08-02 17:17 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
In-Reply-To: <20190802171710.11456-1-sdf@google.com>
Use open_memstream to override stdout during test execution.
The copy of the original stdout is held in env.stdout and used
to print subtest info and dump failed log.
test_{v,}printf are now simple wrappers around stdout and will be
removed in the next patch.
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/test_progs.c | 100 ++++++++++-------------
tools/testing/selftests/bpf/test_progs.h | 2 +-
2 files changed, 46 insertions(+), 56 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index db00196c8315..00d1565d01a3 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -40,14 +40,22 @@ static bool should_run(struct test_selector *sel, int num, const char *name)
static void dump_test_log(const struct prog_test_def *test, bool failed)
{
- if (env.verbose || test->force_log || failed) {
- if (env.log_cnt) {
- fprintf(stdout, "%s", env.log_buf);
- if (env.log_buf[env.log_cnt - 1] != '\n')
- fprintf(stdout, "\n");
+ if (stdout == env.stdout)
+ return;
+
+ fflush(stdout); /* exports env.log_buf & env.log_cap */
+
+ if (env.log_cap && (env.verbose || test->force_log || failed)) {
+ int len = strlen(env.log_buf);
+
+ if (len) {
+ fprintf(env.stdout, "%s", env.log_buf);
+ if (env.log_buf[len - 1] != '\n')
+ fprintf(env.stdout, "\n");
+
+ fseeko(stdout, 0, SEEK_SET); /* rewind */
}
}
- env.log_cnt = 0;
}
void test__end_subtest()
@@ -62,7 +70,7 @@ void test__end_subtest()
dump_test_log(test, sub_error_cnt);
- printf("#%d/%d %s:%s\n",
+ fprintf(env.stdout, "#%d/%d %s:%s\n",
test->test_num, test->subtest_num,
test->subtest_name, sub_error_cnt ? "FAIL" : "OK");
}
@@ -100,53 +108,7 @@ void test__force_log() {
void test__vprintf(const char *fmt, va_list args)
{
- size_t rem_sz;
- int ret = 0;
-
- if (env.verbose || (env.test && env.test->force_log)) {
- vfprintf(stderr, fmt, args);
- return;
- }
-
-try_again:
- rem_sz = env.log_cap - env.log_cnt;
- if (rem_sz) {
- va_list ap;
-
- va_copy(ap, args);
- /* we reserved extra byte for \0 at the end */
- ret = vsnprintf(env.log_buf + env.log_cnt, rem_sz + 1, fmt, ap);
- va_end(ap);
-
- if (ret < 0) {
- env.log_buf[env.log_cnt] = '\0';
- fprintf(stderr, "failed to log w/ fmt '%s'\n", fmt);
- return;
- }
- }
-
- if (!rem_sz || ret > rem_sz) {
- size_t new_sz = env.log_cap * 3 / 2;
- char *new_buf;
-
- if (new_sz < 4096)
- new_sz = 4096;
- if (new_sz < ret + env.log_cnt)
- new_sz = ret + env.log_cnt;
-
- /* +1 for guaranteed space for terminating \0 */
- new_buf = realloc(env.log_buf, new_sz + 1);
- if (!new_buf) {
- fprintf(stderr, "failed to realloc log buffer: %d\n",
- errno);
- return;
- }
- env.log_buf = new_buf;
- env.log_cap = new_sz;
- goto try_again;
- }
-
- env.log_cnt += ret;
+ vprintf(fmt, args);
}
void test__printf(const char *fmt, ...)
@@ -477,6 +439,32 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
return 0;
}
+static void stdout_hijack(void)
+{
+ if (env.verbose || (env.test && env.test->force_log)) {
+ /* nothing to do, output to stdout by default */
+ return;
+ }
+
+ /* stdout -> buffer */
+ fflush(stdout);
+ stdout = open_memstream(&env.log_buf, &env.log_cap);
+}
+
+static void stdout_restore(void)
+{
+ if (stdout == env.stdout)
+ return;
+
+ fclose(stdout);
+ free(env.log_buf);
+
+ env.log_buf = NULL;
+ env.log_cap = 0;
+
+ stdout = env.stdout;
+}
+
int main(int argc, char **argv)
{
static const struct argp argp = {
@@ -495,6 +483,7 @@ int main(int argc, char **argv)
srand(time(NULL));
env.jit_enabled = is_jit_enabled();
+ env.stdout = stdout;
for (i = 0; i < prog_test_cnt; i++) {
struct prog_test_def *test = &prog_test_defs[i];
@@ -508,6 +497,7 @@ int main(int argc, char **argv)
test->test_num, test->test_name))
continue;
+ stdout_hijack();
test->run_test();
/* ensure last sub-test is finalized properly */
if (test->subtest_name)
@@ -522,6 +512,7 @@ int main(int argc, char **argv)
env.succ_cnt++;
dump_test_log(test, test->error_cnt);
+ stdout_restore();
printf("#%d %s:%s\n", test->test_num, test->test_name,
test->error_cnt ? "FAIL" : "OK");
@@ -529,7 +520,6 @@ int main(int argc, char **argv)
printf("Summary: %d/%d PASSED, %d FAILED\n",
env.succ_cnt, env.sub_succ_cnt, env.fail_cnt);
- free(env.log_buf);
free(env.test_selector.num_set);
free(env.subtest_selector.num_set);
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index afd14962456f..9fd89078494f 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -56,8 +56,8 @@ struct test_env {
bool jit_enabled;
struct prog_test_def *test;
+ FILE *stdout;
char *log_buf;
- size_t log_cnt;
size_t log_cap;
int succ_cnt; /* successful tests */
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH bpf-next 2/3] selftests/bpf: test_progs: test__printf -> printf
From: Stanislav Fomichev @ 2019-08-02 17:17 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
In-Reply-To: <20190802171710.11456-1-sdf@google.com>
Now that test__printf is a simple wraper around printf, let's drop it
(and test__vprintf as well).
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
.../selftests/bpf/prog_tests/bpf_verif_scale.c | 4 ++--
.../testing/selftests/bpf/prog_tests/l4lb_all.c | 2 +-
.../testing/selftests/bpf/prog_tests/map_lock.c | 10 +++++-----
.../selftests/bpf/prog_tests/send_signal.c | 4 ++--
.../testing/selftests/bpf/prog_tests/spinlock.c | 2 +-
.../bpf/prog_tests/stacktrace_build_id.c | 4 ++--
.../bpf/prog_tests/stacktrace_build_id_nmi.c | 4 ++--
.../selftests/bpf/prog_tests/xdp_noinline.c | 4 ++--
tools/testing/selftests/bpf/test_progs.c | 16 +---------------
tools/testing/selftests/bpf/test_progs.h | 10 ++++------
10 files changed, 22 insertions(+), 38 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
index b4be96162ff4..3548ba2f24a8 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
@@ -5,13 +5,13 @@ static int libbpf_debug_print(enum libbpf_print_level level,
const char *format, va_list args)
{
if (level != LIBBPF_DEBUG) {
- test__vprintf(format, args);
+ vprintf(format, args);
return 0;
}
if (!strstr(format, "verifier log"))
return 0;
- test__vprintf("%s", args);
+ vprintf("%s", args);
return 0;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/l4lb_all.c b/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
index 5ce572c03a5f..20ddca830e68 100644
--- a/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
+++ b/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
@@ -74,7 +74,7 @@ static void test_l4lb(const char *file)
}
if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
error_cnt++;
- test__printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
+ printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
}
out:
bpf_object__close(obj);
diff --git a/tools/testing/selftests/bpf/prog_tests/map_lock.c b/tools/testing/selftests/bpf/prog_tests/map_lock.c
index 2e78217ed3fd..ee99368c595c 100644
--- a/tools/testing/selftests/bpf/prog_tests/map_lock.c
+++ b/tools/testing/selftests/bpf/prog_tests/map_lock.c
@@ -9,12 +9,12 @@ static void *parallel_map_access(void *arg)
for (i = 0; i < 10000; i++) {
err = bpf_map_lookup_elem_flags(map_fd, &key, vars, BPF_F_LOCK);
if (err) {
- test__printf("lookup failed\n");
+ printf("lookup failed\n");
error_cnt++;
goto out;
}
if (vars[0] != 0) {
- test__printf("lookup #%d var[0]=%d\n", i, vars[0]);
+ printf("lookup #%d var[0]=%d\n", i, vars[0]);
error_cnt++;
goto out;
}
@@ -22,8 +22,8 @@ static void *parallel_map_access(void *arg)
for (j = 2; j < 17; j++) {
if (vars[j] == rnd)
continue;
- test__printf("lookup #%d var[1]=%d var[%d]=%d\n",
- i, rnd, j, vars[j]);
+ printf("lookup #%d var[1]=%d var[%d]=%d\n",
+ i, rnd, j, vars[j]);
error_cnt++;
goto out;
}
@@ -43,7 +43,7 @@ void test_map_lock(void)
err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
if (err) {
- test__printf("test_map_lock:bpf_prog_load errno %d\n", errno);
+ printf("test_map_lock:bpf_prog_load errno %d\n", errno);
goto close_prog;
}
map_fd[0] = bpf_find_map(__func__, obj, "hash_map");
diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
index 461b423d0584..1575f0a1f586 100644
--- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
+++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
@@ -202,8 +202,8 @@ static int test_send_signal_nmi(void)
-1 /* cpu */, -1 /* group_fd */, 0 /* flags */);
if (pmu_fd == -1) {
if (errno == ENOENT) {
- test__printf("%s:SKIP:no PERF_COUNT_HW_CPU_CYCLES\n",
- __func__);
+ printf("%s:SKIP:no PERF_COUNT_HW_CPU_CYCLES\n",
+ __func__);
return 0;
}
/* Let the test fail with a more informative message */
diff --git a/tools/testing/selftests/bpf/prog_tests/spinlock.c b/tools/testing/selftests/bpf/prog_tests/spinlock.c
index deb2db5b85b0..114ebe6a438e 100644
--- a/tools/testing/selftests/bpf/prog_tests/spinlock.c
+++ b/tools/testing/selftests/bpf/prog_tests/spinlock.c
@@ -12,7 +12,7 @@ void test_spinlock(void)
err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
if (err) {
- test__printf("test_spin_lock:bpf_prog_load errno %d\n", errno);
+ printf("test_spin_lock:bpf_prog_load errno %d\n", errno);
goto close_prog;
}
for (i = 0; i < 4; i++)
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
index 356d2c017a9c..ac44fda84833 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
@@ -109,8 +109,8 @@ void test_stacktrace_build_id(void)
if (build_id_matches < 1 && retry--) {
bpf_link__destroy(link);
bpf_object__close(obj);
- test__printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
- __func__);
+ printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
+ __func__);
goto retry;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
index f44f2c159714..9557b7dfb782 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
@@ -140,8 +140,8 @@ void test_stacktrace_build_id_nmi(void)
if (build_id_matches < 1 && retry--) {
bpf_link__destroy(link);
bpf_object__close(obj);
- test__printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
- __func__);
+ printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
+ __func__);
goto retry;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c b/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
index b5404494b8aa..15f7c272edb0 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
@@ -75,8 +75,8 @@ void test_xdp_noinline(void)
}
if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
error_cnt++;
- test__printf("test_xdp_noinline:FAIL:stats %lld %lld\n",
- bytes, pkts);
+ printf("test_xdp_noinline:FAIL:stats %lld %lld\n",
+ bytes, pkts);
}
out:
bpf_object__close(obj);
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 00d1565d01a3..71c717162ac8 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -106,20 +106,6 @@ void test__force_log() {
env.test->force_log = true;
}
-void test__vprintf(const char *fmt, va_list args)
-{
- vprintf(fmt, args);
-}
-
-void test__printf(const char *fmt, ...)
-{
- va_list args;
-
- va_start(args, fmt);
- test__vprintf(fmt, args);
- va_end(args);
-}
-
struct ipv4_packet pkt_v4 = {
.eth.h_proto = __bpf_constant_htons(ETH_P_IP),
.iph.ihl = 5,
@@ -311,7 +297,7 @@ static int libbpf_print_fn(enum libbpf_print_level level,
{
if (!env.very_verbose && level == LIBBPF_DEBUG)
return 0;
- test__vprintf(format, args);
+ vprintf(format, args);
return 0;
}
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 9fd89078494f..cf0486dbb9b4 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -69,8 +69,6 @@ extern int error_cnt;
extern int pass_cnt;
extern struct test_env env;
-extern void test__printf(const char *fmt, ...);
-extern void test__vprintf(const char *fmt, va_list args);
extern void test__force_log();
extern bool test__start_subtest(const char *name);
@@ -96,12 +94,12 @@ extern struct ipv6_packet pkt_v6;
int __ret = !!(condition); \
if (__ret) { \
error_cnt++; \
- test__printf("%s:FAIL:%s ", __func__, tag); \
- test__printf(format); \
+ printf("%s:FAIL:%s ", __func__, tag); \
+ printf(format); \
} else { \
pass_cnt++; \
- test__printf("%s:PASS:%s %d nsec\n", \
- __func__, tag, duration); \
+ printf("%s:PASS:%s %d nsec\n", \
+ __func__, tag, duration); \
} \
__ret; \
})
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH bpf-next 3/3] selftests/bpf: test_progs: drop extra trailing tab
From: Stanislav Fomichev @ 2019-08-02 17:17 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
In-Reply-To: <20190802171710.11456-1-sdf@google.com>
Small (un)related cleanup.
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/test_progs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 71c717162ac8..477539d0adeb 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -279,7 +279,7 @@ enum ARG_KEYS {
ARG_VERIFIER_STATS = 's',
ARG_VERBOSE = 'v',
};
-
+
static const struct argp_option opts[] = {
{ "num", ARG_TEST_NUM, "NUM", 0,
"Run test number NUM only " },
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH v2 0/3] Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 17:23 UTC (permalink / raw)
Cc: Saeed Mahameed, Leon Romanovsky, David S . Miller, Doug Ledford,
Jason Gunthorpe, netdev, linux-rdma, linux-kernel, Chuhong Yuan
Reference counters are preferred to use refcount_t instead of
atomic_t.
This is because the implementation of refcount_t can prevent
overflows and detect possible use-after-free.
First convert the refcount field to refcount_t in mlx5/driver.h.
Then convert the uses to refcount_() APIs.
Changelog:
v1 -> v2:
- Add #include in include/linux/mlx5/driver.h.
Chuhong Yuan (3):
mlx5: Use refcount_t for refcount
net/mlx5: Use refcount_() APIs
IB/mlx5: Use refcount_() APIs
drivers/infiniband/hw/mlx5/srq_cmd.c | 6 +++---
drivers/net/ethernet/mellanox/mlx5/core/qp.c | 6 +++---
include/linux/mlx5/driver.h | 3 ++-
3 files changed, 8 insertions(+), 7 deletions(-)
--
2.20.1
^ 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