* RE: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
From: Rodney Cummings @ 2017-10-16 16:52 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vivien Didelot, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,
David S. Miller, Florian Fainelli, David Laight
In-Reply-To: <20171016164737.GD13339@lunn.ch>
PAUSE is only one higher layer entity. The source MAC address may not matter there, but it definitely matters for other higher layer entities (like IEEE Std 1588).
The Marvell switch can run more than just PAUSE as a higher layer protocol.
How do you guarantee that this MAC address is used for PAUSE, and only for PAUSE?
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Monday, October 16, 2017 11:48 AM
> To: Rodney Cummings <rodney.cummings@ni.com>
> Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> kernel@savoirfairelinux.com; David S. Miller <davem@davemloft.net>;
> Florian Fainelli <f.fainelli@gmail.com>; David Laight
> <David.Laight@ACULAB.COM>
> Subject: Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
>
> On Mon, Oct 16, 2017 at 04:28:04PM +0000, Rodney Cummings wrote:
> > Hi Andrew,
> >
> > I may have misunderstood.
> >
> > If this MAC address is the destination
>
> Nope. This is the source address, for Pause frames.
>
> > My concern is that for a source MAC address, a local random MAC
> > address is not safe in all networks, because it has the potential
> > for duplication. That topic has been discussed quite a bit in IEEE
> > 802.
>
> Duplications don't matter, for pause frames. The source address
> appears to be unused. And these frames don't get passed the direct
> peers MAC layer.
>
> Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
From: Andrew Lunn @ 2017-10-16 16:57 UTC (permalink / raw)
To: Rodney Cummings
Cc: Vivien Didelot, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,
David S. Miller, Florian Fainelli, David Laight
In-Reply-To: <CY1PR0401MB153605AD6262F8E25A41F913924F0@CY1PR0401MB1536.namprd04.prod.outlook.com>
> How do you guarantee that this MAC address is used for PAUSE, and only for PAUSE?
That is the only thing the data sheet says this MAC address is used
for.
Andrew
^ permalink raw reply
* Re: [patch net v2 1/4] net/sched: Change tc_action refcnt and bindcnt to atomic
From: Cong Wang @ 2017-10-16 17:06 UTC (permalink / raw)
To: Chris Mi
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Lucas Bates,
Jiri Pirko, David Miller
In-Reply-To: <1508152718-28726-2-git-send-email-chrism@mellanox.com>
On Mon, Oct 16, 2017 at 4:18 AM, Chris Mi <chrism@mellanox.com> wrote:
> If many filters share the same action. That action's refcnt and bindcnt
> could be manipulated by many RCU callbacks at the same time. This patch
> makes these operations atomic.
Actually I have been thinking about removing these RCU callbacks,
they are not necessary AFAIK, callers hold RTNL lock so they are
allowed to block. The only drawback is that adding a synchronize_rcu(),
but these are slow paths anyway...
I am not sure, it is arguable anyway, essentially it is:
synchronize_rcu() in slow path vs. multiple RCU callback races
>
> Fixes commit in pre-git era.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
This is not true, the action RCU callbacks were introduced
by:
commit c7de2cf053420d63bac85133469c965d4b1083e1
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed Jun 9 02:09:23 2010 +0000
pkt_sched: gen_kill_estimator() rcu fixes
and the filter RCU callbacks were introduced by the
patchset like this one:
commit 1ce87720d456e471de0fbd814dc5d1fe10fc1c44
Author: John Fastabend <john.fastabend@gmail.com>
Date: Fri Sep 12 20:09:16 2014 -0700
net: sched: make cls_u32 lockless
^ permalink raw reply
* Re: [PATCH net] bpf: disallow arithmetic operations on context pointer
From: Jakub Kicinski @ 2017-10-16 17:06 UTC (permalink / raw)
To: Edward Cree; +Cc: netdev, oss-drivers, alexei.starovoitov, daniel
In-Reply-To: <e2c4322a-301f-47f9-d102-725138d1bf67@solarflare.com>
On Mon, 16 Oct 2017 17:47:45 +0100, Edward Cree wrote:
> On 16/10/17 17:30, Jakub Kicinski wrote:
> > On Mon, 16 Oct 2017 17:16:24 +0100, Edward Cree wrote:
> >> On 16/10/17 16:45, Jakub Kicinski wrote:
> >>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >>> index 8b8d6ba39e23..8499759d0c7a 100644
> >>> --- a/kernel/bpf/verifier.c
> >>> +++ b/kernel/bpf/verifier.c
> >>> @@ -1116,7 +1116,12 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> >>> /* ctx accesses must be at a fixed offset, so that we can
> >>> * determine what type of data were returned.
> >>> */
> >>> - if (!tnum_is_const(reg->var_off)) {
> >>> + if (reg->off) {
> >>> + verbose("derefence of modified ctx ptr R%d off=%d+%d, ctx+const is allowed, ctx+const+const is not\n",
> >> This is slightly unclear, it's not that two adds is bad (e.g. r1 += 8;
> >> r0 = *(u32 *)r1 is bad too), it's that the offset must be in the load,
> >> not the register; your message might be accurate for some compilers but
> >> not in full generality (especially for assemblers without compiling).
> > I'm happy to hear better suggestions :) I've spent quite a bit of time
> > scratching my head thinking how to phrase this best. The first
> > part of the message is general enough IMHO, the second is targeted
> > mostly at C developers.
> Hmm, what really bugs me is that if e.g. the compiler turned
> *(ctx + 4 + 4)
> or
> ctx[4 + 4]
> or even
> ctx->arraymemb[4]
> into this kind of arithmetic on ctx, arguably that would be a bug in the
> compiler — if it's doing proper constexpr folding on its IR (or something
> along those lines) it should be able to turn them all into good LDX. The
> same even goes for if (ctx + 4) got stored in a local, because there's no
> reason that has to map to a register.
> So it's not even that "your C source breaks the rules", it's that "your C
> compiler did something silly that we don't handle".
> Maybe the message should be "compiler maybe mishandled ctx+const+const"?
Hm. We have no proof of compilers doing such things. It's probably
more likely that this will be hit if someone does:
struct xxx *X = &skb->cb;
X->field;
Or just tries to add a constant offset to ctx by hand... "Complier may
have done something silly" is probably implied in all verifier
messages :)
FWIW the pre-tnum error would be:
R%d invalid mem access 'inv'
So we are making this a lot more clear anyway.
^ permalink raw reply
* [PATCH net-next] tools: bpftool: use more common tag format
From: Jakub Kicinski @ 2017-10-16 17:12 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Jakub Kicinski
Program tag is usually displayed as string of bytes without
any separators (e.g. as "aa5520b1090cfeb6" vs MAC addr-like
format bpftool uses currently: "aa:55:20:b1:09:0c:fe:b6").
Make bptfool use the more common format both for displaying
the tag and selecting the program by tag.
This was pointed out in review but I misunderstood the comment.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 2 +-
tools/bpf/bpftool/main.h | 3 +--
tools/bpf/bpftool/prog.c | 2 +-
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 04d12f768f06..3968f0bd37db 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -55,7 +55,7 @@ EXAMPLES
**# bpftool prog show**
::
- 10: xdp name some_prog tag 00:5a:3d:21:23:62:0c:8b
+ 10: xdp name some_prog tag 005a3d2123620c8b
loaded_at Sep 29/20:11 uid 0
xlated 528B jited 370B memlock 4096B map_ids 10
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 8e809b2bb311..844e4ef6db56 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -53,8 +53,7 @@
#define NEXT_ARGP() ({ (*argc)--; (*argv)++; if (*argc < 0) usage(); })
#define BAD_ARG() ({ err("what is '%s'?\n", *argv); -1; })
-#define BPF_TAG_FMT "%02hhx:%02hhx:%02hhx:%02hhx:" \
- "%02hhx:%02hhx:%02hhx:%02hhx"
+#define BPF_TAG_FMT "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
#define HELP_SPEC_PROGRAM \
"PROG := { id PROG_ID | pinned FILE | tag PROG_TAG }"
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 9e2681c83717..d60f5307b6e2 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -224,7 +224,7 @@ static int show_prog(int fd)
printf("name %s ", info.name);
printf("tag ");
- print_hex(info.tag, BPF_TAG_SIZE, ":");
+ print_hex(info.tag, BPF_TAG_SIZE, "");
printf("\n");
if (info.load_time) {
--
2.14.1
^ permalink raw reply related
* Re: [PATCH] can: check for null sk before deferencing it via the call to sock_net
From: Oliver Hartkopp @ 2017-10-16 17:32 UTC (permalink / raw)
To: Josh Boyer
Cc: Colin King, Marc Kleine-Budde, David S . Miller, linux-can,
netdev, kernel-janitors, Linux-Kernel@Vger. Kernel. Org
In-Reply-To: <CA+5PVA7fJfZmN-XLuqARGkWL6eY54HaRAj19K_Und3K7vAVfpg@mail.gmail.com>
On 10/16/2017 06:37 PM, Josh Boyer wrote:
> On Fri, Sep 8, 2017 at 1:46 PM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>>
>>
>> On 09/08/2017 05:02 PM, Colin King wrote:
>>>
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> The assignment of net via call sock_net will dereference sk. This
>>> is performed before a sanity null check on sk, so there could be
>>> a potential null dereference on the sock_net call if sk is null.
>>> Fix this by assigning net after the sk null check. Also replace
>>> the sk == NULL with the more usual !sk idiom.
>>>
>>> Detected by CoverityScan CID#1431862 ("Dereference before null check")
>>>
>>> Fixes: 384317ef4187 ("can: network namespace support for CAN_BCM
>>> protocol")
>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>
>>
>> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
>
> I don't see this one queued up in the net or net-next trees. Did it
> fall through the cracks or did it get queued up elsewhere? Seems like
> it's a good candidate to get into 4.14?
It definitely is!
Marc is our responsible guy for CAN related upstreams - but he seems to
be busy as I already poked him here:
https://marc.info/?l=linux-can&m=150771819505097&w=2
If he doesn't send a pull request by beginning of next week, I would ask
Dave to grab these patches - to get them into 4.14.
Best regards,
Oliver
^ permalink raw reply
* Re: act_skbedit unable to check queue_mapping at init time
From: Cong Wang @ 2017-10-16 17:36 UTC (permalink / raw)
To: Florian Fainelli
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
Alexander Duyck
In-Reply-To: <c48c8af5-d4c6-d2f7-cc0f-6cb50f2a865e@gmail.com>
Hi, Florian
On Wed, Oct 11, 2017 at 12:02 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> Hi all,
>
> It is perfectly possible for the following to happen:
>
> # ls /sys/class/net/gphy/queues
> rx-0 tx-0 tx-1 tx-2 tx-3
> # tc qdisc add dev gphy root handle 1: multiq
> # tc filter add dev gphy parent 1: protocol ip prio 1 u32
> match ip dst 192.168.1.1 action skbedit queue_mapping 5
>
> that specifies a queue_mapping value that exceeds what the underlying
> device supports. There is a check for dev->real_num_tx_queue >
> d->queue_mapping in tcf_skbedit() which makes sure we won't overflow the
> queue space, but this will essentially render the action inactive.
>
> I can't find a way to get a reference on a network device from a
> tc_action_ops->init function in order to validate that the queue_mapping
> is valid at ->init() time and return an appropriate error if we are
> exceeding what the device does. This is an action chained to a match and
> a filter, so somehow one would think it would be possible to obtain a
> reference on the network device the filter was installed on...
>
> Any clues how we could fix that?
The problems is actions could be standalone and shared by multiple
filters, it is impossible to check it for this case. However, it is possible
to check it for non-standalone case, the parameter 'bind' indicates it.
You still need to find a way to get either the tp pointer or dev pointer.
Thanks.
^ permalink raw reply
* Re: [net-next RFC 3/4] openvswitch: Add meter infrastructure
From: Pravin Shelar @ 2017-10-16 17:49 UTC (permalink / raw)
To: Andy Zhou; +Cc: Linux Kernel Network Developers, Joe Stringer, Greg Rose
In-Reply-To: <CABKoBm2T6-BgecNr4RRkvtW2+Z4dXBrFo=Wn2TpsX=J+HTuXtg@mail.gmail.com>
On Mon, Oct 16, 2017 at 12:05 AM, Andy Zhou <azhou@ovn.org> wrote:
> On Fri, Oct 13, 2017 at 5:12 PM, Pravin Shelar <pshelar@ovn.org> wrote:
>> On Thu, Oct 12, 2017 at 3:38 PM, Andy Zhou <azhou@ovn.org> wrote:
>>> OVS kernel datapath so far does not support Openflow meter action.
>>> This is the first stab at adding kernel datapath meter support.
>>> This implementation supports only drop band type.
>>>
>>> Signed-off-by: Andy Zhou <azhou@ovn.org>
>>> ---
>>> net/openvswitch/Makefile | 1 +
>>> net/openvswitch/datapath.c | 14 +-
>>> net/openvswitch/datapath.h | 3 +
>>> net/openvswitch/meter.c | 611 +++++++++++++++++++++++++++++++++++++++++++++
>>> net/openvswitch/meter.h | 54 ++++
>>> 5 files changed, 681 insertions(+), 2 deletions(-)
>>> create mode 100644 net/openvswitch/meter.c
>>> create mode 100644 net/openvswitch/meter.h
>>>
>> ...
>>
>>> diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
>>> new file mode 100644
>>> index 000000000000..f24ebb5f7af4
>>> --- /dev/null
>>> +++ b/net/openvswitch/meter.c
>>
>> ....
>> ....
>>> +static int ovs_meter_cmd_features(struct sk_buff *skb, struct genl_info *info)
>>> +{
>>> + struct datapath *dp;
>>> + struct ovs_header *ovs_header = info->userhdr;
>>> + struct sk_buff *reply;
>>> + struct ovs_header *ovs_reply_header;
>>> + struct nlattr *nla, *band_nla;
>>> + int err;
>>> +
>>> + /* Check that the datapath exists */
>>> + ovs_lock();
>>> + dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
>>> + ovs_unlock();
>>> + if (!dp)
>>> + return -ENODEV;
>>> +
>> why dp check is required for this API?
> Is it possible for another core delete the dp, before ovs_lock()
> returns? Then, in theory, get_dp() can
> return NULL, no?
I do not see dp used anywhere in function. so the question was why is
dp lookup done here?
>>
>>> + reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_FEATURES,
>>> + &ovs_reply_header);
>>> + if (!reply)
>>> + return PTR_ERR(reply);
>>> +
>>> + if (nla_put_u32(reply, OVS_METER_ATTR_MAX_METERS, U32_MAX) ||
>>> + nla_put_u32(reply, OVS_METER_ATTR_MAX_BANDS, DP_MAX_BANDS))
>>> + goto nla_put_failure;
>>> +
>>> + nla = nla_nest_start(reply, OVS_METER_ATTR_BANDS);
>>> + if (!nla)
>>> + goto nla_put_failure;
>>> +
>>> + band_nla = nla_nest_start(reply, OVS_BAND_ATTR_UNSPEC);
>>> + if (!band_nla)
>>> + goto nla_put_failure;
>>> + /* Currently only DROP band type is supported. */
>>> + if (nla_put_u32(reply, OVS_BAND_ATTR_TYPE, OVS_METER_BAND_TYPE_DROP))
>>> + goto nla_put_failure;
>>> + nla_nest_end(reply, band_nla);
>>> + nla_nest_end(reply, nla);
>>> +
>>> + genlmsg_end(reply, ovs_reply_header);
>>> + return genlmsg_reply(reply, info);
>>> +
>>> +nla_put_failure:
>>> + nlmsg_free(reply);
>>> + err = -EMSGSIZE;
>>> + return err;
>>> +}
>>> +
>> ....
>>
>>> +static int ovs_meter_cmd_set(struct sk_buff *skb, struct genl_info *info)
>>> +{
>>> + struct nlattr **a = info->attrs;
>>> + struct dp_meter *meter, *old_meter;
>>> + struct sk_buff *reply;
>>> + struct ovs_header *ovs_reply_header;
>>> + struct ovs_header *ovs_header = info->userhdr;
>>> + struct datapath *dp;
>>> + int err;
>>> + u32 meter_id;
>>> + bool failed;
>>> +
>>> + meter = dp_meter_create(a);
>>> + if (IS_ERR_OR_NULL(meter))
>>> + return PTR_ERR(meter);
>>> +
>>> + reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_SET,
>>> + &ovs_reply_header);
>>> + if (IS_ERR(reply)) {
>>> + err = PTR_ERR(reply);
>>> + goto exit_free_meter;
>>> + }
>>> +
>>> + ovs_lock();
>>> + dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
>>> + if (!dp) {
>>> + err = -ENODEV;
>>> + goto exit_unlock;
>>> + }
>>> +
>>> + if (!a[OVS_METER_ATTR_ID]) {
>>> + err = -ENODEV;
>>> + goto exit_unlock;
>>> + }
>>> +
>>> + meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
>>> +
>>> + /* Cannot fail after this. */
>>> + old_meter = lookup_meter(dp, meter_id);
>>> + attach_meter(dp, meter);
>>> + ovs_unlock();
>>> +
>> After the unlock, it is not safe to keep the ref to old_meter. better
>> to release lock at the end. we could optimize it later if required.
>
> I see a problem here: the old_meter has not been removed from the list before
> unlock. The code should have been:
>
> old_meter = lookup_meter(dp, meter_id);
> detch_meter(dp, old_meter);
> attach_meter(dp, meter);
> ovs_unlock();
>
> Do you still see a problem w.r.t. unlock() here? Once detch_meter() is called,
> another thread should not have access to 'old_meter' any more. right?
looks good.
^ permalink raw reply
* Re: [PATCH] [net-next] net: systemport: add NET_DSA dependency
From: Florian Fainelli @ 2017-10-16 17:55 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S. Miller, Michael Chan, Sathya Perla, Nicolas Pitre,
netdev, linux-kernel
In-Reply-To: <20171016113258.3735473-1-arnd@arndb.de>
On 10/16/2017 04:32 AM, Arnd Bergmann wrote:
> The notifier cause a link error when NET_DSA is a loadable
> module:
>
> drivers/net/ethernet/broadcom/bcmsysport.o: In function `bcm_sysport_remove':
> bcmsysport.c:(.text+0x1582): undefined reference to `unregister_dsa_notifier'
> drivers/net/ethernet/broadcom/bcmsysport.o: In function `bcm_sysport_probe':
> bcmsysport.c:(.text+0x278d): undefined reference to `register_dsa_notifier'
>
> This adds a dependency that forces the systemport driver to be
> a loadable module as well when that happens, but otherwise
> allows it to be built normally when DSA is either built-in or
> completely disabled.
>
> Fixes: d156576362c0 ("net: systemport: Establish lower/upper queue mapping")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Thanks Arnd.
--
Florian
^ permalink raw reply
* Recent payment notice to Anita Cochran
From: Anita Cochran @ 2017-10-16 18:48 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 372 bytes --]
\rHi ,
\rYour payment of $2744.29 to Anita Cochran was sent.
\rAllow up to 10 minutes for this transaction to appear.
\rSee your transaction details below:
http://horseoz.com/7363-RMBK/Payment-Details/
\rRespectfully Yours,
Anita Cochran
Please do not reply to this email. \rThis mailbox is for notification purposes only and you will not receive a reply.
^ permalink raw reply
* Re: [PATCH net-next] tools: bpftool: use more common tag format
From: Daniel Borkmann @ 2017-10-16 18:02 UTC (permalink / raw)
To: Jakub Kicinski, netdev; +Cc: oss-drivers, alexei.starovoitov
In-Reply-To: <20171016171254.26225-1-jakub.kicinski@netronome.com>
On 10/16/2017 07:12 PM, Jakub Kicinski wrote:
> Program tag is usually displayed as string of bytes without
> any separators (e.g. as "aa5520b1090cfeb6" vs MAC addr-like
> format bpftool uses currently: "aa:55:20:b1:09:0c:fe:b6").
> Make bptfool use the more common format both for displaying
> the tag and selecting the program by tag.
>
> This was pointed out in review but I misunderstood the comment.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH] sunrcp: make function _svc_create_xprt static
From: J . Bruce Fields @ 2017-10-16 18:06 UTC (permalink / raw)
To: Colin King
Cc: Jeff Layton, Trond Myklebust, Anna Schumaker, David S . Miller,
linux-nfs, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20171016134021.15702-1-colin.king@canonical.com>
Thanks, applied for 4.15.--b.
On Mon, Oct 16, 2017 at 02:40:21PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The function _svc_create_xprt is local to the source and
> does not need to be in global scope, so make it static.
>
> Cleans up sparse warning:
> symbol '_svc_create_xprt' was not declared. Should it be static?
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> net/sunrpc/svc_xprt.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index d16a8b423c20..18e87791350f 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -250,9 +250,9 @@ void svc_add_new_perm_xprt(struct svc_serv *serv, struct svc_xprt *new)
> svc_xprt_received(new);
> }
>
> -int _svc_create_xprt(struct svc_serv *serv, const char *xprt_name,
> - struct net *net, const int family,
> - const unsigned short port, int flags)
> +static int _svc_create_xprt(struct svc_serv *serv, const char *xprt_name,
> + struct net *net, const int family,
> + const unsigned short port, int flags)
> {
> struct svc_xprt_class *xcl;
>
> --
> 2.14.1
^ permalink raw reply
* Re: Ethtool question
From: Ben Greear @ 2017-10-16 18:10 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: David Miller, John W. Linville, netdev@vger.kernel.org
In-Reply-To: <CAJieiUhsxPRPBBV1WJR6NRfzDKt-8c=QYRHhwoW33KchSb-gXg@mail.gmail.com>
On 10/12/2017 03:00 PM, Roopa Prabhu wrote:
> On Thu, Oct 12, 2017 at 2:45 PM, Ben Greear <greearb@candelatech.com> wrote:
>> On 10/11/2017 01:49 PM, David Miller wrote:
>>>
>>> From: "John W. Linville" <linville@tuxdriver.com>
>>> Date: Wed, 11 Oct 2017 16:44:07 -0400
>>>
>>>> On Wed, Oct 11, 2017 at 09:51:56AM -0700, Ben Greear wrote:
>>>>>
>>>>> I noticed today that setting some ethtool settings to the same value
>>>>> returns an error code. I would think this should silently return
>>>>> success instead? Makes it easier to call it from scripts this way:
>>>>>
>>>>> [root@lf0313-6477 lanforge]# ethtool -L eth3 combined 1
>>>>> combined unmodified, ignoring
>>>>> no channel parameters changed, aborting
>>>>> current values: tx 0 rx 0 other 1 combined 1
>>>>> [root@lf0313-6477 lanforge]# echo $?
>>>>> 1
>>>>
>>>>
>>>> I just had this discussion a couple of months ago with someone. My
>>>> initial feeling was like you, a no-op is not a failure. But someone
>>>> convinced me otherwise...I will now endeavour to remember who that
>>>> was and how they convinced me...
>>>>
>>>> Anyone else have input here?
>>>
>>>
>>> I guess this usually happens when drivers don't support changing the
>>> settings at all. So they just make their ethtool operation for the
>>> 'set' always return an error.
>>>
>>> We could have a generic ethtool helper that does "get" and then if the
>>> "set" request is identical just return zero.
>>>
>>> But from another perspective, the error returned from the "set" in this
>>> situation also indicates to the user that the driver does not support
>>> the "set" operation which has value and meaning in and of itself. And
>>> we'd lose that with the given suggestion.
>>
>>
>> In my case, the driver (igb) does support the set, my program just made the
>> same
>> ethtool call several times and it fails after the initial change (that
>> actually
>> changes something), as best as I can figure.
>
>
> This error is returned by ethtool user-space. It does a get, check and
> then set if user has requested changes.
>
So, should we fix ethtool to return 0 in this case instead of an error code?
I think so. If the driver itself returns an error, then probably return the
error code and/or fix the driver as seems appropriate.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* RE: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
From: Rodney Cummings @ 2017-10-16 18:14 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vivien Didelot, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,
David S. Miller, Florian Fainelli, David Laight
In-Reply-To: <20171016165724.GE13339@lunn.ch>
I stand corrected. Thanks for clearing that up Andrew.
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Monday, October 16, 2017 11:57 AM
> To: Rodney Cummings <rodney.cummings@ni.com>
> Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> kernel@savoirfairelinux.com; David S. Miller <davem@davemloft.net>;
> Florian Fainelli <f.fainelli@gmail.com>; David Laight
> <David.Laight@ACULAB.COM>
> Subject: Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
>
> > How do you guarantee that this MAC address is used for PAUSE, and only
> for PAUSE?
>
> That is the only thing the data sheet says this MAC address is used
> for.
>
> Andrew
^ permalink raw reply
* [PATCH net v2] bpf: disallow arithmetic operations on context pointer
From: Jakub Kicinski @ 2017-10-16 18:16 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, ecree, Jakub Kicinski
Commit f1174f77b50c ("bpf/verifier: rework value tracking")
removed the crafty selection of which pointer types are
allowed to be modified. This is OK for most pointer types
since adjust_ptr_min_max_vals() will catch operations on
immutable pointers. One exception is PTR_TO_CTX which is
now allowed to be offseted freely.
The intent of aforementioned commit was to allow context
access via modified registers. The offset passed to
->is_valid_access() verifier callback has been adjusted
by the value of the variable offset.
What is missing, however, is taking the variable offset
into account when the context register is used. Or in terms
of the code adding the offset to the value passed to the
->convert_ctx_access() callback. This leads to the following
eBPF user code:
r1 += 68
r0 = *(u32 *)(r1 + 8)
exit
being translated to this in kernel space:
0: (07) r1 += 68
1: (61) r0 = *(u32 *)(r1 +180)
2: (95) exit
Offset 8 is corresponding to 180 in the kernel, but offset
76 is valid too. Verifier will "accept" access to offset
68+8=76 but then "convert" access to offset 8 as 180.
Effective access to offset 248 is beyond the kernel context.
(This is a __sk_buff example on a debug-heavy kernel -
packet mark is 8 -> 180, 76 would be data.)
Dereferencing the modified context pointer is not as easy
as dereferencing other types, because we have to translate
the access to reading a field in kernel structures which is
usually at a different offset and often of a different size.
To allow modifying the pointer we would have to make sure
that given eBPF instruction will always access the same
field or the fields accessed are "compatible" in terms of
offset and size...
Disallow dereferencing modified context pointers and add
to selftests the test case described here.
Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
Dave, a merge note - in net-next this will need env to be passed
to verbose().
v2:
- spell dereference correctly.
kernel/bpf/verifier.c | 8 ++++++--
tools/testing/selftests/bpf/test_verifier.c | 14 ++++++++++++++
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8b8d6ba39e23..20f3889c006e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1116,7 +1116,12 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
/* ctx accesses must be at a fixed offset, so that we can
* determine what type of data were returned.
*/
- if (!tnum_is_const(reg->var_off)) {
+ if (reg->off) {
+ verbose("dereference of modified ctx ptr R%d off=%d+%d, ctx+const is allowed, ctx+const+const is not\n",
+ regno, reg->off, off - reg->off);
+ return -EACCES;
+ }
+ if (!tnum_is_const(reg->var_off) || reg->var_off.value) {
char tn_buf[48];
tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
@@ -1124,7 +1129,6 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
tn_buf, off, size);
return -EACCES;
}
- off += reg->var_off.value;
err = check_ctx_access(env, insn_idx, off, size, t, ®_type);
if (!err && t == BPF_READ && value_regno >= 0) {
/* ctx access returns either a scalar, or a
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 26f3250bdcd2..3c7d3a45a3c5 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -6645,6 +6645,20 @@ static struct bpf_test tests[] = {
.errstr = "BPF_END uses reserved fields",
.result = REJECT,
},
+ {
+ "arithmetic ops make PTR_TO_CTX unusable",
+ .insns = {
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1,
+ offsetof(struct __sk_buff, data) -
+ offsetof(struct __sk_buff, mark)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, mark)),
+ BPF_EXIT_INSN(),
+ },
+ .errstr = "dereference of modified ctx ptr R1 off=68+8, ctx+const is allowed, ctx+const+const is not",
+ .result = REJECT,
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ },
};
static int probe_filter_length(const struct bpf_insn *fp)
--
2.14.1
^ permalink raw reply related
* [PATCH 1/3] bpf: Don't check for current being NULL
From: Richard Weinberger @ 2017-10-16 18:18 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, daniel, ast, Richard Weinberger
current is never NULL.
Signed-off-by: Richard Weinberger <richard@nod.at>
---
kernel/bpf/helpers.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 3d24e238221e..e8845adcd15e 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -120,9 +120,6 @@ BPF_CALL_0(bpf_get_current_pid_tgid)
{
struct task_struct *task = current;
- if (unlikely(!task))
- return -EINVAL;
-
return (u64) task->tgid << 32 | task->pid;
}
@@ -138,9 +135,6 @@ BPF_CALL_0(bpf_get_current_uid_gid)
kuid_t uid;
kgid_t gid;
- if (unlikely(!task))
- return -EINVAL;
-
current_uid_gid(&uid, &gid);
return (u64) from_kgid(&init_user_ns, gid) << 32 |
from_kuid(&init_user_ns, uid);
@@ -156,9 +150,6 @@ BPF_CALL_2(bpf_get_current_comm, char *, buf, u32, size)
{
struct task_struct *task = current;
- if (unlikely(!task))
- goto err_clear;
-
strncpy(buf, task->comm, size);
/* Verifier guarantees that size > 0. For task->comm exceeding
@@ -167,9 +158,6 @@ BPF_CALL_2(bpf_get_current_comm, char *, buf, u32, size)
*/
buf[size - 1] = 0;
return 0;
-err_clear:
- memset(buf, 0, size);
- return -EINVAL;
}
const struct bpf_func_proto bpf_get_current_comm_proto = {
--
2.13.6
^ permalink raw reply related
* [PATCH 2/3] bpf: Remove dead variable
From: Richard Weinberger @ 2017-10-16 18:18 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, daniel, ast, Richard Weinberger
In-Reply-To: <20171016181856.12497-1-richard@nod.at>
task is never used in bpf_get_current_uid_gid(), kill it.
Signed-off-by: Richard Weinberger <richard@nod.at>
---
kernel/bpf/helpers.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index e8845adcd15e..511c9d522cfc 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -131,7 +131,6 @@ const struct bpf_func_proto bpf_get_current_pid_tgid_proto = {
BPF_CALL_0(bpf_get_current_uid_gid)
{
- struct task_struct *task = current;
kuid_t uid;
kgid_t gid;
--
2.13.6
^ permalink raw reply related
* [PATCH 3/3] bpf: Make sure that ->comm does not change under us.
From: Richard Weinberger @ 2017-10-16 18:18 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, daniel, ast, Richard Weinberger
In-Reply-To: <20171016181856.12497-1-richard@nod.at>
Sadly we cannot use get_task_comm() since bpf_get_current_comm()
allows truncation.
Signed-off-by: Richard Weinberger <richard@nod.at>
---
kernel/bpf/helpers.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 511c9d522cfc..4b042b24524d 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -18,6 +18,7 @@
#include <linux/sched.h>
#include <linux/uidgid.h>
#include <linux/filter.h>
+#include <linux/sched/task.h>
/* If kernel subsystem is allowing eBPF programs to call this function,
* inside its own verifier_ops->get_func_proto() callback it should return
@@ -149,7 +150,9 @@ BPF_CALL_2(bpf_get_current_comm, char *, buf, u32, size)
{
struct task_struct *task = current;
+ task_lock(task);
strncpy(buf, task->comm, size);
+ task_unlock(task);
/* Verifier guarantees that size > 0. For task->comm exceeding
* size, guarantee that buf is %NUL-terminated. Unconditionally
--
2.13.6
^ permalink raw reply related
* Re: [PATCH v2 06/15] PCI: endpoint: make config_item_type const
From: Bjorn Helgaas @ 2017-10-16 18:24 UTC (permalink / raw)
To: Bhumika Goyal
Cc: julia.lawall, rjw, lenb, alexander.shishkin, jic23, knaack.h,
lars, pmeerw, dledford, sean.hefty, hal.rosenstock, hch, sagi,
kishon, bhelgaas, nab, balbi, gregkh, laurent.pinchart, jlbec,
ccaulfie, teigland, mfasheh, linux-acpi, linux-kernel, linux-iio,
linux-rdma, netdev, linux-nvme, linux-pci, linux-scsi,
target-devel, linux-usb, cluster-devel
In-Reply-To: <1508167134-6243-7-git-send-email-bhumirks@gmail.com>
On Mon, Oct 16, 2017 at 05:18:45PM +0200, Bhumika Goyal wrote:
> Make config_item_type structures const as they are either passed to a
> function having the argument as const or stored in the const "ci_type"
> field of a config_item structure.
>
> Done using Coccinelle.
>
> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Please apply this along with the rest of the series, since it depends
on an earlier patch in the series.
> ---
> * Changes in v2- Combine all the followup patches and the constification
> patches into a series.
>
> drivers/pci/endpoint/pci-ep-cfs.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
> index 424fdd6..4f74386 100644
> --- a/drivers/pci/endpoint/pci-ep-cfs.c
> +++ b/drivers/pci/endpoint/pci-ep-cfs.c
> @@ -150,7 +150,7 @@ static void pci_epc_epf_unlink(struct config_item *epc_item,
> .drop_link = pci_epc_epf_unlink,
> };
>
> -static struct config_item_type pci_epc_type = {
> +static const struct config_item_type pci_epc_type = {
> .ct_item_ops = &pci_epc_item_ops,
> .ct_attrs = pci_epc_attrs,
> .ct_owner = THIS_MODULE,
> @@ -361,7 +361,7 @@ static void pci_epf_release(struct config_item *item)
> .release = pci_epf_release,
> };
>
> -static struct config_item_type pci_epf_type = {
> +static const struct config_item_type pci_epf_type = {
> .ct_item_ops = &pci_epf_ops,
> .ct_attrs = pci_epf_attrs,
> .ct_owner = THIS_MODULE,
> @@ -400,7 +400,7 @@ static void pci_epf_drop(struct config_group *group, struct config_item *item)
> .drop_item = &pci_epf_drop,
> };
>
> -static struct config_item_type pci_epf_group_type = {
> +static const struct config_item_type pci_epf_group_type = {
> .ct_group_ops = &pci_epf_group_ops,
> .ct_owner = THIS_MODULE,
> };
> @@ -428,15 +428,15 @@ void pci_ep_cfs_remove_epf_group(struct config_group *group)
> }
> EXPORT_SYMBOL(pci_ep_cfs_remove_epf_group);
>
> -static struct config_item_type pci_functions_type = {
> +static const struct config_item_type pci_functions_type = {
> .ct_owner = THIS_MODULE,
> };
>
> -static struct config_item_type pci_controllers_type = {
> +static const struct config_item_type pci_controllers_type = {
> .ct_owner = THIS_MODULE,
> };
>
> -static struct config_item_type pci_ep_type = {
> +static const struct config_item_type pci_ep_type = {
> .ct_owner = THIS_MODULE,
> };
>
> --
> 1.9.1
>
^ permalink raw reply
* [PATCH v3 0/6] adapt DPAA drivers for DSA
From: Madalin Bucur @ 2017-10-16 18:36 UTC (permalink / raw)
To: netdev, davem; +Cc: f.fainelli, andrew, vivien.didelot, junote, linux-kernel
Junote Cai reported that he was not able to get a DSA setup involving the
DPAA/FMAN driver to work and narrowed it down to of_find_net_device_by_node()
call in DSA setup. The initial attempt to fix this by adding of_node to the
platform device results in a second, failed, probing of the FMan MAC driver
against the new platform device created for the DPAA Ethernet driver.
Solve these issues by removing the of_node pointer from the platform device
and changing the net_dev dev to the of_device dev to ensure the DSA init
will be able to find the DPAA net_dev using of_find_net_device_by_node().
Several changes were required to enable this solution: refactoring the
adjust_link (also resulted in lesser, cleaner code) and renaming the fman
kernel modules to keep the legacy udev rules happy.
Changes in v2:
- fix issue on error path in "dpaa_eth: change device used" patch
- cleanup the dpaa_eth_probe() error paths
Changes in v3:
- remove obsolete comment in moved code
- add explanation for module rename
Madalin Bucur (6):
fsl/fman: remove of_node
dpaa_eth: move of_phy_connect() to the eth driver
dpaa_eth: change device used
dpaa_eth: cleanup dpaa_eth_probe() error paths
fsl/fman: add dpaa in module names
dpaa_eth: remove obsolete comment
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 139 +++++++++++++------------
drivers/net/ethernet/freescale/fman/Makefile | 12 +--
drivers/net/ethernet/freescale/fman/mac.c | 135 +++++++-----------------
drivers/net/ethernet/freescale/fman/mac.h | 6 +-
4 files changed, 115 insertions(+), 177 deletions(-)
--
2.1.0
^ permalink raw reply
* [PATCH v3 1/6] fsl/fman: remove of_node
From: Madalin Bucur @ 2017-10-16 18:36 UTC (permalink / raw)
To: netdev, davem; +Cc: f.fainelli, andrew, vivien.didelot, junote, linux-kernel
In-Reply-To: <1508178970-28945-1-git-send-email-madalin.bucur@nxp.com>
The FMan MAC driver allocates a platform device for the Ethernet
driver to probe on. Setting pdev->dev.of_node with the MAC node
triggers the MAC driver probing of the new platform device. While
this fails quickly and does not affect the functionality of the
drivers, it is incorrect and must be removed. This was added to
address a report that DSA code using of_find_net_device_by_node()
is unable to use the DPAA interfaces. Error message seen before
this fix:
fsl_mac dpaa-ethernet.0: __devm_request_mem_region(mac) failed
fsl_mac: probe of dpaa-ethernet.0 failed with error -16
Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
---
drivers/net/ethernet/freescale/fman/mac.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 387eb4a..9a265f8 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -623,7 +623,6 @@ static struct platform_device *dpaa_eth_add_device(int fman_id,
goto no_mem;
}
- pdev->dev.of_node = node;
pdev->dev.parent = priv->dev;
set_dma_ops(&pdev->dev, get_dma_ops(priv->dev));
--
2.1.0
^ permalink raw reply related
* [PATCH v3 2/6] dpaa_eth: move of_phy_connect() to the eth driver
From: Madalin Bucur @ 2017-10-16 18:36 UTC (permalink / raw)
To: netdev, davem; +Cc: f.fainelli, andrew, vivien.didelot, junote, linux-kernel
In-Reply-To: <1508178970-28945-1-git-send-email-madalin.bucur@nxp.com>
Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 48 +++++++++++--
drivers/net/ethernet/freescale/fman/mac.c | 97 ++++++--------------------
drivers/net/ethernet/freescale/fman/mac.h | 5 +-
3 files changed, 66 insertions(+), 84 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 4225806..7cf61d6 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2435,6 +2435,48 @@ static void dpaa_eth_napi_disable(struct dpaa_priv *priv)
}
}
+static void dpaa_adjust_link(struct net_device *net_dev)
+{
+ struct mac_device *mac_dev;
+ struct dpaa_priv *priv;
+
+ priv = netdev_priv(net_dev);
+ mac_dev = priv->mac_dev;
+ mac_dev->adjust_link(mac_dev);
+}
+
+static int dpaa_phy_init(struct net_device *net_dev)
+{
+ struct mac_device *mac_dev;
+ struct phy_device *phy_dev;
+ struct dpaa_priv *priv;
+
+ priv = netdev_priv(net_dev);
+ mac_dev = priv->mac_dev;
+
+ phy_dev = of_phy_connect(net_dev, mac_dev->phy_node,
+ &dpaa_adjust_link, 0,
+ mac_dev->phy_if);
+ if (!phy_dev) {
+ netif_err(priv, ifup, net_dev, "init_phy() failed\n");
+ return -ENODEV;
+ }
+
+ /* Remove any features not supported by the controller */
+ phy_dev->supported &= mac_dev->if_support;
+
+ /* Enable the symmetric and asymmetric PAUSE frame advertisements,
+ * as most of the PHY drivers do not enable them by default.
+ */
+ phy_dev->supported |= (SUPPORTED_Pause | SUPPORTED_Asym_Pause);
+ phy_dev->advertising = phy_dev->supported;
+
+ mac_dev->phy_dev = phy_dev;
+ net_dev->phydev = phy_dev;
+
+ return 0;
+}
+
static int dpaa_open(struct net_device *net_dev)
{
struct mac_device *mac_dev;
@@ -2445,12 +2487,8 @@ static int dpaa_open(struct net_device *net_dev)
mac_dev = priv->mac_dev;
dpaa_eth_napi_enable(priv);
- net_dev->phydev = mac_dev->init_phy(net_dev, priv->mac_dev);
- if (!net_dev->phydev) {
- netif_err(priv, ifup, net_dev, "init_phy() failed\n");
- err = -ENODEV;
+ if (dpaa_phy_init(net_dev))
goto phy_init_failed;
- }
for (i = 0; i < ARRAY_SIZE(mac_dev->port); i++) {
err = fman_port_enable(mac_dev->port[i]);
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 9a265f8..a0a3107 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -57,9 +57,7 @@ struct mac_priv_s {
struct device *dev;
void __iomem *vaddr;
u8 cell_index;
- phy_interface_t phy_if;
struct fman *fman;
- struct device_node *phy_node;
struct device_node *internal_phy_node;
/* List of multicast addresses */
struct list_head mc_addr_list;
@@ -106,7 +104,7 @@ static void set_fman_mac_params(struct mac_device *mac_dev,
resource_size(mac_dev->res));
memcpy(¶ms->addr, mac_dev->addr, sizeof(mac_dev->addr));
params->max_speed = priv->max_speed;
- params->phy_if = priv->phy_if;
+ params->phy_if = mac_dev->phy_if;
params->basex_if = false;
params->mac_id = priv->cell_index;
params->fm = (void *)priv->fman;
@@ -419,15 +417,12 @@ void fman_get_pause_cfg(struct mac_device *mac_dev, bool *rx_pause,
}
EXPORT_SYMBOL(fman_get_pause_cfg);
-static void adjust_link_void(struct net_device *net_dev)
+static void adjust_link_void(struct mac_device *mac_dev)
{
}
-static void adjust_link_dtsec(struct net_device *net_dev)
+static void adjust_link_dtsec(struct mac_device *mac_dev)
{
- struct device *dev = net_dev->dev.parent;
- struct dpaa_eth_data *eth_data = dev->platform_data;
- struct mac_device *mac_dev = eth_data->mac_dev;
struct phy_device *phy_dev = mac_dev->phy_dev;
struct fman_mac *fman_mac;
bool rx_pause, tx_pause;
@@ -444,14 +439,12 @@ static void adjust_link_dtsec(struct net_device *net_dev)
fman_get_pause_cfg(mac_dev, &rx_pause, &tx_pause);
err = fman_set_mac_active_pause(mac_dev, rx_pause, tx_pause);
if (err < 0)
- netdev_err(net_dev, "fman_set_mac_active_pause() = %d\n", err);
+ dev_err(mac_dev->priv->dev, "fman_set_mac_active_pause() = %d\n",
+ err);
}
-static void adjust_link_memac(struct net_device *net_dev)
+static void adjust_link_memac(struct mac_device *mac_dev)
{
- struct device *dev = net_dev->dev.parent;
- struct dpaa_eth_data *eth_data = dev->platform_data;
- struct mac_device *mac_dev = eth_data->mac_dev;
struct phy_device *phy_dev = mac_dev->phy_dev;
struct fman_mac *fman_mac;
bool rx_pause, tx_pause;
@@ -463,60 +456,12 @@ static void adjust_link_memac(struct net_device *net_dev)
fman_get_pause_cfg(mac_dev, &rx_pause, &tx_pause);
err = fman_set_mac_active_pause(mac_dev, rx_pause, tx_pause);
if (err < 0)
- netdev_err(net_dev, "fman_set_mac_active_pause() = %d\n", err);
-}
-
-/* Initializes driver's PHY state, and attaches to the PHY.
- * Returns 0 on success.
- */
-static struct phy_device *init_phy(struct net_device *net_dev,
- struct mac_device *mac_dev,
- void (*adj_lnk)(struct net_device *))
-{
- struct phy_device *phy_dev;
- struct mac_priv_s *priv = mac_dev->priv;
-
- phy_dev = of_phy_connect(net_dev, priv->phy_node, adj_lnk, 0,
- priv->phy_if);
- if (!phy_dev) {
- netdev_err(net_dev, "Could not connect to PHY\n");
- return NULL;
- }
-
- /* Remove any features not supported by the controller */
- phy_dev->supported &= mac_dev->if_support;
- /* Enable the symmetric and asymmetric PAUSE frame advertisements,
- * as most of the PHY drivers do not enable them by default.
- */
- phy_dev->supported |= (SUPPORTED_Pause | SUPPORTED_Asym_Pause);
- phy_dev->advertising = phy_dev->supported;
-
- mac_dev->phy_dev = phy_dev;
-
- return phy_dev;
-}
-
-static struct phy_device *dtsec_init_phy(struct net_device *net_dev,
- struct mac_device *mac_dev)
-{
- return init_phy(net_dev, mac_dev, &adjust_link_dtsec);
-}
-
-static struct phy_device *tgec_init_phy(struct net_device *net_dev,
- struct mac_device *mac_dev)
-{
- return init_phy(net_dev, mac_dev, adjust_link_void);
-}
-
-static struct phy_device *memac_init_phy(struct net_device *net_dev,
- struct mac_device *mac_dev)
-{
- return init_phy(net_dev, mac_dev, &adjust_link_memac);
+ dev_err(mac_dev->priv->dev, "fman_set_mac_active_pause() = %d\n",
+ err);
}
static void setup_dtsec(struct mac_device *mac_dev)
{
- mac_dev->init_phy = dtsec_init_phy;
mac_dev->init = dtsec_initialization;
mac_dev->set_promisc = dtsec_set_promiscuous;
mac_dev->change_addr = dtsec_modify_mac_address;
@@ -528,14 +473,13 @@ static void setup_dtsec(struct mac_device *mac_dev)
mac_dev->set_multi = set_multi;
mac_dev->start = start;
mac_dev->stop = stop;
-
+ mac_dev->adjust_link = adjust_link_dtsec;
mac_dev->priv->enable = dtsec_enable;
mac_dev->priv->disable = dtsec_disable;
}
static void setup_tgec(struct mac_device *mac_dev)
{
- mac_dev->init_phy = tgec_init_phy;
mac_dev->init = tgec_initialization;
mac_dev->set_promisc = tgec_set_promiscuous;
mac_dev->change_addr = tgec_modify_mac_address;
@@ -547,14 +491,13 @@ static void setup_tgec(struct mac_device *mac_dev)
mac_dev->set_multi = set_multi;
mac_dev->start = start;
mac_dev->stop = stop;
-
+ mac_dev->adjust_link = adjust_link_void;
mac_dev->priv->enable = tgec_enable;
mac_dev->priv->disable = tgec_disable;
}
static void setup_memac(struct mac_device *mac_dev)
{
- mac_dev->init_phy = memac_init_phy;
mac_dev->init = memac_initialization;
mac_dev->set_promisc = memac_set_promiscuous;
mac_dev->change_addr = memac_modify_mac_address;
@@ -566,7 +509,7 @@ static void setup_memac(struct mac_device *mac_dev)
mac_dev->set_multi = set_multi;
mac_dev->start = start;
mac_dev->stop = stop;
-
+ mac_dev->adjust_link = adjust_link_memac;
mac_dev->priv->enable = memac_enable;
mac_dev->priv->disable = memac_disable;
}
@@ -850,13 +793,13 @@ static int mac_probe(struct platform_device *_of_dev)
mac_node);
phy_if = PHY_INTERFACE_MODE_SGMII;
}
- priv->phy_if = phy_if;
+ mac_dev->phy_if = phy_if;
- priv->speed = phy2speed[priv->phy_if];
+ priv->speed = phy2speed[mac_dev->phy_if];
priv->max_speed = priv->speed;
mac_dev->if_support = DTSEC_SUPPORTED;
/* We don't support half-duplex in SGMII mode */
- if (priv->phy_if == PHY_INTERFACE_MODE_SGMII)
+ if (mac_dev->phy_if == PHY_INTERFACE_MODE_SGMII)
mac_dev->if_support &= ~(SUPPORTED_10baseT_Half |
SUPPORTED_100baseT_Half);
@@ -865,12 +808,12 @@ static int mac_probe(struct platform_device *_of_dev)
mac_dev->if_support |= SUPPORTED_1000baseT_Full;
/* The 10G interface only supports one mode */
- if (priv->phy_if == PHY_INTERFACE_MODE_XGMII)
+ if (mac_dev->phy_if == PHY_INTERFACE_MODE_XGMII)
mac_dev->if_support = SUPPORTED_10000baseT_Full;
/* Get the rest of the PHY information */
- priv->phy_node = of_parse_phandle(mac_node, "phy-handle", 0);
- if (!priv->phy_node && of_phy_is_fixed_link(mac_node)) {
+ mac_dev->phy_node = of_parse_phandle(mac_node, "phy-handle", 0);
+ if (!mac_dev->phy_node && of_phy_is_fixed_link(mac_node)) {
struct phy_device *phy;
err = of_phy_register_fixed_link(mac_node);
@@ -884,8 +827,8 @@ static int mac_probe(struct platform_device *_of_dev)
goto _return_dev_set_drvdata;
}
- priv->phy_node = of_node_get(mac_node);
- phy = of_phy_find_device(priv->phy_node);
+ mac_dev->phy_node = of_node_get(mac_node);
+ phy = of_phy_find_device(mac_dev->phy_node);
if (!phy) {
err = -EINVAL;
goto _return_dev_set_drvdata;
@@ -903,7 +846,7 @@ static int mac_probe(struct platform_device *_of_dev)
err = mac_dev->init(mac_dev);
if (err < 0) {
dev_err(dev, "mac_dev->init() = %d\n", err);
- of_node_put(priv->phy_node);
+ of_node_put(mac_dev->phy_node);
goto _return_dev_set_drvdata;
}
diff --git a/drivers/net/ethernet/freescale/fman/mac.h b/drivers/net/ethernet/freescale/fman/mac.h
index d7313f0..1ca85a1 100644
--- a/drivers/net/ethernet/freescale/fman/mac.h
+++ b/drivers/net/ethernet/freescale/fman/mac.h
@@ -50,6 +50,8 @@ struct mac_device {
struct fman_port *port[2];
u32 if_support;
struct phy_device *phy_dev;
+ phy_interface_t phy_if;
+ struct device_node *phy_node;
bool autoneg_pause;
bool rx_pause_req;
@@ -58,11 +60,10 @@ struct mac_device {
bool tx_pause_active;
bool promisc;
- struct phy_device *(*init_phy)(struct net_device *net_dev,
- struct mac_device *mac_dev);
int (*init)(struct mac_device *mac_dev);
int (*start)(struct mac_device *mac_dev);
int (*stop)(struct mac_device *mac_dev);
+ void (*adjust_link)(struct mac_device *mac_dev);
int (*set_promisc)(struct fman_mac *mac_dev, bool enable);
int (*change_addr)(struct fman_mac *mac_dev, enet_addr_t *enet_addr);
int (*set_multi)(struct net_device *net_dev,
--
2.1.0
^ permalink raw reply related
* [PATCH v3 3/6] dpaa_eth: change device used
From: Madalin Bucur @ 2017-10-16 18:36 UTC (permalink / raw)
To: netdev, davem; +Cc: f.fainelli, andrew, vivien.didelot, junote, linux-kernel
In-Reply-To: <1508178970-28945-1-git-send-email-madalin.bucur@nxp.com>
Change device used for DMA mapping to the MAC device that is an
of_device, with proper DMA ops. Using this device for the netdevice
should also address the issue with DSA scenarios that need the
netdevice to be backed by an of_device.
Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 42 ++++++++------------------
drivers/net/ethernet/freescale/fman/mac.c | 37 +++++++++--------------
drivers/net/ethernet/freescale/fman/mac.h | 1 -
3 files changed, 27 insertions(+), 53 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 7cf61d6..823aa65 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -385,34 +385,19 @@ static int dpaa_setup_tc(struct net_device *net_dev, enum tc_setup_type type,
static struct mac_device *dpaa_mac_dev_get(struct platform_device *pdev)
{
- struct platform_device *of_dev;
struct dpaa_eth_data *eth_data;
- struct device *dpaa_dev, *dev;
- struct device_node *mac_node;
+ struct device *dpaa_dev;
struct mac_device *mac_dev;
dpaa_dev = &pdev->dev;
eth_data = dpaa_dev->platform_data;
- if (!eth_data)
+ if (!eth_data) {
+ dev_err(dpaa_dev, "eth_data missing\n");
return ERR_PTR(-ENODEV);
-
- mac_node = eth_data->mac_node;
-
- of_dev = of_find_device_by_node(mac_node);
- if (!of_dev) {
- dev_err(dpaa_dev, "of_find_device_by_node(%pOF) failed\n",
- mac_node);
- of_node_put(mac_node);
- return ERR_PTR(-EINVAL);
}
- of_node_put(mac_node);
-
- dev = &of_dev->dev;
-
- mac_dev = dev_get_drvdata(dev);
+ mac_dev = eth_data->mac_dev;
if (!mac_dev) {
- dev_err(dpaa_dev, "dev_get_drvdata(%s) failed\n",
- dev_name(dev));
+ dev_err(dpaa_dev, "mac_dev missing\n");
return ERR_PTR(-EINVAL);
}
@@ -2696,7 +2681,13 @@ static int dpaa_eth_probe(struct platform_device *pdev)
int err = 0, i, channel;
struct device *dev;
- dev = &pdev->dev;
+ /* device used for DMA mapping */
+ dev = pdev->dev.parent;
+ err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(40));
+ if (err) {
+ dev_err(dev, "dma_coerce_mask_and_coherent() failed\n");
+ return err;
+ }
/* Allocate this early, so we can store relevant information in
* the private area
@@ -2738,14 +2729,6 @@ static int dpaa_eth_probe(struct platform_device *pdev)
priv->buf_layout[RX].priv_data_size = DPAA_RX_PRIV_DATA_SIZE; /* Rx */
priv->buf_layout[TX].priv_data_size = DPAA_TX_PRIV_DATA_SIZE; /* Tx */
- /* device used for DMA mapping */
- set_dma_ops(dev, get_dma_ops(&pdev->dev));
- err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(40));
- if (err) {
- dev_err(dev, "dma_coerce_mask_and_coherent() failed\n");
- goto dev_mask_failed;
- }
-
/* bp init */
for (i = 0; i < DPAA_BPS_NUM; i++) {
int err;
@@ -2879,7 +2862,6 @@ static int dpaa_eth_probe(struct platform_device *pdev)
dpaa_bps_free(priv);
bp_create_failed:
fq_probe_failed:
-dev_mask_failed:
mac_probe_failed:
dev_set_drvdata(dev, NULL);
free_netdev(net_dev);
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index a0a3107..1d6da1e 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -542,8 +542,7 @@ static const u16 phy2speed[] = {
};
static struct platform_device *dpaa_eth_add_device(int fman_id,
- struct mac_device *mac_dev,
- struct device_node *node)
+ struct mac_device *mac_dev)
{
struct platform_device *pdev;
struct dpaa_eth_data data;
@@ -556,10 +555,8 @@ static struct platform_device *dpaa_eth_add_device(int fman_id,
data.mac_dev = mac_dev;
data.mac_hw_id = priv->cell_index;
data.fman_hw_id = fman_id;
- data.mac_node = node;
mutex_lock(ð_lock);
-
pdev = platform_device_alloc("dpaa-ethernet", dpaa_eth_dev_cnt);
if (!pdev) {
ret = -ENOMEM;
@@ -648,9 +645,6 @@ static int mac_probe(struct platform_device *_of_dev)
goto _return;
}
- /* Register mac_dev */
- dev_set_drvdata(dev, mac_dev);
-
INIT_LIST_HEAD(&priv->mc_addr_list);
/* Get the FM node */
@@ -659,7 +653,7 @@ static int mac_probe(struct platform_device *_of_dev)
dev_err(dev, "of_get_parent(%pOF) failed\n",
mac_node);
err = -EINVAL;
- goto _return_dev_set_drvdata;
+ goto _return_of_get_parent;
}
of_dev = of_find_device_by_node(dev_node);
@@ -693,7 +687,7 @@ static int mac_probe(struct platform_device *_of_dev)
if (err < 0) {
dev_err(dev, "of_address_to_resource(%pOF) = %d\n",
mac_node, err);
- goto _return_dev_set_drvdata;
+ goto _return_of_get_parent;
}
mac_dev->res = __devm_request_region(dev,
@@ -703,7 +697,7 @@ static int mac_probe(struct platform_device *_of_dev)
if (!mac_dev->res) {
dev_err(dev, "__devm_request_mem_region(mac) failed\n");
err = -EBUSY;
- goto _return_dev_set_drvdata;
+ goto _return_of_get_parent;
}
priv->vaddr = devm_ioremap(dev, mac_dev->res->start,
@@ -711,7 +705,7 @@ static int mac_probe(struct platform_device *_of_dev)
if (!priv->vaddr) {
dev_err(dev, "devm_ioremap() failed\n");
err = -EIO;
- goto _return_dev_set_drvdata;
+ goto _return_of_get_parent;
}
if (!of_device_is_available(mac_node)) {
@@ -728,7 +722,7 @@ static int mac_probe(struct platform_device *_of_dev)
if (err) {
dev_err(dev, "failed to read cell-index for %pOF\n", mac_node);
err = -EINVAL;
- goto _return_dev_set_drvdata;
+ goto _return_of_get_parent;
}
priv->cell_index = (u8)val;
@@ -737,7 +731,7 @@ static int mac_probe(struct platform_device *_of_dev)
if (!mac_addr) {
dev_err(dev, "of_get_mac_address(%pOF) failed\n", mac_node);
err = -EINVAL;
- goto _return_dev_set_drvdata;
+ goto _return_of_get_parent;
}
memcpy(mac_dev->addr, mac_addr, sizeof(mac_dev->addr));
@@ -747,14 +741,14 @@ static int mac_probe(struct platform_device *_of_dev)
dev_err(dev, "of_count_phandle_with_args(%pOF, fsl,fman-ports) failed\n",
mac_node);
err = nph;
- goto _return_dev_set_drvdata;
+ goto _return_of_get_parent;
}
if (nph != ARRAY_SIZE(mac_dev->port)) {
dev_err(dev, "Not supported number of fman-ports handles of mac node %pOF from device tree\n",
mac_node);
err = -EINVAL;
- goto _return_dev_set_drvdata;
+ goto _return_of_get_parent;
}
for (i = 0; i < ARRAY_SIZE(mac_dev->port); i++) {
@@ -818,20 +812,20 @@ static int mac_probe(struct platform_device *_of_dev)
err = of_phy_register_fixed_link(mac_node);
if (err)
- goto _return_dev_set_drvdata;
+ goto _return_of_get_parent;
priv->fixed_link = kzalloc(sizeof(*priv->fixed_link),
GFP_KERNEL);
if (!priv->fixed_link) {
err = -ENOMEM;
- goto _return_dev_set_drvdata;
+ goto _return_of_get_parent;
}
mac_dev->phy_node = of_node_get(mac_node);
phy = of_phy_find_device(mac_dev->phy_node);
if (!phy) {
err = -EINVAL;
- goto _return_dev_set_drvdata;
+ goto _return_of_get_parent;
}
priv->fixed_link->link = phy->link;
@@ -847,7 +841,7 @@ static int mac_probe(struct platform_device *_of_dev)
if (err < 0) {
dev_err(dev, "mac_dev->init() = %d\n", err);
of_node_put(mac_dev->phy_node);
- goto _return_dev_set_drvdata;
+ goto _return_of_get_parent;
}
/* pause frame autonegotiation enabled */
@@ -868,7 +862,7 @@ static int mac_probe(struct platform_device *_of_dev)
mac_dev->addr[0], mac_dev->addr[1], mac_dev->addr[2],
mac_dev->addr[3], mac_dev->addr[4], mac_dev->addr[5]);
- priv->eth_dev = dpaa_eth_add_device(fman_id, mac_dev, mac_node);
+ priv->eth_dev = dpaa_eth_add_device(fman_id, mac_dev);
if (IS_ERR(priv->eth_dev)) {
dev_err(dev, "failed to add Ethernet platform device for MAC %d\n",
priv->cell_index);
@@ -879,9 +873,8 @@ static int mac_probe(struct platform_device *_of_dev)
_return_of_node_put:
of_node_put(dev_node);
-_return_dev_set_drvdata:
+_return_of_get_parent:
kfree(priv->fixed_link);
- dev_set_drvdata(dev, NULL);
_return:
return err;
}
diff --git a/drivers/net/ethernet/freescale/fman/mac.h b/drivers/net/ethernet/freescale/fman/mac.h
index 1ca85a1..eefb335 100644
--- a/drivers/net/ethernet/freescale/fman/mac.h
+++ b/drivers/net/ethernet/freescale/fman/mac.h
@@ -83,7 +83,6 @@ struct mac_device {
};
struct dpaa_eth_data {
- struct device_node *mac_node;
struct mac_device *mac_dev;
int mac_hw_id;
int fman_hw_id;
--
2.1.0
^ permalink raw reply related
* [PATCH v3 4/6] dpaa_eth: cleanup dpaa_eth_probe() error paths
From: Madalin Bucur @ 2017-10-16 18:36 UTC (permalink / raw)
To: netdev, davem; +Cc: f.fainelli, andrew, vivien.didelot, junote, linux-kernel
In-Reply-To: <1508178970-28945-1-git-send-email-madalin.bucur@nxp.com>
Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 53 ++++++++++----------------
1 file changed, 20 insertions(+), 33 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 823aa65..c6b97a1 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2695,7 +2695,7 @@ static int dpaa_eth_probe(struct platform_device *pdev)
net_dev = alloc_etherdev_mq(sizeof(*priv), DPAA_ETH_TXQ_NUM);
if (!net_dev) {
dev_err(dev, "alloc_etherdev_mq() failed\n");
- goto alloc_etherdev_mq_failed;
+ return -ENOMEM;
}
/* Do this here, so we can be verbose early */
@@ -2711,7 +2711,7 @@ static int dpaa_eth_probe(struct platform_device *pdev)
if (IS_ERR(mac_dev)) {
dev_err(dev, "dpaa_mac_dev_get() failed\n");
err = PTR_ERR(mac_dev);
- goto mac_probe_failed;
+ goto free_netdev;
}
/* If fsl_fm_max_frm is set to a higher value than the all-common 1500,
@@ -2735,7 +2735,7 @@ static int dpaa_eth_probe(struct platform_device *pdev)
dpaa_bps[i] = dpaa_bp_alloc(dev);
if (IS_ERR(dpaa_bps[i]))
- return PTR_ERR(dpaa_bps[i]);
+ goto free_dpaa_bps;
/* the raw size of the buffers used for reception */
dpaa_bps[i]->raw_size = bpool_buffer_raw_size(i, DPAA_BPS_NUM);
/* avoid runtime computations by keeping the usable size here */
@@ -2743,11 +2743,8 @@ static int dpaa_eth_probe(struct platform_device *pdev)
dpaa_bps[i]->dev = dev;
err = dpaa_bp_alloc_pool(dpaa_bps[i]);
- if (err < 0) {
- dpaa_bps_free(priv);
- priv->dpaa_bps[i] = NULL;
- goto bp_create_failed;
- }
+ if (err < 0)
+ goto free_dpaa_bps;
priv->dpaa_bps[i] = dpaa_bps[i];
}
@@ -2758,7 +2755,7 @@ static int dpaa_eth_probe(struct platform_device *pdev)
err = dpaa_alloc_all_fqs(dev, &priv->dpaa_fq_list, &port_fqs);
if (err < 0) {
dev_err(dev, "dpaa_alloc_all_fqs() failed\n");
- goto fq_probe_failed;
+ goto free_dpaa_bps;
}
priv->mac_dev = mac_dev;
@@ -2767,7 +2764,7 @@ static int dpaa_eth_probe(struct platform_device *pdev)
if (channel < 0) {
dev_err(dev, "dpaa_get_channel() failed\n");
err = channel;
- goto get_channel_failed;
+ goto free_dpaa_bps;
}
priv->channel = (u16)channel;
@@ -2787,20 +2784,20 @@ static int dpaa_eth_probe(struct platform_device *pdev)
err = dpaa_eth_cgr_init(priv);
if (err < 0) {
dev_err(dev, "Error initializing CGR\n");
- goto tx_cgr_init_failed;
+ goto free_dpaa_bps;
}
err = dpaa_ingress_cgr_init(priv);
if (err < 0) {
dev_err(dev, "Error initializing ingress CGR\n");
- goto rx_cgr_init_failed;
+ goto delete_egress_cgr;
}
/* Add the FQs to the interface, and make them active */
list_for_each_entry_safe(dpaa_fq, tmp, &priv->dpaa_fq_list, list) {
err = dpaa_fq_init(dpaa_fq, false);
if (err < 0)
- goto fq_alloc_failed;
+ goto free_dpaa_fqs;
}
priv->tx_headroom = dpaa_get_headroom(&priv->buf_layout[TX]);
@@ -2810,7 +2807,7 @@ static int dpaa_eth_probe(struct platform_device *pdev)
err = dpaa_eth_init_ports(mac_dev, dpaa_bps, DPAA_BPS_NUM, &port_fqs,
&priv->buf_layout[0], dev);
if (err)
- goto init_ports_failed;
+ goto free_dpaa_fqs;
/* Rx traffic distribution based on keygen hashing defaults to on */
priv->keygen_in_use = true;
@@ -2819,7 +2816,7 @@ static int dpaa_eth_probe(struct platform_device *pdev)
if (!priv->percpu_priv) {
dev_err(dev, "devm_alloc_percpu() failed\n");
err = -ENOMEM;
- goto alloc_percpu_failed;
+ goto free_dpaa_fqs;
}
for_each_possible_cpu(i) {
percpu_priv = per_cpu_ptr(priv->percpu_priv, i);
@@ -2832,11 +2829,11 @@ static int dpaa_eth_probe(struct platform_device *pdev)
/* Initialize NAPI */
err = dpaa_napi_add(net_dev);
if (err < 0)
- goto napi_add_failed;
+ goto delete_dpaa_napi;
err = dpaa_netdev_init(net_dev, &dpaa_ops, tx_timeout);
if (err < 0)
- goto netdev_init_failed;
+ goto delete_dpaa_napi;
dpaa_eth_sysfs_init(&net_dev->dev);
@@ -2845,31 +2842,21 @@ static int dpaa_eth_probe(struct platform_device *pdev)
return 0;
-netdev_init_failed:
-napi_add_failed:
+delete_dpaa_napi:
dpaa_napi_del(net_dev);
-alloc_percpu_failed:
-init_ports_failed:
+free_dpaa_fqs:
dpaa_fq_free(dev, &priv->dpaa_fq_list);
-fq_alloc_failed:
qman_delete_cgr_safe(&priv->ingress_cgr);
qman_release_cgrid(priv->ingress_cgr.cgrid);
-rx_cgr_init_failed:
+delete_egress_cgr:
qman_delete_cgr_safe(&priv->cgr_data.cgr);
qman_release_cgrid(priv->cgr_data.cgr.cgrid);
-tx_cgr_init_failed:
-get_channel_failed:
+free_dpaa_bps:
dpaa_bps_free(priv);
-bp_create_failed:
-fq_probe_failed:
-mac_probe_failed:
+free_netdev:
dev_set_drvdata(dev, NULL);
free_netdev(net_dev);
-alloc_etherdev_mq_failed:
- for (i = 0; i < DPAA_BPS_NUM && dpaa_bps[i]; i++) {
- if (atomic_read(&dpaa_bps[i]->refs) == 0)
- devm_kfree(dev, dpaa_bps[i]);
- }
+
return err;
}
--
2.1.0
^ permalink raw reply related
* [PATCH v3 5/6] fsl/fman: add dpaa in module names
From: Madalin Bucur @ 2017-10-16 18:36 UTC (permalink / raw)
To: netdev, davem; +Cc: f.fainelli, andrew, vivien.didelot, junote, linux-kernel
In-Reply-To: <1508178970-28945-1-git-send-email-madalin.bucur@nxp.com>
This change just renames the FMan driver modules, using a common prefix
for the DPAA FMan and DPAA Ethernet drivers. Besides making the names more
aligned, this allows writing udev rules that match on either driver name,
if needed, using the fsl_dpaa_* prefix. The change of netdev dev required
for the DSA probing makes the previous rules written using this prefix
fail, this change makes them work again, ensuring backwards compatibility
for their users.
Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
---
drivers/net/ethernet/freescale/fman/Makefile | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/Makefile b/drivers/net/ethernet/freescale/fman/Makefile
index 2c38119..4ae524a 100644
--- a/drivers/net/ethernet/freescale/fman/Makefile
+++ b/drivers/net/ethernet/freescale/fman/Makefile
@@ -1,9 +1,9 @@
subdir-ccflags-y += -I$(srctree)/drivers/net/ethernet/freescale/fman
-obj-$(CONFIG_FSL_FMAN) += fsl_fman.o
-obj-$(CONFIG_FSL_FMAN) += fsl_fman_port.o
-obj-$(CONFIG_FSL_FMAN) += fsl_mac.o
+obj-$(CONFIG_FSL_FMAN) += fsl_dpaa_fman.o
+obj-$(CONFIG_FSL_FMAN) += fsl_dpaa_fman_port.o
+obj-$(CONFIG_FSL_FMAN) += fsl_dpaa_mac.o
-fsl_fman-objs := fman_muram.o fman.o fman_sp.o fman_keygen.o
-fsl_fman_port-objs := fman_port.o
-fsl_mac-objs:= mac.o fman_dtsec.o fman_memac.o fman_tgec.o
+fsl_dpaa_fman-objs := fman_muram.o fman.o fman_sp.o fman_keygen.o
+fsl_dpaa_fman_port-objs := fman_port.o
+fsl_dpaa_mac-objs:= mac.o fman_dtsec.o fman_memac.o fman_tgec.o
--
2.1.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox