* Re: [PATCH REPOST 1/2] can: flexcan: fix deadlock when using self wakeup
From: Sean Nyekjaer @ 2019-08-20 10:25 UTC (permalink / raw)
To: Joakim Zhang, mkl@pengutronix.de, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org, dl-linux-imx,
Martin Hundebøll
In-Reply-To: <20190816081749.19300-2-qiangqing.zhang@nxp.com>
On 16/08/2019 10.20, Joakim Zhang wrote:
> As reproted by Sean Nyekjaer below:
> When suspending, when there is still can traffic on the interfaces the
> flexcan immediately wakes the platform again. As it should :-). But it
> throws this error msg:
> [ 3169.378661] PM: noirq suspend of devices failed
>
> On the way down to suspend the interface that throws the error message does
> call flexcan_suspend but fails to call flexcan_noirq_suspend. That means the
> flexcan_enter_stop_mode is called, but on the way out of suspend the driver
> only calls flexcan_resume and skips flexcan_noirq_resume, thus it doesn't call
> flexcan_exit_stop_mode. This leaves the flexcan in stop mode, and with the
> current driver it can't recover from this even with a soft reboot, it requires
> a hard reboot.
>
> The best way to exit stop mode is in Wake Up interrupt context, and then
> suspend() and resume() functions can be symmetric. However, stop mode
> request and ack will be controlled by SCU(System Control Unit) firmware(manage
> clock,power,stop mode, etc. by Cortex-M4 core) in coming i.MX8(QM/QXP). And SCU
> firmware interface can't be available in interrupt context.
>
> For compatibillity, the wake up mechanism can't be symmetric, so we need
> in_stop_mode hack.
>
> Fixes: de3578c198c6 ("can: flexcan: add self wakeup support")
> Reported-by: Sean Nyekjaer <sean@geanix.com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
>
Unfortunatly it's still possible to reproduce the deadlock with this
patch...
[ 689.921717] flexcan: probe of 2094000.flexcan failed with error -110
My test setup:
PC with CAN-USB dongle connected to can0 and can1.
PC:
$ while true; do cansend can0 '123#DEADBEEF'; done
iMX6ull:
root@iwg26:~# systemctl suspend
[ 365.858054] systemd[1]: Reached target Sleep.
root@iwg26:~# [ 365.939826] systemd[1]: Starting Suspend...
[ 366.115839] systemd-sleep[248]: Suspending system...
[ 366.517949] dpm_run_callback(): platform_pm_suspend+0x0/0x5c returns -110
[ 366.518249] PM: Device 2094000.flexcan failed to suspend: error -110
[ 366.518406] PM: Some devices failed to suspend, or early wake event
detected
[ 366.732162] dpm_run_callback(): platform_pm_suspend+0x0/0x5c returns -110
[ 366.732285] PM: Device 2090000.flexcan failed to suspend: error -110
[ 366.732330] PM: Some devices failed to suspend, or early wake event
detected
[ 366.890637] systemd-sleep[248]: System resumed.
[ 366.923062] systemd[1]: Started Suspend.
[ 366.942819] systemd[1]: sleep.target: Unit not needed anymore. Stopping.
[ 366.954791] systemd[1]: Stopped target Sleep.
[ 366.962402] systemd[1]: Reached target Suspend.
[ 366.977546] systemd-logind[135]: Operation 'sleep' finished.
[ 366.979194] systemd[1]: suspend.target: Unit not needed anymore.
Stopping.
[ 366.993831] systemd[1]: Stopped target Suspend.
[ 367.139972] systemd-networkd[220]: usb0: Lost carrier
[ 367.294077] systemd-networkd[220]: usb0: Gained carrier
root@iwg26:~# candump can0 | head -n 2
can0 123 [4] DE AD BE EF
can0 123 [4] DE AD BE EF
root@iwg26:~# candump can1 | head -n 2
can1 123 [4] DE AD BE EF
can1 123 [4] DE AD BE EF
root@iwg26:~# systemctl suspend
root@iwg26:~# [ 385.106658] systemd[1]: Reached target Sleep.
[ 385.147602] systemd[1]: Starting Suspend...
[ 385.246421] systemd-sleep[260]: Suspending system...
[ 385.634733] dpm_run_callback(): platform_pm_suspend+0x0/0x5c returns -110
[ 385.634855] PM: Device 2090000.flexcan failed to suspend: error -110
[ 385.634897] PM: Some devices failed to suspend, or early wake event
detected
[ 385.856251] PM: noirq suspend of devices failed
[ 385.998364] systemd-sleep[260]: System resumed.
[ 386.023390] systemd[1]: Started Suspend.
[ 386.031570] systemd[1]: sleep.target: Unit not needed anymore. Stopping.
[ 386.055886] systemd[1]: Stopped target Sleep.
[ 386.061430] systemd[1]: Reached target Suspend.
[ 386.066142] systemd[1]: suspend.target: Unit not needed anymore.
Stopping.
[ 386.112575] systemd-networkd[220]: usb0: Lost carrier
[ 386.116797] systemd-logind[135]: Operation 'sleep' finished.
[ 386.146161] systemd[1]: Stopped target Suspend.
[ 386.260866] systemd-networkd[220]: usb0: Gained carrier
root@iwg26:~# candump can0 | head -n 2
can0 123 [4] DE AD BE EF
can0 123 [4] DE AD BE EF
root@iwg26:~# candump can1 | head -n 2
can1 123 [4] DE AD BE EF
can1 123 [4] DE AD BE EF
root@iwg26:~# systemctl suspend
[ 396.919303] systemd[1]: Reached target Sleep.
root@iwg26:~# [ 396.964722] systemd[1]: Starting Suspend...
[ 397.067336] systemd-sleep[268]: Suspending system...
[ 397.574571] PM: noirq suspend of devices failed
[ 397.834731] PM: noirq suspend of devices failed
[ 397.807996] systemd-networkd[220]: usb0: Lost carrier
[ 398.156295] dpm_run_callback(): platform_pm_suspend+0x0/0x5c returns -110
[ 398.156339] PM: Device 2094000.flexcan failed to suspend: error -110
[ 398.156509] PM: Some devices failed to suspend, or early wake event
detected
[ 398.053555] systemd-sleep[268]: Failed to write /sys/power/state:
Device or resource busy
[ 398.074751] systemd[1]: systemd-suspend.service: Main process exited,
code=exited, status=1/FAILURE
[ 398.076779] systemd[1]: systemd-suspend.service: Failed with result
'exit-code'.
[ 398.109255] systemd[1]: Failed to start Suspend.
[ 398.118704] systemd[1]: Dependency failed for Suspend.
[ 398.136283] systemd-logind[135]: Operation 'sleep' finished.
[ 398.137770] systemd[1]: suspend.target: Job suspend.target/start
failed with result 'dependency'.
[ 398.139105] systemd[1]: sleep.target: Unit not needed anymore. Stopping.
[ 398.167590] systemd[1]: Stopped target Sleep.
[ 398.201558] systemd-networkd[220]: usb0: Gained carrier
root@iwg26:~# candump can0 | head -n 2
can0 123 [4] DE AD BE EF
can0 123 [4] DE AD BE EF
root@iwg26:~# candump can1 | head -n 2
nothing on can1 anymore :-(
root@iwg26:~# rmmod flexcan
[ 622.884746] systemd-networkd[220]: can1: Lost carrier
[ 623.046766] systemd-networkd[220]: can0: Lost carrier
root@iwg26:~# insmod /mnt/flexcan.ko
[ 628.323981] flexcan 2094000.flexcan: registering netdev failed
and can1 fails to register with:
[ 628.347485] flexcan: probe of 2094000.flexcan failed with error -110
/Sean
^ permalink raw reply
* Capturing syscall arguments: `kprobe`, `kretprobe` or `tracepoint` or `raw_tracepoint` type programs
From: Tahsin Rahman @ 2019-08-20 10:26 UTC (permalink / raw)
To: netdev
Hi,
I'm new to ebpf. I want to write an ebpf program that can trace the
syscall arguments and return values.
According to my research, I can do this using `kprobe`, `kretprobe` or
`tracepoint` or `raw_tracepoint` type of bpf programs.
- What factors should I consider when choosing one type of program over another?
- Is the main difference among them is performance benefits? I'd be
great help if one can point me to any documentations about the
performance difference among different types of ebpf programs.
- How can I benchmark these programs?
Thanks!
^ permalink raw reply
* Re: [PATCH net-next 6/6] net: dsa: tag_8021q: Restore bridge pvid when enabling vlan_filtering
From: Vladimir Oltean @ 2019-08-20 10:28 UTC (permalink / raw)
To: Florian Fainelli
Cc: Vivien Didelot, Andrew Lunn, Ido Schimmel, Roopa Prabhu, nikolay,
David S. Miller, netdev
In-Reply-To: <bf0c064e-6304-ba31-8f45-3a6226ed8939@gmail.com>
Hi Florian,
On Tue, 20 Aug 2019 at 06:33, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 8/19/2019 5:00 PM, Vladimir Oltean wrote:
> > The bridge core assumes that enabling/disabling vlan_filtering will
> > translate into the simple toggling of a flag for switchdev drivers.
> >
> > That is clearly not the case for sja1105, which alters the VLAN table
> > and the pvids in order to obtain port separation in standalone mode.
> >
> > So, since the bridge will not call any vlan operation through switchdev
> > after enabling vlan_filtering, we need to ensure we're in a functional
> > state ourselves.
> >
> > Hence read the pvid that the bridge is aware of, and program that into
> > our ports.
>
> That is arguably applicable with DSA at large and not just specifically
> for tag_8021q.c no? Is there a reason why you are not seeking to solve
> this on a more global scale?
>
Perhaps because I don't have a good feel for what are other DSA
drivers' struggles with restoring the pvid, even after re-reading the
"What to do when a bridge port gets its pvid deleted?" thread.
I understand b53 has a similar need, and for that purpose maybe you
can EXPORT_SYMBOL_GPL(dsa_port_restore_pvid) and use it, but
otherwise, what is the more global scale?
> >
> > Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
> > ---
> > net/dsa/tag_8021q.c | 32 +++++++++++++++++++++++++++++++-
> > 1 file changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
> > index 67a1bc635a7b..6423beb1efcd 100644
> > --- a/net/dsa/tag_8021q.c
> > +++ b/net/dsa/tag_8021q.c
> > @@ -93,6 +93,33 @@ int dsa_8021q_rx_source_port(u16 vid)
> > }
> > EXPORT_SYMBOL_GPL(dsa_8021q_rx_source_port);
> >
> > +static int dsa_port_restore_pvid(struct dsa_switch *ds, int port)
> > +{
> > + struct bridge_vlan_info vinfo;
> > + struct net_device *slave;
> > + u16 pvid;
> > + int err;
> > +
> > + if (!dsa_is_user_port(ds, port))
> > + return 0;
> > +
> > + slave = ds->ports[port].slave;
> > +
> > + err = br_vlan_get_pvid(slave, &pvid);
> > + if (err < 0) {
> > + dev_err(ds->dev, "Couldn't determine bridge PVID\n");
> > + return err;
> > + }
> > +
> > + err = br_vlan_get_info(slave, pvid, &vinfo);
> > + if (err < 0) {
> > + dev_err(ds->dev, "Couldn't determine PVID attributes\n");
> > + return err;
> > + }
> > +
> > + return dsa_port_vid_add(&ds->ports[port], pvid, vinfo.flags);
> > +}
> > +
> > /* RX VLAN tagging (left) and TX VLAN tagging (right) setup shown for a single
> > * front-panel switch port (here swp0).
> > *
> > @@ -223,7 +250,10 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
> > return err;
> > }
> >
> > - return 0;
> > + if (!enabled)
> > + err = dsa_port_restore_pvid(ds, port);
> > +
> > + return err;
> > }
> > EXPORT_SYMBOL_GPL(dsa_port_setup_8021q_tagging);
> >
> >
>
> --
> Florian
Regards,
-Vladimir
^ permalink raw reply
* RE: Help needed - Kernel lockup while running ipsec
From: Vakul Garg @ 2019-08-20 10:38 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev@vger.kernel.org
In-Reply-To: <DB7PR04MB46204E237BB1E495FC799E588BAB0@DB7PR04MB4620.eurprd04.prod.outlook.com>
>
> > -----Original Message-----
> > From: Florian Westphal <fw@strlen.de>
> > Sent: Tuesday, August 20, 2019 3:08 PM
> > To: Vakul Garg <vakul.garg@nxp.com>
> > Cc: Florian Westphal <fw@strlen.de>; netdev@vger.kernel.org
> > Subject: Re: Help needed - Kernel lockup while running ipsec
> >
> > Vakul Garg <vakul.garg@nxp.com> wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Florian Westphal <fw@strlen.de>
> > > > Sent: Tuesday, August 20, 2019 2:53 PM
> > > > To: Vakul Garg <vakul.garg@nxp.com>
> > > > Cc: Florian Westphal <fw@strlen.de>; netdev@vger.kernel.org
> > > > Subject: Re: Help needed - Kernel lockup while running ipsec
> > > >
> > > > Vakul Garg <vakul.garg@nxp.com> wrote:
> > > > > > > With kernel 4.14.122, I am getting a kernel softlockup while
> > > > > > > running single
> > > > > > static ipsec tunnel.
> > > > > > > The problem reproduces mostly after running 8-10 hours of
> > > > > > > ipsec encap
> > > > > > test (on my dual core arm board).
> > > > > > >
> > > > > > > I found that in function xfrm_policy_lookup_bytype(), the
> > > > > > > policy in variable
> > > > > > 'ret' shows refcnt=0 under problem situation.
> > > > > > > This creates an infinite loop in xfrm_policy_lookup_bytype()
> > > > > > > and hence the
> > > > > > lockup.
> > > > > > >
> > > > > > > Can some body please provide me pointers about 'refcnt'?
> > > > > > > Is it legitimate for 'refcnt' to become '0'? Under what
> > > > > > > condition can it
> > > > > > become '0'?
> > > > > >
> > > > > > Yes, when policy is destroyed and the last user calls
> > > > > > xfrm_pol_put() which will invoke call_rcu to free the structure.
> > > > >
> > > > > It seems that policy reference count never gets decremented during
> > > > > packet
> > > > ipsec encap.
> > > > > It is getting incremented for every frame that hits the policy.
> > > > > In setkey -DP output, I see refcnt to be wrapping around after '0'.
> > > >
> > > > Thats a bug. Does this affect 4.14 only or does this happen on
> > > > current tree as well?
> > >
> > > I am yet to try it on 4.19.
> > > Can you help me with the right fix? Which part of code should it get
> > decremented?
> > > I am not conversant with xfrm code.
> >
> > Normally policy reference counts get decremented when the skb is free'd,
> via
> > dst destruction (xfrm_dst_destroy()).
> >
> > Do you see a dst leak as well?
>
> Can you please guide me how to detect it?
>
> (I am checking refcount on recent kernel and will let you know.)
Policy refcount is decreasing properly on 4.19.
Same should be on the latest kernel too.
^ permalink raw reply
* [PATCH net-next 0/2] netfilter: payload mangling offload support
From: Pablo Neira Ayuso @ 2019-08-20 10:48 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev, jakub.kicinski, jiri, vladbu
Hi,
This patchset adds payload mangling offload support for Netfilter:
1) Adapt existing drivers to allow for mangling up to four 32-bit words
with one single flow_rule action. Hence, once single action can be
used to mangle an IPv6 address.
2) Add support for netfilter packet mangling.
Please, apply.
Pablo Neira Ayuso (2):
net: flow_offload: mangle 128-bit packet field with one action
netfilter: nft_payload: packet mangling offload support
.../net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 44 ++++++++----
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 50 +++++++++----
drivers/net/ethernet/netronome/nfp/flower/action.c | 69 ++++++++++++------
include/net/flow_offload.h | 9 ++-
net/netfilter/nft_payload.c | 82 ++++++++++++++++++++++
net/sched/cls_api.c | 7 +-
6 files changed, 207 insertions(+), 54 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH net-next 1/2] net: flow_offload: mangle 128-bit packet field with one action
From: Pablo Neira Ayuso @ 2019-08-20 10:52 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev, jakub.kicinski, jiri, vladbu
The existing infrastructure needs the front-end to generate up to four
actions (one for each 32-bit word) to mangle an IPv6 address. This patch
allows you to mangle fields than are longer than 4-bytes with one single
action. Drivers have been adapted to this new representation following a
simple approach, that is, iterate over the array of words and configure
the hardware IR to make the packet mangling. FLOW_ACTION_MANGLE_MAX_WORDS
defines the maximum number of words from one given offset (currently 4
words).
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
.../net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 44 ++++++++++----
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 50 +++++++++++-----
drivers/net/ethernet/netronome/nfp/flower/action.c | 69 ++++++++++++++--------
include/net/flow_offload.h | 9 ++-
net/sched/cls_api.c | 7 ++-
5 files changed, 125 insertions(+), 54 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
index e447976bdd3e..6a961e29a904 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
@@ -428,13 +428,17 @@ static void cxgb4_process_flow_actions(struct net_device *in,
case FLOW_ACTION_MANGLE: {
u32 mask, val, offset;
u8 htype;
+ int i;
htype = act->mangle.htype;
- mask = act->mangle.mask;
- val = act->mangle.val;
offset = act->mangle.offset;
- process_pedit_field(fs, val, mask, offset, htype);
+ for (i = 0; i < act->mangle.words; i++) {
+ mask = act->mangle.data[i].mask;
+ val = act->mangle.data[i].val;
+ process_pedit_field(fs, val, mask, offset, htype);
+ offset += sizeof(u32);
+ }
}
break;
default:
@@ -456,16 +460,9 @@ static bool valid_l4_mask(u32 mask)
return hi && lo ? false : true;
}
-static bool valid_pedit_action(struct net_device *dev,
- const struct flow_action_entry *act)
+static bool __valid_pedit_action(struct net_device *dev, u8 htype,
+ __be32 mask, __be32 offset)
{
- u32 mask, offset;
- u8 htype;
-
- htype = act->mangle.htype;
- mask = act->mangle.mask;
- offset = act->mangle.offset;
-
switch (htype) {
case FLOW_ACT_MANGLE_HDR_TYPE_ETH:
switch (offset) {
@@ -541,6 +538,29 @@ static bool valid_pedit_action(struct net_device *dev,
netdev_err(dev, "%s: Unsupported pedit type\n", __func__);
return false;
}
+
+ return true;
+}
+
+static bool valid_pedit_action(struct net_device *dev,
+ const struct flow_action_entry *act)
+{
+ u32 mask, offset;
+ u8 htype;
+ int i;
+
+ htype = act->mangle.htype;
+ offset = act->mangle.offset;
+
+ for (i = 0; i < act->mangle.words; i++) {
+ mask = act->mangle.data[i].mask;
+
+ if (!__valid_pedit_action(dev, htype, mask, offset))
+ return false;
+
+ offset += sizeof(u32);
+ }
+
return true;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index c57f7533a6d0..bb24616ee27f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2411,6 +2411,7 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
int err = -EOPNOTSUPP;
u32 mask, val, offset;
u8 htype;
+ int i;
htype = act->mangle.htype;
err = -EOPNOTSUPP; /* can't be all optimistic */
@@ -2426,15 +2427,19 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
goto out_err;
}
- mask = act->mangle.mask;
- val = act->mangle.val;
offset = act->mangle.offset;
- err = set_pedit_val(htype, ~mask, val, offset, &hdrs[cmd]);
- if (err)
- goto out_err;
+ for (i = 0; i < act->mangle.words; i++) {
+ val = act->mangle.data[i].val;
+ mask = act->mangle.data[i].mask;
- hdrs[cmd].pedits++;
+ err = set_pedit_val(htype, ~mask, val, offset, &hdrs[cmd]);
+ if (err)
+ goto out_err;
+
+ offset += sizeof(u32);
+ hdrs[cmd].pedits++;
+ }
return 0;
out_err:
@@ -2523,14 +2528,8 @@ struct ipv6_hoplimit_word {
__u8 hop_limit;
};
-static bool is_action_keys_supported(const struct flow_action_entry *act)
+static bool __is_action_keys_supported(u8 htype, u32 offset, u32 mask)
{
- u32 mask, offset;
- u8 htype;
-
- htype = act->mangle.htype;
- offset = act->mangle.offset;
- mask = ~act->mangle.mask;
/* For IPv4 & IPv6 header check 4 byte word,
* to determine that modified fields
* are NOT ttl & hop_limit only.
@@ -2557,6 +2556,26 @@ static bool is_action_keys_supported(const struct flow_action_entry *act)
return false;
}
+static bool is_action_keys_supported(const struct flow_action_entry *act)
+{
+ u32 mask, offset;
+ u8 htype;
+ int i;
+
+ htype = act->mangle.htype;
+ offset = act->mangle.offset;
+
+ for (i = 0; i < act->mangle.words; i++) {
+ mask = ~act->mangle.data[i].mask;
+ if (!__is_action_keys_supported(htype, offset, mask))
+ return false;
+
+ offset += sizeof(u32);
+ }
+
+ return true;
+}
+
static bool modify_header_match_supported(struct mlx5_flow_spec *spec,
struct flow_action *flow_action,
u32 actions,
@@ -2654,8 +2673,9 @@ static int add_vlan_rewrite_action(struct mlx5e_priv *priv, int namespace,
.id = FLOW_ACTION_MANGLE,
.mangle.htype = FLOW_ACT_MANGLE_HDR_TYPE_ETH,
.mangle.offset = offsetof(struct vlan_ethhdr, h_vlan_TCI),
- .mangle.mask = ~(u32)be16_to_cpu(*(__be16 *)&mask16),
- .mangle.val = (u32)be16_to_cpu(*(__be16 *)&val16),
+ .mangle.data[0].mask = ~(u32)be16_to_cpu(*(__be16 *)&mask16),
+ .mangle.data[0].val = (u32)be16_to_cpu(*(__be16 *)&val16),
+ .mangle.words = 1,
};
u8 match_prio_mask, match_prio_val;
void *headers_c, *headers_v;
diff --git a/drivers/net/ethernet/netronome/nfp/flower/action.c b/drivers/net/ethernet/netronome/nfp/flower/action.c
index 1b019fdfcd97..15bace2354dc 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/action.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/action.c
@@ -485,7 +485,7 @@ static void nfp_fl_set_helper32(u32 value, u32 mask, u8 *p_exact, u8 *p_mask)
}
static int
-nfp_fl_set_eth(const struct flow_action_entry *act, u32 off,
+nfp_fl_set_eth(const struct flow_action_entry *act, u32 idx, u32 off,
struct nfp_fl_set_eth *set_eth, struct netlink_ext_ack *extack)
{
u32 exact, mask;
@@ -495,8 +495,8 @@ nfp_fl_set_eth(const struct flow_action_entry *act, u32 off,
return -EOPNOTSUPP;
}
- mask = ~act->mangle.mask;
- exact = act->mangle.val;
+ mask = ~act->mangle.data[idx].mask;
+ exact = act->mangle.data[idx].val;
if (exact & ~mask) {
NL_SET_ERR_MSG_MOD(extack, "unsupported offload: invalid pedit ethernet action");
@@ -520,7 +520,7 @@ struct ipv4_ttl_word {
};
static int
-nfp_fl_set_ip4(const struct flow_action_entry *act, u32 off,
+nfp_fl_set_ip4(const struct flow_action_entry *act, u32 idx, u32 off,
struct nfp_fl_set_ip4_addrs *set_ip_addr,
struct nfp_fl_set_ip4_ttl_tos *set_ip_ttl_tos,
struct netlink_ext_ack *extack)
@@ -532,8 +532,8 @@ nfp_fl_set_ip4(const struct flow_action_entry *act, u32 off,
__be32 exact, mask;
/* We are expecting tcf_pedit to return a big endian value */
- mask = (__force __be32)~act->mangle.mask;
- exact = (__force __be32)act->mangle.val;
+ mask = (__force __be32)~act->mangle.data[idx].mask;
+ exact = (__force __be32)act->mangle.data[idx].val;
if (exact & ~mask) {
NL_SET_ERR_MSG_MOD(extack, "unsupported offload: invalid pedit IPv4 action");
@@ -662,7 +662,7 @@ nfp_fl_set_ip6_hop_limit_flow_label(u32 off, __be32 exact, __be32 mask,
}
static int
-nfp_fl_set_ip6(const struct flow_action_entry *act, u32 off,
+nfp_fl_set_ip6(const struct flow_action_entry *act, u32 idx, u32 off,
struct nfp_fl_set_ipv6_addr *ip_dst,
struct nfp_fl_set_ipv6_addr *ip_src,
struct nfp_fl_set_ipv6_tc_hl_fl *ip_hl_fl,
@@ -673,8 +673,8 @@ nfp_fl_set_ip6(const struct flow_action_entry *act, u32 off,
u8 word;
/* We are expecting tcf_pedit to return a big endian value */
- mask = (__force __be32)~act->mangle.mask;
- exact = (__force __be32)act->mangle.val;
+ mask = (__force __be32)~act->mangle.data[idx].mask;
+ exact = (__force __be32)act->mangle.data[idx].val;
if (exact & ~mask) {
NL_SET_ERR_MSG_MOD(extack, "unsupported offload: invalid pedit IPv6 action");
@@ -702,7 +702,7 @@ nfp_fl_set_ip6(const struct flow_action_entry *act, u32 off,
}
static int
-nfp_fl_set_tport(const struct flow_action_entry *act, u32 off,
+nfp_fl_set_tport(const struct flow_action_entry *act, u32 idx, u32 off,
struct nfp_fl_set_tport *set_tport, int opcode,
struct netlink_ext_ack *extack)
{
@@ -713,8 +713,8 @@ nfp_fl_set_tport(const struct flow_action_entry *act, u32 off,
return -EOPNOTSUPP;
}
- mask = ~act->mangle.mask;
- exact = act->mangle.val;
+ mask = ~act->mangle.data[idx].mask;
+ exact = act->mangle.data[idx].val;
if (exact & ~mask) {
NL_SET_ERR_MSG_MOD(extack, "unsupported offload: invalid pedit L4 action");
@@ -860,32 +860,31 @@ nfp_fl_commit_mangle(struct flow_cls_offload *flow, char *nfp_action,
}
static int
-nfp_fl_pedit(const struct flow_action_entry *act,
- struct flow_cls_offload *flow, char *nfp_action, int *a_len,
- u32 *csum_updated, struct nfp_flower_pedit_acts *set_act,
- struct netlink_ext_ack *extack)
+__nfp_fl_pedit(const struct flow_action_entry *act, u32 idx, u32 offset,
+ struct flow_cls_offload *flow, char *nfp_action, int *a_len,
+ u32 *csum_updated, struct nfp_flower_pedit_acts *set_act,
+ struct netlink_ext_ack *extack)
{
enum flow_action_mangle_base htype;
- u32 offset;
htype = act->mangle.htype;
- offset = act->mangle.offset;
switch (htype) {
case TCA_PEDIT_KEY_EX_HDR_TYPE_ETH:
- return nfp_fl_set_eth(act, offset, &set_act->set_eth, extack);
+ return nfp_fl_set_eth(act, idx, offset, &set_act->set_eth,
+ extack);
case TCA_PEDIT_KEY_EX_HDR_TYPE_IP4:
- return nfp_fl_set_ip4(act, offset, &set_act->set_ip_addr,
+ return nfp_fl_set_ip4(act, idx, offset, &set_act->set_ip_addr,
&set_act->set_ip_ttl_tos, extack);
case TCA_PEDIT_KEY_EX_HDR_TYPE_IP6:
- return nfp_fl_set_ip6(act, offset, &set_act->set_ip6_dst,
+ return nfp_fl_set_ip6(act, idx, offset, &set_act->set_ip6_dst,
&set_act->set_ip6_src,
&set_act->set_ip6_tc_hl_fl, extack);
case TCA_PEDIT_KEY_EX_HDR_TYPE_TCP:
- return nfp_fl_set_tport(act, offset, &set_act->set_tport,
+ return nfp_fl_set_tport(act, idx, offset, &set_act->set_tport,
NFP_FL_ACTION_OPCODE_SET_TCP, extack);
case TCA_PEDIT_KEY_EX_HDR_TYPE_UDP:
- return nfp_fl_set_tport(act, offset, &set_act->set_tport,
+ return nfp_fl_set_tport(act, idx, offset, &set_act->set_tport,
NFP_FL_ACTION_OPCODE_SET_UDP, extack);
default:
NL_SET_ERR_MSG_MOD(extack, "unsupported offload: pedit on unsupported header");
@@ -894,6 +893,30 @@ nfp_fl_pedit(const struct flow_action_entry *act,
}
static int
+nfp_fl_pedit(const struct flow_action_entry *act,
+ struct flow_cls_offload *flow, char *nfp_action, int *a_len,
+ u32 *csum_updated, struct nfp_flower_pedit_acts *set_act,
+ struct netlink_ext_ack *extack)
+{
+ u32 offset, idx;
+ int err;
+
+ offset = act->mangle.offset;
+
+ for (idx = 0; idx < act->mangle.words; idx++) {
+ err = __nfp_fl_pedit(act, idx, offset, flow,
+ nfp_action, a_len, csum_updated, set_act,
+ extack);
+ if (err < 0)
+ return err;
+
+ offset += sizeof(u32);
+ }
+
+ return 0;
+}
+
+static int
nfp_flower_output_action(struct nfp_app *app,
const struct flow_action_entry *act,
struct nfp_fl_payload *nfp_fl, int *a_len,
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index e8069b6c474c..d3fc6b7dcd6a 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -153,6 +153,8 @@ enum flow_action_mangle_base {
FLOW_ACT_MANGLE_HDR_TYPE_UDP,
};
+#define FLOW_ACTION_MANGLE_MAX_WORDS 4
+
struct flow_action_entry {
enum flow_action_id id;
union {
@@ -166,8 +168,11 @@ struct flow_action_entry {
struct { /* FLOW_ACTION_PACKET_EDIT */
enum flow_action_mangle_base htype;
u32 offset;
- u32 mask;
- u32 val;
+ struct {
+ u32 mask;
+ u32 val;
+ } data[FLOW_ACTION_MANGLE_MAX_WORDS];
+ u32 words;
} mangle;
const struct ip_tunnel_info *tunnel; /* FLOW_ACTION_TUNNEL_ENCAP */
u32 csum_flags; /* FLOW_ACTION_CSUM */
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index e0d8b456e9f5..041cd4000389 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -3077,9 +3077,12 @@ int tc_setup_flow_action(struct flow_action *flow_action,
goto err_out;
}
entry->mangle.htype = tcf_pedit_htype(act, k);
- entry->mangle.mask = tcf_pedit_mask(act, k);
- entry->mangle.val = tcf_pedit_val(act, k);
+ entry->mangle.data[0].mask =
+ tcf_pedit_mask(act, k);
+ entry->mangle.data[0].val =
+ tcf_pedit_val(act, k);
entry->mangle.offset = tcf_pedit_offset(act, k);
+ entry->mangle.words = 1;
entry = &flow_action->entries[++j];
}
} else if (is_tcf_csum(act)) {
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 2/2] netfilter: nft_payload: packet mangling offload support
From: Pablo Neira Ayuso @ 2019-08-20 10:52 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev, jakub.kicinski, jiri, vladbu
In-Reply-To: <20190820105225.13943-1-pablo@netfilter.org>
This patch allows for mangling packet fields using hardware offload
infrastructure.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nft_payload.c | 82 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 82 insertions(+)
diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
index 22a80eb60222..d758c8900835 100644
--- a/net/netfilter/nft_payload.c
+++ b/net/netfilter/nft_payload.c
@@ -562,12 +562,94 @@ static int nft_payload_set_dump(struct sk_buff *skb, const struct nft_expr *expr
return -1;
}
+static int nft_payload_offload_set_nh(struct nft_offload_ctx *ctx,
+ struct nft_flow_rule *flow,
+ const struct nft_payload_set *priv)
+{
+ int type = FLOW_ACT_MANGLE_UNSPEC;
+
+ switch (ctx->dep.l3num) {
+ case htons(ETH_P_IP):
+ type = FLOW_ACT_MANGLE_HDR_TYPE_IP4;
+ break;
+ case htons(ETH_P_IPV6):
+ type = FLOW_ACT_MANGLE_HDR_TYPE_IP6;
+ break;
+ }
+
+ return type;
+}
+
+static int nft_payload_offload_set_th(struct nft_offload_ctx *ctx,
+ struct nft_flow_rule *flow,
+ const struct nft_payload_set *priv)
+{
+ int type = FLOW_ACT_MANGLE_UNSPEC;
+
+ switch (ctx->dep.protonum) {
+ case IPPROTO_TCP:
+ type = FLOW_ACT_MANGLE_HDR_TYPE_TCP;
+ break;
+ case IPPROTO_UDP:
+ type = FLOW_ACT_MANGLE_HDR_TYPE_UDP;
+ break;
+ }
+
+ return type;
+}
+
+static inline u32 nft_payload_mask(int len)
+{
+ return (1 << (len * BITS_PER_BYTE)) - 1;
+}
+
+static int nft_payload_set_offload(struct nft_offload_ctx *ctx,
+ struct nft_flow_rule *flow,
+ const struct nft_expr *expr)
+{
+ const struct nft_payload_set *priv = nft_expr_priv(expr);
+ struct nft_offload_reg *sreg = &ctx->regs[priv->sreg];
+ int type = FLOW_ACT_MANGLE_UNSPEC;
+ struct flow_action_entry *entry;
+ u32 words;
+ int i;
+
+ switch (priv->base) {
+ case NFT_PAYLOAD_LL_HEADER:
+ type = FLOW_ACT_MANGLE_HDR_TYPE_ETH;
+ break;
+ case NFT_PAYLOAD_NETWORK_HEADER:
+ type = nft_payload_offload_set_nh(ctx, flow, priv);
+ break;
+ case NFT_PAYLOAD_TRANSPORT_HEADER:
+ type = nft_payload_offload_set_th(ctx, flow, priv);
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ break;
+ }
+ words = round_up(priv->len, sizeof(u32)) / sizeof(u32);
+
+ entry = &flow->rule->action.entries[ctx->num_actions++];
+ entry->mangle.htype = type;
+ entry->mangle.offset = priv->offset;
+ for (i = 0; i < words; i++) {
+ entry->mangle.data[i].mask =
+ ~htonl(nft_payload_mask(priv->len));
+ entry->mangle.data[i].val = sreg->data.data[i];
+ }
+ entry->mangle.words = words;
+
+ return type != FLOW_ACT_MANGLE_UNSPEC ? 0 : -EOPNOTSUPP;
+}
+
static const struct nft_expr_ops nft_payload_set_ops = {
.type = &nft_payload_type,
.size = NFT_EXPR_SIZE(sizeof(struct nft_payload_set)),
.eval = nft_payload_set_eval,
.init = nft_payload_set_init,
.dump = nft_payload_set_dump,
+ .offload = nft_payload_set_offload,
};
static const struct nft_expr_ops *
--
2.11.0
^ permalink raw reply related
* RE: [PATCH REPOST 1/2] can: flexcan: fix deadlock when using self wakeup
From: Joakim Zhang @ 2019-08-20 11:24 UTC (permalink / raw)
To: Sean Nyekjaer, mkl@pengutronix.de, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org, dl-linux-imx,
Martin Hundebøll
In-Reply-To: <dd8f5269-8403-702b-b054-e031423ffc73@geanix.com>
> -----Original Message-----
> From: Sean Nyekjaer <sean@geanix.com>
> Sent: 2019年8月20日 18:25
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; mkl@pengutronix.de;
> linux-can@vger.kernel.org
> Cc: wg@grandegger.com; netdev@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>; Martin Hundebøll <martin@geanix.com>
> Subject: Re: [PATCH REPOST 1/2] can: flexcan: fix deadlock when using self
> wakeup
>
>
>
> On 16/08/2019 10.20, Joakim Zhang wrote:
> > As reproted by Sean Nyekjaer below:
> > When suspending, when there is still can traffic on the interfaces the
> > flexcan immediately wakes the platform again. As it should :-). But it
> > throws this error msg:
> > [ 3169.378661] PM: noirq suspend of devices failed
> >
> > On the way down to suspend the interface that throws the error message
> > does call flexcan_suspend but fails to call flexcan_noirq_suspend.
> > That means the flexcan_enter_stop_mode is called, but on the way out
> > of suspend the driver only calls flexcan_resume and skips
> > flexcan_noirq_resume, thus it doesn't call flexcan_exit_stop_mode.
> > This leaves the flexcan in stop mode, and with the current driver it
> > can't recover from this even with a soft reboot, it requires a hard reboot.
> >
> > The best way to exit stop mode is in Wake Up interrupt context, and
> > then
> > suspend() and resume() functions can be symmetric. However, stop mode
> > request and ack will be controlled by SCU(System Control Unit)
> > firmware(manage clock,power,stop mode, etc. by Cortex-M4 core) in
> > coming i.MX8(QM/QXP). And SCU firmware interface can't be available in
> interrupt context.
> >
> > For compatibillity, the wake up mechanism can't be symmetric, so we
> > need in_stop_mode hack.
> >
> > Fixes: de3578c198c6 ("can: flexcan: add self wakeup support")
> > Reported-by: Sean Nyekjaer <sean@geanix.com>
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> >
>
> Unfortunatly it's still possible to reproduce the deadlock with this patch...
>
> [ 689.921717] flexcan: probe of 2094000.flexcan failed with error -110
>
> My test setup:
> PC with CAN-USB dongle connected to can0 and can1.
>
> PC:
> $ while true; do cansend can0 '123#DEADBEEF'; done
>
> iMX6ull:
> root@iwg26:~# systemctl suspend
>
>
> [ 365.858054] systemd[1]: Reached target Sleep.
> root@iwg26:~# [ 365.939826] systemd[1]: Starting Suspend...
> [ 366.115839] systemd-sleep[248]: Suspending system...
> [ 366.517949] dpm_run_callback(): platform_pm_suspend+0x0/0x5c returns
> -110 [ 366.518249] PM: Device 2094000.flexcan failed to suspend: error -110
> [ 366.518406] PM: Some devices failed to suspend, or early wake event
> detected [ 366.732162] dpm_run_callback():
> platform_pm_suspend+0x0/0x5c returns -110 [ 366.732285] PM: Device
> 2090000.flexcan failed to suspend: error -110 [ 366.732330] PM: Some
> devices failed to suspend, or early wake event detected [ 366.890637]
> systemd-sleep[248]: System resumed.
CAN1, CAN0 suspended failed, then CAN0, CAN1 resumed back, so CAN0/CAN1 can work fine.
> [ 366.923062] systemd[1]: Started Suspend.
> [ 366.942819] systemd[1]: sleep.target: Unit not needed anymore. Stopping.
> [ 366.954791] systemd[1]: Stopped target Sleep.
> [ 366.962402] systemd[1]: Reached target Suspend.
> [ 366.977546] systemd-logind[135]: Operation 'sleep' finished.
> [ 366.979194] systemd[1]: suspend.target: Unit not needed anymore.
> Stopping.
> [ 366.993831] systemd[1]: Stopped target Suspend.
> [ 367.139972] systemd-networkd[220]: usb0: Lost carrier [ 367.294077]
> systemd-networkd[220]: usb0: Gained carrier
>
> root@iwg26:~# candump can0 | head -n 2
>
> can0 123 [4] DE AD BE EF
> can0 123 [4] DE AD BE EF
> root@iwg26:~# candump can1 | head -n 2
>
> can1 123 [4] DE AD BE EF
> can1 123 [4] DE AD BE EF
> root@iwg26:~# systemctl suspend
>
> root@iwg26:~# [ 385.106658] systemd[1]: Reached target Sleep.
> [ 385.147602] systemd[1]: Starting Suspend...
> [ 385.246421] systemd-sleep[260]: Suspending system...
> [ 385.634733] dpm_run_callback(): platform_pm_suspend+0x0/0x5c returns
> -110 [ 385.634855] PM: Device 2090000.flexcan failed to suspend: error -110
> [ 385.634897] PM: Some devices failed to suspend, or early wake event
> detected [ 385.856251] PM: noirq suspend of devices failed [ 385.998364]
> systemd-sleep[260]: System resumed.
CAN0 suspended failed, CAN1 noirq suspended failed, then CAN1, CAN0 resumed back, so CAN0/CAN1 can work fine.
> [ 386.023390] systemd[1]: Started Suspend.
> [ 386.031570] systemd[1]: sleep.target: Unit not needed anymore. Stopping.
> [ 386.055886] systemd[1]: Stopped target Sleep.
> [ 386.061430] systemd[1]: Reached target Suspend.
> [ 386.066142] systemd[1]: suspend.target: Unit not needed anymore.
> Stopping.
> [ 386.112575] systemd-networkd[220]: usb0: Lost carrier [ 386.116797]
> systemd-logind[135]: Operation 'sleep' finished.
> [ 386.146161] systemd[1]: Stopped target Suspend.
> [ 386.260866] systemd-networkd[220]: usb0: Gained carrier root@iwg26:~#
> candump can0 | head -n 2
> can0 123 [4] DE AD BE EF
> can0 123 [4] DE AD BE EF
> root@iwg26:~# candump can1 | head -n 2
>
> can1 123 [4] DE AD BE EF
> can1 123 [4] DE AD BE EF
> root@iwg26:~# systemctl suspend
>
> [ 396.919303] systemd[1]: Reached target Sleep.
> root@iwg26:~# [ 396.964722] systemd[1]: Starting Suspend...
> [ 397.067336] systemd-sleep[268]: Suspending system...
> [ 397.574571] PM: noirq suspend of devices failed [ 397.834731] PM: noirq
> suspend of devices failed [ 397.807996] systemd-networkd[220]: usb0: Lost
> carrier [ 398.156295] dpm_run_callback(): platform_pm_suspend+0x0/0x5c
> returns -110 [ 398.156339] PM: Device 2094000.flexcan failed to suspend:
> error -110 [ 398.156509] PM: Some devices failed to suspend, or early wake
> event detected [ 398.053555] systemd-sleep[268]: Failed to write
> /sys/power/state:
> Device or resource busy
But the log here is very strange and chaotic, it looks like CAN0 suspended failed, then resumed back, so CAN0 can work fine.
CAN1 noirq suspend failed, but have not resumed back, so CAN1 still in stop mode, cannot work. I think this may be other device noirq suspend failed
broke the resume of CAN1.
Could you do more debug to help locate the issue?
> [ 398.074751] systemd[1]: systemd-suspend.service: Main process exited,
> code=exited, status=1/FAILURE [ 398.076779] systemd[1]:
> systemd-suspend.service: Failed with result 'exit-code'.
> [ 398.109255] systemd[1]: Failed to start Suspend.
> [ 398.118704] systemd[1]: Dependency failed for Suspend.
> [ 398.136283] systemd-logind[135]: Operation 'sleep' finished.
> [ 398.137770] systemd[1]: suspend.target: Job suspend.target/start failed
> with result 'dependency'.
> [ 398.139105] systemd[1]: sleep.target: Unit not needed anymore. Stopping.
> [ 398.167590] systemd[1]: Stopped target Sleep.
> [ 398.201558] systemd-networkd[220]: usb0: Gained carrier
Log here also strange.
Best Regards,
Joakim Zhang
> root@iwg26:~# candump can0 | head -n 2
> can0 123 [4] DE AD BE EF
> can0 123 [4] DE AD BE EF
> root@iwg26:~# candump can1 | head -n 2
>
> nothing on can1 anymore :-(
>
> root@iwg26:~# rmmod flexcan
> [ 622.884746] systemd-networkd[220]: can1: Lost carrier [ 623.046766]
> systemd-networkd[220]: can0: Lost carrier root@iwg26:~# insmod
> /mnt/flexcan.ko [ 628.323981] flexcan 2094000.flexcan: registering netdev
> failed
>
> and can1 fails to register with:
> [ 628.347485] flexcan: probe of 2094000.flexcan failed with error -110
>
> /Sean
^ permalink raw reply
* RE: [PATCH v2 0/2] Simplify mtty driver and mdev core
From: Parav Pandit @ 2019-08-20 11:25 UTC (permalink / raw)
To: Christophe de Dinechin
Cc: Alex Williamson, Jiri Pirko, David S . Miller, Kirti Wankhede,
Cornelia Huck, kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
cjia, netdev@vger.kernel.org
In-Reply-To: <m1o90kduow.fsf@dinechin.org>
> -----Original Message-----
> From: Christophe de Dinechin <christophe.de.dinechin@gmail.com>
> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core
>
>
> Parav Pandit writes:
>
> > + Dave.
> >
> > Hi Jiri, Dave, Alex, Kirti, Cornelia,
> >
> > Please provide your feedback on it, how shall we proceed?
> >
> > Hence, I would like to discuss below options.
> >
> > Option-1: mdev index
> > Introduce an optional mdev index/handle as u32 during mdev create time.
> > User passes mdev index/handle as input.
> >
> > phys_port_name=mIndex=m%u
> > mdev_index will be available in sysfs as mdev attribute for udev to name the
> mdev's netdev.
> >
> > example mdev create command:
> > UUID=$(uuidgen)
> > echo $UUID index=10 >
> > /sys/class/net/ens2f0/mdev_supported_types/mlx5_core_mdev/create
> > example netdevs:
> > repnetdev=ens2f0_m10 /*ens2f0 is parent PF's netdevice */
> > mdev_netdev=enm10
> >
> > Pros:
> > 1. mdevctl and any other existing tools are unaffected.
> > 2. netdev stack, ovs and other switching platforms are unaffected.
> > 3. achieves unique phys_port_name for representor netdev 4. achieves
> > unique mdev eth netdev name for the mdev using udev/systemd extension.
> > 5. Aligns well with mdev and netdev subsystem and similar to existing sriov
> bdf's.
> >
> > Option-2: shorter mdev name
> > Extend mdev to have shorter mdev device name in addition to UUID.
> > such as 'foo', 'bar'.
> > Mdev will continue to have UUID.
> > phys_port_name=mdev_name
> >
> > Pros:
> > 1. All same as option-1, except mdevctl needs upgrade for newer usage.
> > It is common practice to upgrade iproute2 package along with the kernel.
> > Similar practice to be done with mdevctl.
> > 2. Newer users of mdevctl who wants to work with non_UUID names, will use
> newer mdevctl/tools.
> > Cons:
> > 1. Dual naming scheme of mdev might affect some of the existing tools.
> > It's unclear how/if it actually affects.
> > mdevctl [2] is very recently developed and can be enhanced for dual naming
> scheme.
> >
> > Option-3: mdev uuid alias
> > Instead of shorter mdev name or mdev index, have alpha-numeric name
> alias.
> > Alias is an optional mdev sysfs attribute such as 'foo', 'bar'.
> > example mdev create command:
> > UUID=$(uuidgen)
> > echo $UUID alias=foo >
> > /sys/class/net/ens2f0/mdev_supported_types/mlx5_core_mdev/create
> > example netdevs:
> > examle netdevs:
> > repnetdev = ens2f0_mfoo
> > mdev_netdev=enmfoo
> >
> > Pros:
> > 1. All same as option-1.
> > 2. Doesn't affect existing mdev naming scheme.
> > Cons:
> > 1. Index scheme of option-1 is better which can number large number of
> mdevs with fewer characters, simplifying the management tool.
>
> I believe that Alex pointed out another "Cons" to all three options, which is that
> it forces user-space to resolve potential race conditions when creating an index
> or short name or alias.
>
This race condition exists for at least two subsystems that I know of, i.e. netdev and rdma.
If a device with a given name exists, subsystem returns error.
When user space gets error code EEXIST, and it can picks up different identifier(s).
> Also, what happens if `index=10` is not provided on the command-line?
> Does that make the device unusable for your purpose?
Yes, it is unusable to an extent.
Currently we have DEVLINK_PORT_FLAVOUR_PCI_VF in include/uapi/linux/devlink.h
Similar to it, we need to have DEVLINK_PORT_FLAVOUR_MDEV for mdev eswitch ports.
This port flavour needs to generate phys_port_name(). This should be user parameter driven.
Because representor netdevice name is generated based on this parameter.
^ permalink raw reply
* Re: [PATCH -next] bpf: Use PTR_ERR_OR_ZERO in xsk_map_inc()
From: Yuehaibing @ 2019-08-20 11:46 UTC (permalink / raw)
To: Dan Carpenter, Björn Töpel
Cc: Björn Töpel, Karlsson, Magnus, Jonathan Lemon,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, Netdev, bpf, kernel-janitors
In-Reply-To: <20190820094444.GA3964@kadam>
On 2019/8/20 17:44, Dan Carpenter wrote:
> On Tue, Aug 20, 2019 at 11:25:29AM +0200, Björn Töpel wrote:
>> On Tue, 20 Aug 2019 at 10:59, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>>>
>>> On Tue, Aug 20, 2019 at 09:28:26AM +0200, Björn Töpel wrote:
>>>> For future patches: Prefix AF_XDP socket work with "xsk:" and use "PATCH
>>>> bpf-next" to let the developers know what tree you're aiming for.
>>>
>>> There are over 300 trees in linux-next. It impossible to try remember
>>> everyone's trees. No one else has this requirement.
>>>
>>
>> Net/bpf are different, and I wanted to point that out to lessen the
>> burden for the maintainers. It's documented in:
>>
>> Documentation/bpf/bpf_devel_QA.rst.
>> Documentation/networking/netdev-FAQ.rst
>
> Ah... I hadn't realized that BPF patches were confusing to Dave.
>
> I actually do keep track of net and net-next. I do quite a bit of extra
> stuff for netdev patches. So what about if we used [PATCH] for bpf and
> [PATCH net] and [PATCH net-next] for networking?
>
> I will do that.
bpf-next is a good choice.
>
> regards,
> dan carpenter
>
> .
>
^ permalink raw reply
* [RFC bpf-next 0/5] Convert iproute2 to use libbpf (WIP)
From: Toke Høiland-Jørgensen @ 2019-08-20 11:47 UTC (permalink / raw)
To: Stephen Hemminger, Daniel Borkmann, Alexei Starovoitov
Cc: Martin KaFai Lau, Song Liu, Yonghong Song, David Miller,
Jesper Dangaard Brouer, netdev, bpf,
Toke Høiland-Jørgensen
iproute2 uses its own bpf loader to load eBPF programs, which has
evolved separately from libbpf. Since we are now standardising on
libbpf, this becomes a problem as iproute2 is slowly accumulating
feature incompatibilities with libbpf-based loaders. In particular,
iproute2 has its own (expanded) version of the map definition struct,
which makes it difficult to write programs that can be loaded with both
custom loaders and iproute2.
This series seeks to address this by converting iproute2 to using libbpf
for all its bpf needs. This version is an early proof-of-concept RFC, to
get some feedback on whether people think this is the right direction.
What this series does is the following:
- Updates the libbpf map definition struct to match that of iproute2
(patch 1).
- Adds functionality to libbpf to support automatic pinning of maps when
loading an eBPF program, while re-using pinned maps if they already
exist (patches 2-3).
- Modifies iproute2 to make it possible to compile it against libbpf
without affecting any existing functionality (patch 4).
- Changes the iproute2 eBPF loader to use libbpf for loading XDP
programs (patch 5).
As this is an early PoC, there are still a few missing pieces before
this can be merged. Including (but probably not limited to):
- Consolidate the map definition struct in the bpf_helpers.h file in the
kernel tree. This contains a different, and incompatible, update to
the struct. Since the iproute2 version has actually been released for
use outside the kernel tree (and thus is subject to API stability
constraints), I think it makes the most sense to keep that, and port
the selftests to use it.
- The iproute2 loader supports automatically populating map-in-map
definitions on load. This needs to be added to libbpf as well. There
is an implementation of this in the selftests in the kernel tree,
which will have to be ported (related to the previous point).
- The iproute2 port needs to be completed; this means at least
supporting TC eBPF programs as well, figuring out how to deal with
cBPF programs, and getting the verbose output back to the same state
as before the port. Also, I guess the iproute2 maintainers need to ACK
that they are good with adding a dependency on libbpf.
- Some of the code added to libbpf in patch 2 in this series include
code derived from iproute2, which is GPLv2+. So it will need to be
re-licensed to be usable in libbpf. Since `git blame` indicated that
the original code was written by Daniel, I figure he can ACK that
relicensing before applying the patches :)
Please take a look at this series and let me know if you agree
that this is the right direction to go. Assuming there's consensus that
it is, I'll focus on getting the rest of the libbpf patches ready for
merging. I'll send those as a separate series, and hold off on the
iproute2 patches until they are merged; but for this version I'm
including both in one series so it's easier to see the context.
-Toke
libbpf:
Toke Høiland-Jørgensen (3):
libbpf: Add map definition struct fields from iproute2
libbpf: Add support for auto-pinning of maps with reuse on program
load
libbpf: Add support for specifying map pinning path via callback
tools/lib/bpf/libbpf.c | 205 +++++++++++++++++++++++++++++++++++++++--
tools/lib/bpf/libbpf.h | 18 ++++
2 files changed, 214 insertions(+), 9 deletions(-)
iproute2:
Toke Høiland-Jørgensen (2):
iproute2: Allow compiling against libbpf
iproute2: Support loading XDP programs with libbpf
configure | 16 +++++
include/bpf_util.h | 6 +-
ip/ipvrf.c | 4 +-
lib/bpf.c | 148 ++++++++++++++++++++++++++++++++++++---------
4 files changed, 142 insertions(+), 32 deletions(-)
--
2.22.1
^ permalink raw reply
* [RFC bpf-next 1/5] libbpf: Add map definition struct fields from iproute2
From: Toke Høiland-Jørgensen @ 2019-08-20 11:47 UTC (permalink / raw)
To: Stephen Hemminger, Daniel Borkmann, Alexei Starovoitov
Cc: Martin KaFai Lau, Song Liu, Yonghong Song, David Miller,
Jesper Dangaard Brouer, netdev, bpf,
Toke Høiland-Jørgensen
In-Reply-To: <20190820114706.18546-1-toke@redhat.com>
The iproute2 bpf headers define four more fields for map definitions than
libbpf does. This adds those fields to the libbpf headers, in preparation
for porting the bpf loading functionality of iproute2 to be based on
libbpf. Subsequent commits add the functionality needed in libbpf to be
able to port over iproute2.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
tools/lib/bpf/libbpf.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index e8f70977d137..5facba6ea1e1 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -289,6 +289,10 @@ struct bpf_map_def {
unsigned int value_size;
unsigned int max_entries;
unsigned int map_flags;
+ unsigned int map_id;
+ unsigned int pinning;
+ unsigned int inner_id;
+ unsigned int inner_idx;
};
/*
--
2.22.1
^ permalink raw reply related
* [RFC bpf-next 2/5] libbpf: Add support for auto-pinning of maps with reuse on program load
From: Toke Høiland-Jørgensen @ 2019-08-20 11:47 UTC (permalink / raw)
To: Stephen Hemminger, Daniel Borkmann, Alexei Starovoitov
Cc: Martin KaFai Lau, Song Liu, Yonghong Song, David Miller,
Jesper Dangaard Brouer, netdev, bpf,
Toke Høiland-Jørgensen
In-Reply-To: <20190820114706.18546-1-toke@redhat.com>
This adds support for automatically pinning maps on program load to libbpf.
This is needed for porting iproute2 bpf support to libbpf, but is also
useful in other contexts.
The semantics are modelled on those of the same functionality in iproute2,
namely:
- A path can be supplied in bpf_prog_load_attr specifying the directory
that maps should be pinned into.
- Only maps that specify a non-zero value in its 'pinning' definition
attribute will be pinned in the automatic mode.
- If an existing pinning is found at the pinning destination, its
attributes will be compared and if they match, the existing map will be
reused instead of creating a new map.
A subsequent commit will expand the functionality to enable programs to
support different pinning paths for different values of the map pinning
attribute, similar to what iproute2 does today.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
tools/lib/bpf/libbpf.c | 161 ++++++++++++++++++++++++++++++++++++++++-
tools/lib/bpf/libbpf.h | 8 ++
2 files changed, 168 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2233f919dd88..6d372a965c9d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -220,6 +220,7 @@ struct bpf_map {
size_t sec_offset;
int map_ifindex;
int inner_map_fd;
+ int pin_reused;
struct bpf_map_def def;
__u32 btf_key_type_id;
__u32 btf_value_type_id;
@@ -3994,8 +3995,10 @@ int bpf_map__unpin(struct bpf_map *map, const char *path)
return 0;
}
-int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
+int bpf_object__pin_maps2(struct bpf_object *obj, const char *path,
+ enum bpf_pin_mode mode)
{
+ int explicit = (mode == BPF_PIN_MODE_EXPLICIT);
struct bpf_map *map;
int err;
@@ -4015,6 +4018,9 @@ int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
char buf[PATH_MAX];
int len;
+ if ((explicit && !map->def.pinning) || map->pin_reused)
+ continue;
+
len = snprintf(buf, PATH_MAX, "%s/%s", path,
bpf_map__name(map));
if (len < 0) {
@@ -4037,6 +4043,9 @@ int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
char buf[PATH_MAX];
int len;
+ if ((explicit && !map->def.pinning) || map->pin_reused)
+ continue;
+
len = snprintf(buf, PATH_MAX, "%s/%s", path,
bpf_map__name(map));
if (len < 0)
@@ -4050,6 +4059,11 @@ int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
return err;
}
+int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
+{
+ return bpf_object__pin_maps2(obj, path, BPF_PIN_MODE_ALL);
+}
+
int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
{
struct bpf_map *map;
@@ -4802,6 +4816,141 @@ int bpf_prog_load(const char *file, enum bpf_prog_type type,
return bpf_prog_load_xattr(&attr, pobj, prog_fd);
}
+static int bpf_read_map_info(int fd, struct bpf_map_def *map,
+ enum bpf_prog_type *type)
+{
+ unsigned int val, owner_type = 0;
+ char file[PATH_MAX], buff[4096];
+ FILE *fp;
+
+ snprintf(file, sizeof(file), "/proc/%d/fdinfo/%d", getpid(), fd);
+ memset(map, 0, sizeof(*map));
+
+ fp = fopen(file, "r");
+ if (!fp) {
+ pr_warning("No procfs support?!\n");
+ return -EIO;
+ }
+
+ while (fgets(buff, sizeof(buff), fp)) {
+ if (sscanf(buff, "map_type:\t%u", &val) == 1)
+ map->type = val;
+ else if (sscanf(buff, "key_size:\t%u", &val) == 1)
+ map->key_size = val;
+ else if (sscanf(buff, "value_size:\t%u", &val) == 1)
+ map->value_size = val;
+ else if (sscanf(buff, "max_entries:\t%u", &val) == 1)
+ map->max_entries = val;
+ else if (sscanf(buff, "map_flags:\t%i", &val) == 1)
+ map->map_flags = val;
+ else if (sscanf(buff, "owner_prog_type:\t%i", &val) == 1)
+ owner_type = val;
+ }
+
+ fclose(fp);
+ if (type)
+ *type = owner_type;
+
+ return 0;
+}
+
+static void bpf_map_pin_report(const struct bpf_map_def *pin,
+ const struct bpf_map_def *obj)
+{
+ pr_warning("Map specification differs from pinned file!\n");
+
+ if (obj->type != pin->type)
+ pr_warning(" - Type: %u (obj) != %u (pin)\n",
+ obj->type, pin->type);
+ if (obj->key_size != pin->key_size)
+ pr_warning(" - Key size: %u (obj) != %u (pin)\n",
+ obj->key_size, pin->key_size);
+ if (obj->value_size != pin->value_size)
+ pr_warning(" - Value size: %u (obj) != %u (pin)\n",
+ obj->value_size, pin->value_size);
+ if (obj->max_entries != pin->max_entries)
+ pr_warning(" - Max entries: %u (obj) != %u (pin)\n",
+ obj->max_entries, pin->max_entries);
+ if (obj->map_flags != pin->map_flags)
+ pr_warning(" - Flags: %#x (obj) != %#x (pin)\n",
+ obj->map_flags, pin->map_flags);
+
+ pr_warning("\n");
+}
+
+
+
+static int bpf_map_selfcheck_pinned(int fd, const struct bpf_map_def *map,
+ int length, enum bpf_prog_type type)
+{
+ enum bpf_prog_type owner_type = 0;
+ struct bpf_map_def tmp, zero = {};
+ int ret;
+
+ ret = bpf_read_map_info(fd, &tmp, &owner_type);
+ if (ret < 0)
+ return ret;
+
+ /* The decision to reject this is on kernel side eventually, but
+ * at least give the user a chance to know what's wrong.
+ */
+ if (owner_type && owner_type != type)
+ pr_warning("Program array map owner types differ: %u (obj) != %u (pin)\n",
+ type, owner_type);
+
+ if (!memcmp(&tmp, map, length)) {
+ return 0;
+ } else {
+ /* If kernel doesn't have eBPF-related fdinfo, we cannot do much,
+ * so just accept it. We know we do have an eBPF fd and in this
+ * case, everything is 0. It is guaranteed that no such map exists
+ * since map type of 0 is unloadable BPF_MAP_TYPE_UNSPEC.
+ */
+ if (!memcmp(&tmp, &zero, length))
+ return 0;
+
+ bpf_map_pin_report(&tmp, map);
+ return -EINVAL;
+ }
+}
+
+
+int bpf_probe_pinned(const struct bpf_map *map,
+ const struct bpf_prog_load_attr *attr)
+{
+ const char *name = bpf_map__name(map);
+ char buf[PATH_MAX];
+ int fd, len, ret;
+
+ if (!attr->auto_pin_path)
+ return -ENOENT;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", attr->auto_pin_path,
+ name);
+ if (len < 0)
+ return -EINVAL;
+ else if (len >= PATH_MAX)
+ return -ENAMETOOLONG;
+
+ fd = bpf_obj_get(buf);
+ if (fd <= 0)
+ return fd;
+
+ ret = bpf_map_selfcheck_pinned(fd, &map->def,
+ offsetof(struct bpf_map_def,
+ map_id),
+ attr->prog_type);
+ if (ret < 0) {
+ close(fd);
+ pr_warning("Map \'%s\' self-check failed!\n", name);
+ return ret;
+ }
+ if (attr->log_level)
+ pr_debug("Map \'%s\' loaded as pinned!\n", name);
+
+ return fd;
+}
+
int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
struct bpf_object **pobj, int *prog_fd)
{
@@ -4853,8 +5002,14 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
}
bpf_object__for_each_map(map, obj) {
+ int fd;
+
if (!bpf_map__is_offload_neutral(map))
map->map_ifindex = attr->ifindex;
+
+ fd = bpf_probe_pinned(map, attr);
+ if (fd > 0 && !bpf_map__reuse_fd(map, fd))
+ map->pin_reused = 1;
}
if (!first_prog) {
@@ -4869,6 +5024,10 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
return -EINVAL;
}
+ if (attr->auto_pin_path)
+ bpf_object__pin_maps2(obj, attr->auto_pin_path,
+ BPF_PIN_MODE_EXPLICIT);
+
*pobj = obj;
*prog_fd = bpf_program__fd(first_prog);
return 0;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 5facba6ea1e1..3c5c3256e22d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -67,6 +67,11 @@ struct bpf_object_open_attr {
enum bpf_prog_type prog_type;
};
+enum bpf_pin_mode {
+ BPF_PIN_MODE_ALL = 0,
+ BPF_PIN_MODE_EXPLICIT,
+};
+
LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
LIBBPF_API struct bpf_object *
bpf_object__open_xattr(struct bpf_object_open_attr *attr);
@@ -79,6 +84,8 @@ int bpf_object__section_size(const struct bpf_object *obj, const char *name,
__u32 *size);
int bpf_object__variable_offset(const struct bpf_object *obj, const char *name,
__u32 *off);
+LIBBPF_API int bpf_object__pin_maps2(struct bpf_object *obj, const char *path,
+ enum bpf_pin_mode mode);
LIBBPF_API int bpf_object__pin_maps(struct bpf_object *obj, const char *path);
LIBBPF_API int bpf_object__unpin_maps(struct bpf_object *obj,
const char *path);
@@ -353,6 +360,7 @@ struct bpf_prog_load_attr {
int ifindex;
int log_level;
int prog_flags;
+ const char *auto_pin_path;
};
LIBBPF_API int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
--
2.22.1
^ permalink raw reply related
* [RFC bpf-next 3/5] libbpf: Add support for specifying map pinning path via callback
From: Toke Høiland-Jørgensen @ 2019-08-20 11:47 UTC (permalink / raw)
To: Stephen Hemminger, Daniel Borkmann, Alexei Starovoitov
Cc: Martin KaFai Lau, Song Liu, Yonghong Song, David Miller,
Jesper Dangaard Brouer, netdev, bpf,
Toke Høiland-Jørgensen
In-Reply-To: <20190820114706.18546-1-toke@redhat.com>
This adds a callback parameter that can be set in struct bpf_prog_load_attr
which will allow the calling program to specify arbitrary paths for each
map included in an ELF file being loaded.
In particular, this allows iproute2 to implement its current semantics for
map pinning on top of libbpf.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
tools/lib/bpf/libbpf.c | 73 +++++++++++++++++++++++++++++-------------
tools/lib/bpf/libbpf.h | 6 ++++
2 files changed, 57 insertions(+), 22 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6d372a965c9d..a5c379378f26 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3995,8 +3995,33 @@ int bpf_map__unpin(struct bpf_map *map, const char *path)
return 0;
}
-int bpf_object__pin_maps2(struct bpf_object *obj, const char *path,
- enum bpf_pin_mode mode)
+int __bpf_map__pin_path(const struct bpf_map *map,
+ const char *path,
+ map_pinning_path_fn fn, void *fn_ctx,
+ char *buf, int buf_len)
+{
+ const char *name = bpf_map__name(map);
+ int len;
+
+ if (fn) {
+ len = fn(fn_ctx, buf, buf_len, name, map->def.pinning);
+ buf[buf_len - 1] = '\0';
+ return len;
+ }
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ name);
+ if (len < 0)
+ return -EINVAL;
+ else if (len >= PATH_MAX)
+ return -ENAMETOOLONG;
+
+ return len;
+}
+
+int __bpf_object__pin_maps(struct bpf_object *obj, enum bpf_pin_mode mode,
+ const char *path,
+ map_pinning_path_fn cb, void *cb_ctx)
{
int explicit = (mode == BPF_PIN_MODE_EXPLICIT);
struct bpf_map *map;
@@ -4010,9 +4035,11 @@ int bpf_object__pin_maps2(struct bpf_object *obj, const char *path,
return -ENOENT;
}
- err = make_dir(path);
- if (err)
- return err;
+ if (path) {
+ err = make_dir(path);
+ if (err)
+ return err;
+ }
bpf_object__for_each_map(map, obj) {
char buf[PATH_MAX];
@@ -4021,14 +4048,10 @@ int bpf_object__pin_maps2(struct bpf_object *obj, const char *path,
if ((explicit && !map->def.pinning) || map->pin_reused)
continue;
- len = snprintf(buf, PATH_MAX, "%s/%s", path,
- bpf_map__name(map));
+ len = __bpf_map__pin_path(map, path, cb, cb_ctx, buf, PATH_MAX);
if (len < 0) {
err = -EINVAL;
goto err_unpin_maps;
- } else if (len >= PATH_MAX) {
- err = -ENAMETOOLONG;
- goto err_unpin_maps;
}
err = bpf_map__pin(map, buf);
@@ -4059,6 +4082,13 @@ int bpf_object__pin_maps2(struct bpf_object *obj, const char *path,
return err;
}
+int bpf_object__pin_maps2(struct bpf_object *obj, const char *path,
+ enum bpf_pin_mode mode)
+{
+ return __bpf_object__pin_maps(obj, mode, path, NULL, NULL);
+}
+
+
int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
{
return bpf_object__pin_maps2(obj, path, BPF_PIN_MODE_ALL);
@@ -4914,23 +4944,21 @@ static int bpf_map_selfcheck_pinned(int fd, const struct bpf_map_def *map,
}
}
-
int bpf_probe_pinned(const struct bpf_map *map,
const struct bpf_prog_load_attr *attr)
{
const char *name = bpf_map__name(map);
char buf[PATH_MAX];
- int fd, len, ret;
+ int fd, ret;
- if (!attr->auto_pin_path)
+ if (!attr->auto_pin_path && !attr->auto_pin_cb)
return -ENOENT;
- len = snprintf(buf, PATH_MAX, "%s/%s", attr->auto_pin_path,
- name);
- if (len < 0)
- return -EINVAL;
- else if (len >= PATH_MAX)
- return -ENAMETOOLONG;
+ ret = __bpf_map__pin_path(map, attr->auto_pin_path,
+ attr->auto_pin_cb, attr->auto_pin_ctx,
+ buf, PATH_MAX);
+ if (ret < 0)
+ return ret;
fd = bpf_obj_get(buf);
if (fd <= 0)
@@ -5024,9 +5052,10 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
return -EINVAL;
}
- if (attr->auto_pin_path)
- bpf_object__pin_maps2(obj, attr->auto_pin_path,
- BPF_PIN_MODE_EXPLICIT);
+ if (attr->auto_pin_path || attr->auto_pin_cb)
+ __bpf_object__pin_maps(obj, BPF_PIN_MODE_EXPLICIT,
+ attr->auto_pin_path, attr->auto_pin_cb,
+ attr->auto_pin_ctx);
*pobj = obj;
*prog_fd = bpf_program__fd(first_prog);
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 3c5c3256e22d..65fdd3389d27 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -72,6 +72,10 @@ enum bpf_pin_mode {
BPF_PIN_MODE_EXPLICIT,
};
+struct bpf_map_def;
+typedef int (*map_pinning_path_fn)(void *ctx, char *buf, int buf_len,
+ const char *name, unsigned int pinning);
+
LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
LIBBPF_API struct bpf_object *
bpf_object__open_xattr(struct bpf_object_open_attr *attr);
@@ -361,6 +365,8 @@ struct bpf_prog_load_attr {
int log_level;
int prog_flags;
const char *auto_pin_path;
+ map_pinning_path_fn auto_pin_cb;
+ void *auto_pin_ctx;
};
LIBBPF_API int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
--
2.22.1
^ permalink raw reply related
* [RFC bpf-next 4/5] iproute2: Allow compiling against libbpf
From: Toke Høiland-Jørgensen @ 2019-08-20 11:47 UTC (permalink / raw)
To: Stephen Hemminger, Daniel Borkmann, Alexei Starovoitov
Cc: Martin KaFai Lau, Song Liu, Yonghong Song, David Miller,
Jesper Dangaard Brouer, netdev, bpf,
Toke Høiland-Jørgensen
In-Reply-To: <20190820114706.18546-1-toke@redhat.com>
This adds a configure check for libbpf and renames functions to allow
lib/bpf.c to be compiled with it present. This makes it possible to
port functionality piecemeal to use libbpf.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
configure | 16 ++++++++++++++++
include/bpf_util.h | 6 +++---
ip/ipvrf.c | 4 ++--
lib/bpf.c | 33 +++++++++++++++++++--------------
4 files changed, 40 insertions(+), 19 deletions(-)
diff --git a/configure b/configure
index 45fcffb6..5a89ee9f 100755
--- a/configure
+++ b/configure
@@ -238,6 +238,19 @@ check_elf()
fi
}
+check_libbpf()
+{
+ if ${PKG_CONFIG} libbpf --exists; then
+ echo "HAVE_LIBBPF:=y" >>$CONFIG
+ echo "yes"
+
+ echo 'CFLAGS += -DHAVE_LIBBPF' `${PKG_CONFIG} libbpf --cflags` >> $CONFIG
+ echo 'LDLIBS += ' `${PKG_CONFIG} libbpf --libs` >>$CONFIG
+ else
+ echo "no"
+ fi
+}
+
check_selinux()
# SELinux is a compile time option in the ss utility
{
@@ -386,6 +399,9 @@ check_selinux
echo -n "ELF support: "
check_elf
+echo -n "libbpf support: "
+check_libbpf
+
echo -n "libmnl support: "
check_mnl
diff --git a/include/bpf_util.h b/include/bpf_util.h
index 63db07ca..72d3a32c 100644
--- a/include/bpf_util.h
+++ b/include/bpf_util.h
@@ -274,9 +274,9 @@ int bpf_trace_pipe(void);
void bpf_print_ops(struct rtattr *bpf_ops, __u16 len);
-int bpf_prog_load(enum bpf_prog_type type, const struct bpf_insn *insns,
- size_t size_insns, const char *license, char *log,
- size_t size_log);
+int bpf_prog_load_buf(enum bpf_prog_type type, const struct bpf_insn *insns,
+ size_t size_insns, const char *license, char *log,
+ size_t size_log);
int bpf_prog_attach_fd(int prog_fd, int target_fd, enum bpf_attach_type type);
int bpf_prog_detach_fd(int target_fd, enum bpf_attach_type type);
diff --git a/ip/ipvrf.c b/ip/ipvrf.c
index 43366f6e..1d1aae6f 100644
--- a/ip/ipvrf.c
+++ b/ip/ipvrf.c
@@ -256,8 +256,8 @@ static int prog_load(int idx)
BPF_EXIT_INSN(),
};
- return bpf_prog_load(BPF_PROG_TYPE_CGROUP_SOCK, prog, sizeof(prog),
- "GPL", bpf_log_buf, sizeof(bpf_log_buf));
+ return bpf_prog_load_buf(BPF_PROG_TYPE_CGROUP_SOCK, prog, sizeof(prog),
+ "GPL", bpf_log_buf, sizeof(bpf_log_buf));
}
static int vrf_configure_cgroup(const char *path, int ifindex)
diff --git a/lib/bpf.c b/lib/bpf.c
index 7d2a322f..c6e3bd0d 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -28,6 +28,11 @@
#include <gelf.h>
#endif
+#ifdef HAVE_LIBBPF
+#include <bpf/libbpf.h>
+#include <bpf/bpf.h>
+#endif
+
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/un.h>
@@ -795,7 +800,7 @@ out:
return mnt;
}
-static int bpf_obj_get(const char *pathname, enum bpf_prog_type type)
+static int bpf_obj_get_path(const char *pathname, enum bpf_prog_type type)
{
union bpf_attr attr = {};
char tmp[PATH_MAX];
@@ -814,7 +819,7 @@ static int bpf_obj_get(const char *pathname, enum bpf_prog_type type)
static int bpf_obj_pinned(const char *pathname, enum bpf_prog_type type)
{
- int prog_fd = bpf_obj_get(pathname, type);
+ int prog_fd = bpf_obj_get_path(pathname, type);
if (prog_fd < 0)
fprintf(stderr, "Couldn\'t retrieve pinned program \'%s\': %s\n",
@@ -1036,7 +1041,7 @@ int bpf_graft_map(const char *map_path, uint32_t *key, int argc, char **argv)
}
}
- map_fd = bpf_obj_get(map_path, cfg.type);
+ map_fd = bpf_obj_get_path(map_path, cfg.type);
if (map_fd < 0) {
fprintf(stderr, "Couldn\'t retrieve pinned map \'%s\': %s\n",
map_path, strerror(errno));
@@ -1105,9 +1110,9 @@ static int bpf_prog_load_dev(enum bpf_prog_type type,
return bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
}
-int bpf_prog_load(enum bpf_prog_type type, const struct bpf_insn *insns,
- size_t size_insns, const char *license, char *log,
- size_t size_log)
+int bpf_prog_load_buf(enum bpf_prog_type type, const struct bpf_insn *insns,
+ size_t size_insns, const char *license, char *log,
+ size_t size_log)
{
return bpf_prog_load_dev(type, insns, size_insns, license, 0,
log, size_log);
@@ -1284,7 +1289,7 @@ static int bpf_btf_load(void *btf, size_t size_btf,
return bpf(BPF_BTF_LOAD, &attr, sizeof(attr));
}
-static int bpf_obj_pin(int fd, const char *pathname)
+static int bpf_obj_pin_fd(int fd, const char *pathname)
{
union bpf_attr attr = {};
@@ -1433,7 +1438,7 @@ static int bpf_probe_pinned(const char *name, const struct bpf_elf_ctx *ctx,
return 0;
bpf_make_pathname(pathname, sizeof(pathname), name, ctx, pinning);
- return bpf_obj_get(pathname, ctx->type);
+ return bpf_obj_get_path(pathname, ctx->type);
}
static int bpf_make_obj_path(const struct bpf_elf_ctx *ctx)
@@ -1501,7 +1506,7 @@ static int bpf_place_pinned(int fd, const char *name,
return ret;
bpf_make_pathname(pathname, sizeof(pathname), name, ctx, pinning);
- return bpf_obj_pin(fd, pathname);
+ return bpf_obj_pin_fd(fd, pathname);
}
static void bpf_prog_report(int fd, const char *section,
@@ -1523,9 +1528,9 @@ static void bpf_prog_report(int fd, const char *section,
bpf_dump_error(ctx, "Verifier analysis:\n\n");
}
-static int bpf_prog_attach(const char *section,
- const struct bpf_elf_prog *prog,
- struct bpf_elf_ctx *ctx)
+static int bpf_prog_attach_section(const char *section,
+ const struct bpf_elf_prog *prog,
+ struct bpf_elf_ctx *ctx)
{
int tries = 0, fd;
retry:
@@ -2347,7 +2352,7 @@ static int bpf_fetch_prog(struct bpf_elf_ctx *ctx, const char *section,
prog.insns_num = prog.size / sizeof(struct bpf_insn);
prog.insns = data.sec_data->d_buf;
- fd = bpf_prog_attach(section, &prog, ctx);
+ fd = bpf_prog_attach_section(section, &prog, ctx);
if (fd < 0)
return fd;
@@ -2513,7 +2518,7 @@ static int bpf_fetch_prog_relo(struct bpf_elf_ctx *ctx, const char *section,
goto out;
}
- fd = bpf_prog_attach(section, prog, ctx);
+ fd = bpf_prog_attach_section(section, prog, ctx);
free(prog->insns);
if (fd < 0) {
*lderr = true;
--
2.22.1
^ permalink raw reply related
* [RFC bpf-next 5/5] iproute2: Support loading XDP programs with libbpf
From: Toke Høiland-Jørgensen @ 2019-08-20 11:47 UTC (permalink / raw)
To: Stephen Hemminger, Daniel Borkmann, Alexei Starovoitov
Cc: Martin KaFai Lau, Song Liu, Yonghong Song, David Miller,
Jesper Dangaard Brouer, netdev, bpf,
Toke Høiland-Jørgensen
In-Reply-To: <20190820114706.18546-1-toke@redhat.com>
This switches over loading of XDP programs to the using libbpf, if it is
available. It uses the automatic pinning features added to libbpf to
construct the same pinning paths as the libelf-based loader.
Since map-in-map support has not yet been added to libbpf, this means that
map-in-map definitions will not work with this patch.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
lib/bpf.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 102 insertions(+), 13 deletions(-)
diff --git a/lib/bpf.c b/lib/bpf.c
index c6e3bd0d..de1a655a 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -938,9 +938,17 @@ static int bpf_do_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
return ret;
}
+#ifdef HAVE_LIBBPF
+static int bpf_do_load_libbpf(struct bpf_cfg_in *cfg);
+#endif
+
static int bpf_do_load(struct bpf_cfg_in *cfg)
{
if (cfg->mode == EBPF_OBJECT) {
+#ifdef HAVE_LIBBPF
+ if(cfg->type == BPF_PROG_TYPE_XDP)
+ return bpf_do_load_libbpf(cfg);
+#endif
cfg->prog_fd = bpf_obj_open(cfg->object, cfg->type,
cfg->section, cfg->ifindex,
cfg->verbose);
@@ -1407,25 +1415,22 @@ static bool bpf_no_pinning(const struct bpf_elf_ctx *ctx,
}
}
-static void bpf_make_pathname(char *pathname, size_t len, const char *name,
+static int bpf_make_pathname(char *pathname, size_t len, const char *name,
const struct bpf_elf_ctx *ctx, uint32_t pinning)
{
switch (pinning) {
case PIN_OBJECT_NS:
- snprintf(pathname, len, "%s/%s/%s",
- bpf_get_work_dir(ctx->type),
- ctx->obj_uid, name);
- break;
+ return snprintf(pathname, len, "%s/%s/%s",
+ bpf_get_work_dir(ctx->type),
+ ctx->obj_uid, name);
case PIN_GLOBAL_NS:
- snprintf(pathname, len, "%s/%s/%s",
- bpf_get_work_dir(ctx->type),
- BPF_DIR_GLOBALS, name);
- break;
+ return snprintf(pathname, len, "%s/%s/%s",
+ bpf_get_work_dir(ctx->type),
+ BPF_DIR_GLOBALS, name);
default:
- snprintf(pathname, len, "%s/../%s/%s",
- bpf_get_work_dir(ctx->type),
- bpf_custom_pinning(ctx, pinning), name);
- break;
+ return snprintf(pathname, len, "%s/../%s/%s",
+ bpf_get_work_dir(ctx->type),
+ bpf_custom_pinning(ctx, pinning), name);
}
}
@@ -3160,3 +3165,87 @@ int bpf_recv_map_fds(const char *path, int *fds, struct bpf_map_aux *aux,
return ret;
}
#endif /* HAVE_ELF */
+
+#ifdef HAVE_LIBBPF
+static int bpf_gen_pin_name(void *priv, char *buf, int buf_len,
+ const char *name, unsigned int pinning)
+{
+ struct bpf_elf_ctx *ctx = priv;
+ const char *tmp;
+ int ret = 0;
+
+ if (bpf_no_pinning(ctx, pinning) || !bpf_get_work_dir(ctx->type))
+ return 0;
+
+ if (pinning == PIN_OBJECT_NS)
+ ret = bpf_make_obj_path(ctx);
+ else if ((tmp = bpf_custom_pinning(ctx, pinning)))
+ ret = bpf_make_custom_path(ctx, tmp);
+ if (ret < 0)
+ return ret;
+
+ return bpf_make_pathname(buf, buf_len, name, ctx, pinning);
+}
+
+static int bpf_elf_ctx_init_stub(struct bpf_elf_ctx *ctx, const char *pathname,
+ enum bpf_prog_type type, int verbose)
+{
+ uint8_t tmp[20];
+ int ret;
+
+ memset(ctx, 0, sizeof(*ctx));
+ ret = bpf_obj_hash(pathname, tmp, sizeof(tmp));
+ if (ret)
+ ctx->noafalg = true;
+ else
+ hexstring_n2a(tmp, sizeof(tmp), ctx->obj_uid,
+ sizeof(ctx->obj_uid));
+
+ ctx->verbose = verbose;
+ ctx->type = type;
+ bpf_hash_init(ctx, CONFDIR "/bpf_pinning");
+
+ return 0;
+}
+
+static int verbose_print(enum libbpf_print_level level, const char *format,
+ va_list args)
+{
+ return vfprintf(stderr, format, args);
+}
+
+static int bpf_do_load_libbpf(struct bpf_cfg_in *cfg)
+{
+ struct bpf_elf_ctx *ctx = &__ctx;
+ struct bpf_program *prog;
+ struct bpf_object *obj;
+ int err, prog_fd = -1;
+
+ struct bpf_prog_load_attr attr = {
+ .file = cfg->object,
+ .prog_type = cfg->type,
+ .ifindex = cfg->ifindex,
+ .log_level = cfg->verbose,
+ .auto_pin_cb = bpf_gen_pin_name,
+ .auto_pin_ctx = ctx,
+ };
+
+ if (cfg->verbose)
+ libbpf_set_print(verbose_print);
+
+ bpf_elf_ctx_init_stub(ctx, cfg->object, cfg->type, cfg->verbose);
+
+ err = bpf_prog_load_xattr(&attr, &obj, &prog_fd);
+ if (err)
+ return err;
+
+ if (cfg->section) {
+ prog = bpf_object__find_program_by_title(obj,
+ cfg->section);
+ prog_fd = bpf_program__fd(prog);
+ }
+
+ cfg->prog_fd = prog_fd;
+ return cfg->prog_fd;
+}
+#endif
--
2.22.1
^ permalink raw reply related
* [PATCH] xsk: fix ptr_ret.cocci warnings
From: kbuild test robot @ 2019-08-20 11:50 UTC (permalink / raw)
To: =?unknown-8bit?B?QmrDtnJuIFTDtnBlbA==?=
Cc: kbuild-all, Daniel Borkmann, Magnus Karlsson, Jonathan Lemon,
Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
netdev, bpf
In-Reply-To: <201908201913.pObcBf2Z%lkp@intel.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 1019 bytes --]
From: kbuild test robot <lkp@intel.com>
kernel/bpf/xskmap.c:24:8-14: WARNING: PTR_ERR_OR_ZERO can be used
Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
Generated by: scripts/coccinelle/api/ptr_ret.cocci
Fixes: 0402acd683c6 ("xsk: remove AF_XDP socket from map when the socket is released")
CC: Björn Töpel <bjorn.topel@intel.com>
Signed-off-by: kbuild test robot <lkp@intel.com>
---
tree: https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next.git master
head: 54c851a8cc739ce7f1aaea583940054cdfe2223f
commit: 0402acd683c678874df6bdbc23530ca07ea19353 [7165/7710] xsk: remove AF_XDP socket from map when the socket is released
xskmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/kernel/bpf/xskmap.c
+++ b/kernel/bpf/xskmap.c
@@ -21,7 +21,7 @@ int xsk_map_inc(struct xsk_map *map)
struct bpf_map *m = &map->map;
m = bpf_map_inc(m, false);
- return IS_ERR(m) ? PTR_ERR(m) : 0;
+ return PTR_ERR_OR_ZERO(m);
}
void xsk_map_put(struct xsk_map *map)
^ permalink raw reply
* [PATCH v6 1/7] nfc: pn533: i2c: "pn532" as dt compatible string
From: Lars Poeschel @ 2019-08-20 12:03 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jilayne Lovejoy, Thomas Gleixner,
Kate Stewart, Steve Winslow, Lars Poeschel,
open list:NFC SUBSYSTEM, open list
Cc: Johan Hovold
It is favourable to have one unified compatible string for devices that
have multiple interfaces. So this adds simply "pn532" as the devicetree
binding compatible string and makes a note that the old ones are
deprecated.
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v6:
- Rebased the patch series on v5.3-rc5
Changes in v3:
- This patch is new in v3
drivers/nfc/pn533/i2c.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/nfc/pn533/i2c.c b/drivers/nfc/pn533/i2c.c
index 1832cd921ea7..1abd40398a5a 100644
--- a/drivers/nfc/pn533/i2c.c
+++ b/drivers/nfc/pn533/i2c.c
@@ -245,6 +245,11 @@ static int pn533_i2c_remove(struct i2c_client *client)
}
static const struct of_device_id of_pn533_i2c_match[] = {
+ { .compatible = "nxp,pn532", },
+ /*
+ * NOTE: The use of the compatibles with the trailing "...-i2c" is
+ * deprecated and will be removed.
+ */
{ .compatible = "nxp,pn533-i2c", },
{ .compatible = "nxp,pn532-i2c", },
{},
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH v6 3/7] nfc: pn533: Add dev_up/dev_down hooks to phy_ops
From: Lars Poeschel @ 2019-08-20 12:03 UTC (permalink / raw)
To: Steve Winslow, Thomas Gleixner, Kees Cook, Allison Randal,
Gustavo A. R. Silva, Lars Poeschel, Jilayne Lovejoy, Kate Stewart,
open list:NFC SUBSYSTEM, open list
Cc: Johan Hovold
In-Reply-To: <20190820120345.22593-1-poeschel@lemonage.de>
This adds hooks for dev_up and dev_down to the phy_ops. They are
optional.
The idea is to inform the phy driver when the nfc chip is really going
to be used. When it is not used, the phy driver can suspend it's
interface to the nfc chip to save some power. The nfc chip is considered
not in use before dev_up and after dev_down.
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v6:
- Rebased the patch series on v5.3-rc5
Changes in v5:
- (dev->phy_ops->dev_up) instead (dev->phy_ops)
Changes in v4:
- This patch is new in v4
drivers/nfc/pn533/pn533.c | 12 +++++++++++-
drivers/nfc/pn533/pn533.h | 9 +++++++++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index a172a32aa9d9..64836c727aee 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -2458,6 +2458,9 @@ static int pn533_dev_up(struct nfc_dev *nfc_dev)
{
struct pn533 *dev = nfc_get_drvdata(nfc_dev);
+ if (dev->phy_ops->dev_up)
+ dev->phy_ops->dev_up(dev);
+
if (dev->device_type == PN533_DEVICE_PN532) {
int rc = pn532_sam_configuration(nfc_dev);
@@ -2470,7 +2473,14 @@ static int pn533_dev_up(struct nfc_dev *nfc_dev)
static int pn533_dev_down(struct nfc_dev *nfc_dev)
{
- return pn533_rf_field(nfc_dev, 0);
+ struct pn533 *dev = nfc_get_drvdata(nfc_dev);
+ int ret;
+
+ ret = pn533_rf_field(nfc_dev, 0);
+ if (dev->phy_ops->dev_down && !ret)
+ dev->phy_ops->dev_down(dev);
+
+ return ret;
}
static struct nfc_ops pn533_nfc_ops = {
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 8bf9d6ece0f5..570ee0a3e832 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -207,6 +207,15 @@ struct pn533_phy_ops {
struct sk_buff *out);
int (*send_ack)(struct pn533 *dev, gfp_t flags);
void (*abort_cmd)(struct pn533 *priv, gfp_t flags);
+ /*
+ * dev_up and dev_down are optional.
+ * They are used to inform the phy layer that the nfc chip
+ * is going to be really used very soon. The phy layer can then
+ * bring up it's interface to the chip and have it suspended for power
+ * saving reasons otherwise.
+ */
+ void (*dev_up)(struct pn533 *priv);
+ void (*dev_down)(struct pn533 *priv);
};
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH v6 4/7] nfc: pn533: Split pn533 init & nfc_register
From: Lars Poeschel @ 2019-08-20 12:03 UTC (permalink / raw)
To: Greg Kroah-Hartman, Allison Randal, Steve Winslow,
Thomas Gleixner, Kate Stewart, Lars Poeschel, Gustavo A. R. Silva,
Kees Cook, Jilayne Lovejoy, open list:NFC SUBSYSTEM, open list
Cc: Johan Hovold
In-Reply-To: <20190820120345.22593-1-poeschel@lemonage.de>
There is a problem in the initialisation and setup of the pn533: It
registers with nfc too early. It could happen, that it finished
registering with nfc and someone starts using it. But setup of the pn533
is not yet finished. Bad or at least unintended things could happen.
So I split out nfc registering (and unregistering) to seperate functions
that have to be called late in probe then.
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v6:
- Rebased the patch series on v5.3-rc5
Changes in v5:
- This patch is new in v5
drivers/nfc/pn533/i2c.c | 17 +++++-----
drivers/nfc/pn533/pn533.c | 66 ++++++++++++++++++++-------------------
drivers/nfc/pn533/pn533.h | 11 ++++---
drivers/nfc/pn533/usb.c | 12 +++++--
4 files changed, 59 insertions(+), 47 deletions(-)
diff --git a/drivers/nfc/pn533/i2c.c b/drivers/nfc/pn533/i2c.c
index 1abd40398a5a..e9e5a1ec8857 100644
--- a/drivers/nfc/pn533/i2c.c
+++ b/drivers/nfc/pn533/i2c.c
@@ -193,12 +193,10 @@ static int pn533_i2c_probe(struct i2c_client *client,
phy->i2c_dev = client;
i2c_set_clientdata(client, phy);
- priv = pn533_register_device(PN533_DEVICE_PN532,
- PN533_NO_TYPE_B_PROTOCOLS,
+ priv = pn53x_common_init(PN533_DEVICE_PN532,
PN533_PROTO_REQ_ACK_RESP,
phy, &i2c_phy_ops, NULL,
- &phy->i2c_dev->dev,
- &client->dev);
+ &phy->i2c_dev->dev);
if (IS_ERR(priv)) {
r = PTR_ERR(priv);
@@ -220,13 +218,17 @@ static int pn533_i2c_probe(struct i2c_client *client,
if (r)
goto fn_setup_err;
- return 0;
+ r = pn53x_register_nfc(priv, PN533_NO_TYPE_B_PROTOCOLS, &client->dev);
+ if (r)
+ goto fn_setup_err;
+
+ return r;
fn_setup_err:
free_irq(client->irq, phy);
irq_rqst_err:
- pn533_unregister_device(phy->priv);
+ pn53x_common_clean(phy->priv);
return r;
}
@@ -239,7 +241,8 @@ static int pn533_i2c_remove(struct i2c_client *client)
free_irq(client->irq, phy);
- pn533_unregister_device(phy->priv);
+ pn53x_unregister_nfc(phy->priv);
+ pn53x_common_clean(phy->priv);
return 0;
}
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index 64836c727aee..a8c756caa678 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -2590,14 +2590,12 @@ int pn533_finalize_setup(struct pn533 *dev)
}
EXPORT_SYMBOL_GPL(pn533_finalize_setup);
-struct pn533 *pn533_register_device(u32 device_type,
- u32 protocols,
+struct pn533 *pn53x_common_init(u32 device_type,
enum pn533_protocol_type protocol_type,
void *phy,
struct pn533_phy_ops *phy_ops,
struct pn533_frame_ops *fops,
- struct device *dev,
- struct device *parent)
+ struct device *dev)
{
struct pn533 *priv;
int rc = -ENOMEM;
@@ -2638,43 +2636,18 @@ struct pn533 *pn533_register_device(u32 device_type,
skb_queue_head_init(&priv->fragment_skb);
INIT_LIST_HEAD(&priv->cmd_queue);
-
- priv->nfc_dev = nfc_allocate_device(&pn533_nfc_ops, protocols,
- priv->ops->tx_header_len +
- PN533_CMD_DATAEXCH_HEAD_LEN,
- priv->ops->tx_tail_len);
- if (!priv->nfc_dev) {
- rc = -ENOMEM;
- goto destroy_wq;
- }
-
- nfc_set_parent_dev(priv->nfc_dev, parent);
- nfc_set_drvdata(priv->nfc_dev, priv);
-
- rc = nfc_register_device(priv->nfc_dev);
- if (rc)
- goto free_nfc_dev;
-
return priv;
-free_nfc_dev:
- nfc_free_device(priv->nfc_dev);
-
-destroy_wq:
- destroy_workqueue(priv->wq);
error:
kfree(priv);
return ERR_PTR(rc);
}
-EXPORT_SYMBOL_GPL(pn533_register_device);
+EXPORT_SYMBOL_GPL(pn53x_common_init);
-void pn533_unregister_device(struct pn533 *priv)
+void pn53x_common_clean(struct pn533 *priv)
{
struct pn533_cmd *cmd, *n;
- nfc_unregister_device(priv->nfc_dev);
- nfc_free_device(priv->nfc_dev);
-
flush_delayed_work(&priv->poll_work);
destroy_workqueue(priv->wq);
@@ -2689,8 +2662,37 @@ void pn533_unregister_device(struct pn533 *priv)
kfree(priv);
}
-EXPORT_SYMBOL_GPL(pn533_unregister_device);
+EXPORT_SYMBOL_GPL(pn53x_common_clean);
+
+int pn53x_register_nfc(struct pn533 *priv, u32 protocols,
+ struct device *parent)
+{
+ int rc = -ENOMEM;
+
+ priv->nfc_dev = nfc_allocate_device(&pn533_nfc_ops, protocols,
+ priv->ops->tx_header_len +
+ PN533_CMD_DATAEXCH_HEAD_LEN,
+ priv->ops->tx_tail_len);
+ if (!priv->nfc_dev)
+ return -ENOMEM;
+
+ nfc_set_parent_dev(priv->nfc_dev, parent);
+ nfc_set_drvdata(priv->nfc_dev, priv);
+
+ rc = nfc_register_device(priv->nfc_dev);
+ if (rc)
+ nfc_free_device(priv->nfc_dev);
+
+ return rc;
+}
+EXPORT_SYMBOL_GPL(pn53x_register_nfc);
+void pn53x_unregister_nfc(struct pn533 *priv)
+{
+ nfc_unregister_device(priv->nfc_dev);
+ nfc_free_device(priv->nfc_dev);
+}
+EXPORT_SYMBOL_GPL(pn53x_unregister_nfc);
MODULE_AUTHOR("Lauro Ramos Venancio <lauro.venancio@openbossa.org>");
MODULE_AUTHOR("Aloisio Almeida Jr <aloisio.almeida@openbossa.org>");
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 570ee0a3e832..510ddebbd896 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -219,18 +219,19 @@ struct pn533_phy_ops {
};
-struct pn533 *pn533_register_device(u32 device_type,
- u32 protocols,
+struct pn533 *pn53x_common_init(u32 device_type,
enum pn533_protocol_type protocol_type,
void *phy,
struct pn533_phy_ops *phy_ops,
struct pn533_frame_ops *fops,
- struct device *dev,
- struct device *parent);
+ struct device *dev);
int pn533_finalize_setup(struct pn533 *dev);
-void pn533_unregister_device(struct pn533 *priv);
+void pn53x_common_clean(struct pn533 *priv);
void pn533_recv_frame(struct pn533 *dev, struct sk_buff *skb, int status);
+int pn53x_register_nfc(struct pn533 *priv, u32 protocols,
+ struct device *parent);
+void pn53x_unregister_nfc(struct pn533 *priv);
bool pn533_rx_frame_is_cmd_response(struct pn533 *dev, void *frame);
bool pn533_rx_frame_is_ack(void *_frame);
diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index c5289eaf17ee..a1c6a41944c6 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -534,9 +534,9 @@ static int pn533_usb_probe(struct usb_interface *interface,
goto error;
}
- priv = pn533_register_device(id->driver_info, protocols, protocol_type,
+ priv = pn53x_common_init(id->driver_info, protocol_type,
phy, &usb_phy_ops, fops,
- &phy->udev->dev, &interface->dev);
+ &phy->udev->dev);
if (IS_ERR(priv)) {
rc = PTR_ERR(priv);
@@ -550,9 +550,14 @@ static int pn533_usb_probe(struct usb_interface *interface,
goto error;
usb_set_intfdata(interface, phy);
+ rc = pn53x_register_nfc(priv, protocols, &interface->dev);
+ if (rc)
+ goto err_clean;
return 0;
+err_clean:
+ pn53x_common_clean(priv);
error:
usb_free_urb(phy->in_urb);
usb_free_urb(phy->out_urb);
@@ -570,7 +575,8 @@ static void pn533_usb_disconnect(struct usb_interface *interface)
if (!phy)
return;
- pn533_unregister_device(phy->priv);
+ pn53x_unregister_nfc(phy->priv);
+ pn53x_common_clean(phy->priv);
usb_set_intfdata(interface, NULL);
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH v6 5/7] nfc: pn533: add UART phy driver
From: Lars Poeschel @ 2019-08-20 12:03 UTC (permalink / raw)
To: Greg Kroah-Hartman, Thomas Gleixner, Lars Poeschel, Steve Winslow,
Allison Randal, Jilayne Lovejoy, Kate Stewart, open list,
open list:NFC SUBSYSTEM
Cc: Johan Hovold
In-Reply-To: <20190820120345.22593-1-poeschel@lemonage.de>
This adds the UART phy interface for the pn533 driver.
The pn533 driver can be used through UART interface this way.
It is implemented as a serdev device.
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v6:
- Rebased the patch series on v5.3-rc5
Changes in v5:
- Use the splitted pn53x_common_init and pn53x_register_nfc
and pn53x_common_clean and pn53x_unregister_nfc alike
Changes in v4:
- SPDX-License-Identifier: GPL-2.0+
- Source code comments above refering items
- Error check for serdev_device_write's
- Change if (xxx == NULL) to if (!xxx)
- Remove device name from a dev_err
- move pn533_register in _probe a bit towards the end of _probe
- make use of newly added dev_up / dev_down phy_ops
- control send_wakeup variable from dev_up / dev_down
Changes in v3:
- depend on SERIAL_DEV_BUS in Kconfig
Changes in v2:
- switched from tty line discipline to serdev, resulting in many
simplifications
- SPDX License Identifier
drivers/nfc/pn533/Kconfig | 11 ++
drivers/nfc/pn533/Makefile | 2 +
drivers/nfc/pn533/pn533.h | 8 +
drivers/nfc/pn533/uart.c | 316 +++++++++++++++++++++++++++++++++++++
4 files changed, 337 insertions(+)
create mode 100644 drivers/nfc/pn533/uart.c
diff --git a/drivers/nfc/pn533/Kconfig b/drivers/nfc/pn533/Kconfig
index f6d6b345ba0d..7fe1bbe26568 100644
--- a/drivers/nfc/pn533/Kconfig
+++ b/drivers/nfc/pn533/Kconfig
@@ -26,3 +26,14 @@ config NFC_PN533_I2C
If you choose to build a module, it'll be called pn533_i2c.
Say N if unsure.
+
+config NFC_PN532_UART
+ tristate "NFC PN532 device support (UART)"
+ depends on SERIAL_DEV_BUS
+ select NFC_PN533
+ ---help---
+ This module adds support for the NXP pn532 UART interface.
+ Select this if your platform is using the UART bus.
+
+ If you choose to build a module, it'll be called pn532_uart.
+ Say N if unsure.
diff --git a/drivers/nfc/pn533/Makefile b/drivers/nfc/pn533/Makefile
index 43c25b4f9466..b9648337576f 100644
--- a/drivers/nfc/pn533/Makefile
+++ b/drivers/nfc/pn533/Makefile
@@ -4,7 +4,9 @@
#
pn533_usb-objs = usb.o
pn533_i2c-objs = i2c.o
+pn532_uart-objs = uart.o
obj-$(CONFIG_NFC_PN533) += pn533.o
obj-$(CONFIG_NFC_PN533_USB) += pn533_usb.o
obj-$(CONFIG_NFC_PN533_I2C) += pn533_i2c.o
+obj-$(CONFIG_NFC_PN532_UART) += pn532_uart.o
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 510ddebbd896..6541088fad73 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -43,6 +43,11 @@
/* Preamble (1), SoPC (2), ACK Code (2), Postamble (1) */
#define PN533_STD_FRAME_ACK_SIZE 6
+/*
+ * Preamble (1), SoPC (2), Packet Length (1), Packet Length Checksum (1),
+ * Specific Application Level Error Code (1) , Postamble (1)
+ */
+#define PN533_STD_ERROR_FRAME_SIZE 8
#define PN533_STD_FRAME_CHECKSUM(f) (f->data[f->datalen])
#define PN533_STD_FRAME_POSTAMBLE(f) (f->data[f->datalen + 1])
/* Half start code (3), LEN (4) should be 0xffff for extended frame */
@@ -84,6 +89,9 @@
#define PN533_CMD_MI_MASK 0x40
#define PN533_CMD_RET_SUCCESS 0x00
+#define PN533_FRAME_DATALEN_ACK 0x00
+#define PN533_FRAME_DATALEN_ERROR 0x01
+#define PN533_FRAME_DATALEN_EXTENDED 0xFF
enum pn533_protocol_type {
PN533_PROTO_REQ_ACK_RESP = 0,
diff --git a/drivers/nfc/pn533/uart.c b/drivers/nfc/pn533/uart.c
new file mode 100644
index 000000000000..f1cc2354a4fd
--- /dev/null
+++ b/drivers/nfc/pn533/uart.c
@@ -0,0 +1,316 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for NXP PN532 NFC Chip - UART transport layer
+ *
+ * Copyright (C) 2018 Lemonage Software GmbH
+ * Author: Lars Pöschel <poeschel@lemonage.de>
+ * All rights reserved.
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nfc.h>
+#include <linux/netdevice.h>
+#include <linux/of.h>
+#include <linux/serdev.h>
+#include "pn533.h"
+
+#define PN532_UART_SKB_BUFF_LEN (PN533_CMD_DATAEXCH_DATA_MAXLEN * 2)
+
+enum send_wakeup {
+ PN532_SEND_NO_WAKEUP = 0,
+ PN532_SEND_WAKEUP,
+ PN532_SEND_LAST_WAKEUP,
+};
+
+
+struct pn532_uart_phy {
+ struct serdev_device *serdev;
+ struct sk_buff *recv_skb;
+ struct pn533 *priv;
+ enum send_wakeup send_wakeup;
+ struct timer_list cmd_timeout;
+ struct sk_buff *cur_out_buf;
+};
+
+static int pn532_uart_send_frame(struct pn533 *dev,
+ struct sk_buff *out)
+{
+ /* wakeup sequence and dummy bytes for waiting time */
+ static const u8 wakeup[] = {
+ 0x55, 0x55, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+ struct pn532_uart_phy *pn532 = dev->phy;
+ int err;
+
+ print_hex_dump_debug("PN532_uart TX: ", DUMP_PREFIX_NONE, 16, 1,
+ out->data, out->len, false);
+
+ pn532->cur_out_buf = out;
+ if (pn532->send_wakeup) {
+ err= serdev_device_write(pn532->serdev,
+ wakeup, sizeof(wakeup),
+ MAX_SCHEDULE_TIMEOUT);
+ if (err < 0)
+ return err;
+ }
+
+ if (pn532->send_wakeup == PN532_SEND_LAST_WAKEUP) {
+ pn532->send_wakeup = PN532_SEND_NO_WAKEUP;
+ }
+
+ err = serdev_device_write(pn532->serdev, out->data, out->len,
+ MAX_SCHEDULE_TIMEOUT);
+ if (err < 0)
+ return err;
+
+ mod_timer(&pn532->cmd_timeout, HZ / 40 + jiffies);
+ return 0;
+}
+
+static int pn532_uart_send_ack(struct pn533 *dev, gfp_t flags)
+{
+ struct pn532_uart_phy *pn532 = dev->phy;
+ /* spec 7.1.1.3: Preamble, SoPC (2), ACK Code (2), Postamble */
+ static const u8 ack[PN533_STD_FRAME_ACK_SIZE] = {
+ 0x00, 0x00, 0xff, 0x00, 0xff, 0x00};
+ int err;
+
+ err = serdev_device_write(pn532->serdev, ack, sizeof(ack),
+ MAX_SCHEDULE_TIMEOUT);
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static void pn532_uart_abort_cmd(struct pn533 *dev, gfp_t flags)
+{
+ /* An ack will cancel the last issued command */
+ pn532_uart_send_ack(dev, flags);
+ /* schedule cmd_complete_work to finish current command execution */
+ pn533_recv_frame(dev, NULL, -ENOENT);
+}
+
+static void pn532_dev_up(struct pn533 *dev)
+{
+ struct pn532_uart_phy *pn532 = dev->phy;
+
+ serdev_device_open(pn532->serdev);
+ pn532->send_wakeup = PN532_SEND_LAST_WAKEUP;
+}
+
+static void pn532_dev_down(struct pn533 *dev)
+{
+ struct pn532_uart_phy *pn532 = dev->phy;
+
+ serdev_device_close(pn532->serdev);
+ pn532->send_wakeup = PN532_SEND_WAKEUP;
+}
+
+static struct pn533_phy_ops uart_phy_ops = {
+ .send_frame = pn532_uart_send_frame,
+ .send_ack = pn532_uart_send_ack,
+ .abort_cmd = pn532_uart_abort_cmd,
+ .dev_up = pn532_dev_up,
+ .dev_down = pn532_dev_down,
+};
+
+static void pn532_cmd_timeout(struct timer_list *t)
+{
+ struct pn532_uart_phy *dev = from_timer(dev, t, cmd_timeout);
+
+ pn532_uart_send_frame(dev->priv, dev->cur_out_buf);
+}
+
+/*
+ * scans the buffer if it contains a pn532 frame. It is not checked if the
+ * frame is really valid. This is later done with pn533_rx_frame_is_valid.
+ * This is useful for malformed or errornous transmitted frames. Adjusts the
+ * bufferposition where the frame starts, since pn533_recv_frame expects a
+ * well formed frame.
+ */
+static int pn532_uart_rx_is_frame(struct sk_buff *skb)
+{
+ int i;
+ u16 frame_len;
+ struct pn533_std_frame *std;
+ struct pn533_ext_frame *ext;
+
+ for (i = 0; i + PN533_STD_FRAME_ACK_SIZE <= skb->len; i++) {
+ std = (struct pn533_std_frame *)&skb->data[i];
+ /* search start code */
+ if (std->start_frame != cpu_to_be16(PN533_STD_FRAME_SOF))
+ continue;
+
+ /* frame type */
+ switch (std->datalen) {
+ case PN533_FRAME_DATALEN_ACK:
+ if (std->datalen_checksum == 0xff) {
+ skb_pull(skb, i);
+ return 1;
+ }
+
+ break;
+ case PN533_FRAME_DATALEN_ERROR:
+ if ((std->datalen_checksum == 0xff) &&
+ (skb->len >=
+ PN533_STD_ERROR_FRAME_SIZE)) {
+ skb_pull(skb, i);
+ return 1;
+ }
+
+ break;
+ case PN533_FRAME_DATALEN_EXTENDED:
+ ext = (struct pn533_ext_frame *)&skb->data[i];
+ frame_len = ext->datalen;
+ if (skb->len >= frame_len +
+ sizeof(struct pn533_ext_frame) +
+ 2 /* CKS + Postamble */) {
+ skb_pull(skb, i);
+ return 1;
+ }
+
+ break;
+ default: /* normal information frame */
+ frame_len = std->datalen;
+ if (skb->len >= frame_len +
+ sizeof(struct pn533_std_frame) +
+ 2 /* CKS + Postamble */) {
+ skb_pull(skb, i);
+ return 1;
+ }
+
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static int pn532_receive_buf(struct serdev_device *serdev,
+ const unsigned char *data, size_t count)
+{
+ struct pn532_uart_phy *dev = serdev_device_get_drvdata(serdev);
+ size_t i;
+
+ del_timer(&dev->cmd_timeout);
+ for (i = 0; i < count; i++) {
+ skb_put_u8(dev->recv_skb, *data++);
+ if (!pn532_uart_rx_is_frame(dev->recv_skb))
+ continue;
+
+ pn533_recv_frame(dev->priv, dev->recv_skb, 0);
+ dev->recv_skb = alloc_skb(PN532_UART_SKB_BUFF_LEN, GFP_KERNEL);
+ if (!dev->recv_skb)
+ return 0;
+ }
+
+ return i;
+}
+
+static struct serdev_device_ops pn532_serdev_ops = {
+ .receive_buf = pn532_receive_buf,
+ .write_wakeup = serdev_device_write_wakeup,
+};
+
+static const struct of_device_id pn532_uart_of_match[] = {
+ { .compatible = "nxp,pn532", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, pn532_uart_of_match);
+
+static int pn532_uart_probe(struct serdev_device *serdev)
+{
+ struct pn532_uart_phy *pn532;
+ struct pn533 *priv;
+ int err;
+
+ err = -ENOMEM;
+ pn532 = kzalloc(sizeof(*pn532), GFP_KERNEL);
+ if (!pn532)
+ goto err_exit;
+
+ pn532->recv_skb = alloc_skb(PN532_UART_SKB_BUFF_LEN, GFP_KERNEL);
+ if (!pn532->recv_skb)
+ goto err_free;
+
+ pn532->serdev = serdev;
+ serdev_device_set_drvdata(serdev, pn532);
+ serdev_device_set_client_ops(serdev, &pn532_serdev_ops);
+ err = serdev_device_open(serdev);
+ if (err) {
+ dev_err(&serdev->dev, "Unable to open device\n");
+ goto err_skb;
+ }
+
+ err = serdev_device_set_baudrate(serdev, 115200);
+ if (err != 115200) {
+ err = -EINVAL;
+ goto err_serdev;
+ }
+
+ serdev_device_set_flow_control(serdev, false);
+ pn532->send_wakeup = PN532_SEND_WAKEUP;
+ timer_setup(&pn532->cmd_timeout, pn532_cmd_timeout, 0);
+ priv = pn53x_common_init(PN533_DEVICE_PN532,
+ PN533_PROTO_REQ_ACK_RESP,
+ pn532, &uart_phy_ops, NULL,
+ &pn532->serdev->dev);
+ if (IS_ERR(priv)) {
+ err = PTR_ERR(priv);
+ goto err_serdev;
+ }
+
+ pn532->priv = priv;
+ err = pn533_finalize_setup(pn532->priv);
+ if (err)
+ goto err_clean;
+
+ serdev_device_close(serdev);
+ err = pn53x_register_nfc(priv, PN533_NO_TYPE_B_PROTOCOLS, &serdev->dev);
+ if (err) {
+ pn53x_common_clean(pn532->priv);
+ goto err_skb;
+ }
+
+ return err;
+
+err_clean:
+ pn53x_common_clean(pn532->priv);
+err_serdev:
+ serdev_device_close(serdev);
+err_skb:
+ kfree_skb(pn532->recv_skb);
+err_free:
+ kfree(pn532);
+err_exit:
+ return err;
+}
+
+static void pn532_uart_remove(struct serdev_device *serdev)
+{
+ struct pn532_uart_phy *pn532 = serdev_device_get_drvdata(serdev);
+
+ pn53x_unregister_nfc(pn532->priv);
+ serdev_device_close(serdev);
+ pn53x_common_clean(pn532->priv);
+ kfree_skb(pn532->recv_skb);
+ kfree(pn532);
+}
+
+static struct serdev_device_driver pn532_uart_driver = {
+ .probe = pn532_uart_probe,
+ .remove = pn532_uart_remove,
+ .driver = {
+ .name = "pn532_uart",
+ .of_match_table = of_match_ptr(pn532_uart_of_match),
+ },
+};
+
+module_serdev_device_driver(pn532_uart_driver);
+
+MODULE_AUTHOR("Lars Pöschel <poeschel@lemonage.de>");
+MODULE_DESCRIPTION("PN532 UART driver");
+MODULE_LICENSE("GPL");
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH v6 6/7] nfc: pn533: Add autopoll capability
From: Lars Poeschel @ 2019-08-20 12:03 UTC (permalink / raw)
To: Allison Randal, Steve Winslow, Greg Kroah-Hartman,
Gustavo A. R. Silva, Kate Stewart, Lars Poeschel, Thomas Gleixner,
open list:NFC SUBSYSTEM, open list
Cc: Johan Hovold
In-Reply-To: <20190820120345.22593-1-poeschel@lemonage.de>
pn532 devices support an autopoll command, that lets the chip
automatically poll for selected nfc technologies instead of manually
looping through every single nfc technology the user is interested in.
This is faster and less cpu and bus intensive than manually polling.
This adds this autopoll capability to the pn533 driver.
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v6:
- Rebased the patch series on v5.3-rc5
drivers/nfc/pn533/pn533.c | 193 +++++++++++++++++++++++++++++++++++++-
drivers/nfc/pn533/pn533.h | 10 +-
2 files changed, 197 insertions(+), 6 deletions(-)
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index a8c756caa678..7e915239222b 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -185,6 +185,32 @@ struct pn533_cmd_jump_dep_response {
u8 gt[];
} __packed;
+struct pn532_autopoll_resp {
+ u8 type;
+ u8 ln;
+ u8 tg;
+ u8 tgdata[];
+} __packed;
+
+/* PN532_CMD_IN_AUTOPOLL */
+#define PN532_AUTOPOLL_POLLNR_INFINITE 0xff
+#define PN532_AUTOPOLL_PERIOD 0x03 /* in units of 150 ms */
+
+#define PN532_AUTOPOLL_TYPE_GENERIC_106 0x00
+#define PN532_AUTOPOLL_TYPE_GENERIC_212 0x01
+#define PN532_AUTOPOLL_TYPE_GENERIC_424 0x02
+#define PN532_AUTOPOLL_TYPE_JEWEL 0x04
+#define PN532_AUTOPOLL_TYPE_MIFARE 0x10
+#define PN532_AUTOPOLL_TYPE_FELICA212 0x11
+#define PN532_AUTOPOLL_TYPE_FELICA424 0x12
+#define PN532_AUTOPOLL_TYPE_ISOA 0x20
+#define PN532_AUTOPOLL_TYPE_ISOB 0x23
+#define PN532_AUTOPOLL_TYPE_DEP_PASSIVE_106 0x40
+#define PN532_AUTOPOLL_TYPE_DEP_PASSIVE_212 0x41
+#define PN532_AUTOPOLL_TYPE_DEP_PASSIVE_424 0x42
+#define PN532_AUTOPOLL_TYPE_DEP_ACTIVE_106 0x80
+#define PN532_AUTOPOLL_TYPE_DEP_ACTIVE_212 0x81
+#define PN532_AUTOPOLL_TYPE_DEP_ACTIVE_424 0x82
/* PN533_TG_INIT_AS_TARGET */
#define PN533_INIT_TARGET_PASSIVE 0x1
@@ -1389,6 +1415,101 @@ static int pn533_poll_dep(struct nfc_dev *nfc_dev)
return rc;
}
+static int pn533_autopoll_complete(struct pn533 *dev, void *arg,
+ struct sk_buff *resp)
+{
+ u8 nbtg;
+ int rc;
+ struct pn532_autopoll_resp *apr;
+ struct nfc_target nfc_tgt;
+
+ if (IS_ERR(resp)) {
+ rc = PTR_ERR(resp);
+
+ nfc_err(dev->dev, "%s autopoll complete error %d\n",
+ __func__, rc);
+
+ if (rc == -ENOENT) {
+ if (dev->poll_mod_count != 0)
+ return rc;
+ goto stop_poll;
+ } else if (rc < 0) {
+ nfc_err(dev->dev,
+ "Error %d when running autopoll\n", rc);
+ goto stop_poll;
+ }
+ }
+
+ nbtg = resp->data[0];
+ if ((nbtg > 2) || (nbtg <= 0))
+ return -EAGAIN;
+
+ apr = (struct pn532_autopoll_resp *)&resp->data[1];
+ while (nbtg--) {
+ memset(&nfc_tgt, 0, sizeof(struct nfc_target));
+ switch (apr->type) {
+ case PN532_AUTOPOLL_TYPE_ISOA:
+ dev_dbg(dev->dev, "ISOA");
+ rc = pn533_target_found_type_a(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ case PN532_AUTOPOLL_TYPE_FELICA212:
+ case PN532_AUTOPOLL_TYPE_FELICA424:
+ dev_dbg(dev->dev, "FELICA");
+ rc = pn533_target_found_felica(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ case PN532_AUTOPOLL_TYPE_JEWEL:
+ dev_dbg(dev->dev, "JEWEL");
+ rc = pn533_target_found_jewel(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ case PN532_AUTOPOLL_TYPE_ISOB:
+ dev_dbg(dev->dev, "ISOB");
+ rc = pn533_target_found_type_b(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ case PN532_AUTOPOLL_TYPE_MIFARE:
+ dev_dbg(dev->dev, "Mifare");
+ rc = pn533_target_found_type_a(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ default:
+ nfc_err(dev->dev,
+ "Unknown current poll modulation");
+ rc = -EPROTO;
+ }
+
+ if (rc)
+ goto done;
+
+ if (!(nfc_tgt.supported_protocols & dev->poll_protocols)) {
+ nfc_err(dev->dev,
+ "The Tg found doesn't have the desired protocol");
+ rc = -EAGAIN;
+ goto done;
+ }
+
+ dev->tgt_available_prots = nfc_tgt.supported_protocols;
+ apr = (struct pn532_autopoll_resp *)
+ (apr->tgdata + (apr->ln - 1));
+ }
+
+ pn533_poll_reset_mod_list(dev);
+ nfc_targets_found(dev->nfc_dev, &nfc_tgt, 1);
+
+done:
+ dev_kfree_skb(resp);
+ return rc;
+
+stop_poll:
+ nfc_err(dev->dev, "autopoll operation has been stopped\n");
+
+ pn533_poll_reset_mod_list(dev);
+ dev->poll_protocols = 0;
+ return rc;
+}
+
static int pn533_poll_complete(struct pn533 *dev, void *arg,
struct sk_buff *resp)
{
@@ -1534,6 +1655,7 @@ static int pn533_start_poll(struct nfc_dev *nfc_dev,
struct pn533_poll_modulations *cur_mod;
u8 rand_mod;
int rc;
+ struct sk_buff *skb;
dev_dbg(dev->dev,
"%s: im protocols 0x%x tm protocols 0x%x\n",
@@ -1557,9 +1679,73 @@ static int pn533_start_poll(struct nfc_dev *nfc_dev,
tm_protocols = 0;
}
- pn533_poll_create_mod_list(dev, im_protocols, tm_protocols);
dev->poll_protocols = im_protocols;
dev->listen_protocols = tm_protocols;
+ if (dev->device_type == PN533_DEVICE_PN532_AUTOPOLL) {
+ skb = pn533_alloc_skb(dev, 4 + 6);
+ if (!skb)
+ return -ENOMEM;
+
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_POLLNR_INFINITE;
+ *((u8 *)skb_put(skb, sizeof(u8))) = PN532_AUTOPOLL_PERIOD;
+
+ if ((im_protocols & NFC_PROTO_MIFARE_MASK) &&
+ (im_protocols & NFC_PROTO_ISO14443_MASK) &&
+ (im_protocols & NFC_PROTO_NFC_DEP_MASK))
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_GENERIC_106;
+ else {
+ if (im_protocols & NFC_PROTO_MIFARE_MASK)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_MIFARE;
+
+ if (im_protocols & NFC_PROTO_ISO14443_MASK)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_ISOA;
+
+ if (im_protocols & NFC_PROTO_NFC_DEP_MASK) {
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_DEP_PASSIVE_106;
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_DEP_PASSIVE_212;
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_DEP_PASSIVE_424;
+ }
+ }
+
+ if (im_protocols & NFC_PROTO_FELICA_MASK ||
+ im_protocols & NFC_PROTO_NFC_DEP_MASK) {
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_FELICA212;
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_FELICA424;
+ }
+
+ if (im_protocols & NFC_PROTO_JEWEL_MASK)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_JEWEL;
+
+ if (im_protocols & NFC_PROTO_ISO14443_B_MASK)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_ISOB;
+
+ if (tm_protocols)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_DEP_ACTIVE_106;
+
+ rc = pn533_send_cmd_async(dev, PN533_CMD_IN_AUTOPOLL, skb,
+ pn533_autopoll_complete, NULL);
+
+ if (rc < 0)
+ dev_kfree_skb(skb);
+ else
+ dev->poll_mod_count++;
+
+ return rc;
+ }
+
+ pn533_poll_create_mod_list(dev, im_protocols, tm_protocols);
/* Do not always start polling from the same modulation */
get_random_bytes(&rand_mod, sizeof(rand_mod));
@@ -2461,7 +2647,8 @@ static int pn533_dev_up(struct nfc_dev *nfc_dev)
if (dev->phy_ops->dev_up)
dev->phy_ops->dev_up(dev);
- if (dev->device_type == PN533_DEVICE_PN532) {
+ if ((dev->device_type == PN533_DEVICE_PN532) ||
+ (dev->device_type == PN533_DEVICE_PN532_AUTOPOLL)) {
int rc = pn532_sam_configuration(nfc_dev);
if (rc)
@@ -2508,6 +2695,7 @@ static int pn533_setup(struct pn533 *dev)
case PN533_DEVICE_PASORI:
case PN533_DEVICE_ACR122U:
case PN533_DEVICE_PN532:
+ case PN533_DEVICE_PN532_AUTOPOLL:
max_retries.mx_rty_atr = 0x2;
max_retries.mx_rty_psl = 0x1;
max_retries.mx_rty_passive_act =
@@ -2544,6 +2732,7 @@ static int pn533_setup(struct pn533 *dev)
switch (dev->device_type) {
case PN533_DEVICE_STD:
case PN533_DEVICE_PN532:
+ case PN533_DEVICE_PN532_AUTOPOLL:
break;
case PN533_DEVICE_PASORI:
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 6541088fad73..388fc1b4fcc1 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -6,10 +6,11 @@
* Copyright (C) 2012-2013 Tieto Poland
*/
-#define PN533_DEVICE_STD 0x1
-#define PN533_DEVICE_PASORI 0x2
-#define PN533_DEVICE_ACR122U 0x3
-#define PN533_DEVICE_PN532 0x4
+#define PN533_DEVICE_STD 0x1
+#define PN533_DEVICE_PASORI 0x2
+#define PN533_DEVICE_ACR122U 0x3
+#define PN533_DEVICE_PN532 0x4
+#define PN533_DEVICE_PN532_AUTOPOLL 0x5
#define PN533_ALL_PROTOCOLS (NFC_PROTO_JEWEL_MASK | NFC_PROTO_MIFARE_MASK |\
NFC_PROTO_FELICA_MASK | NFC_PROTO_ISO14443_MASK |\
@@ -75,6 +76,7 @@
#define PN533_CMD_IN_ATR 0x50
#define PN533_CMD_IN_RELEASE 0x52
#define PN533_CMD_IN_JUMP_FOR_DEP 0x56
+#define PN533_CMD_IN_AUTOPOLL 0x60
#define PN533_CMD_TG_INIT_AS_TARGET 0x8c
#define PN533_CMD_TG_GET_DATA 0x86
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH v6 7/7] nfc: pn532_uart: Make use of pn532 autopoll
From: Lars Poeschel @ 2019-08-20 12:03 UTC (permalink / raw)
To: GitAuthor: Lars Poeschel, open list:NFC SUBSYSTEM, open list; +Cc: Johan Hovold
In-Reply-To: <20190820120345.22593-1-poeschel@lemonage.de>
This switches the pn532 UART phy driver from manually polling to the new
autopoll mechanism.
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v6:
- Rebased the patch series on v5.3-rc5
drivers/nfc/pn533/uart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nfc/pn533/uart.c b/drivers/nfc/pn533/uart.c
index f1cc2354a4fd..e3085e5b2d4c 100644
--- a/drivers/nfc/pn533/uart.c
+++ b/drivers/nfc/pn533/uart.c
@@ -254,7 +254,7 @@ static int pn532_uart_probe(struct serdev_device *serdev)
serdev_device_set_flow_control(serdev, false);
pn532->send_wakeup = PN532_SEND_WAKEUP;
timer_setup(&pn532->cmd_timeout, pn532_cmd_timeout, 0);
- priv = pn53x_common_init(PN533_DEVICE_PN532,
+ priv = pn53x_common_init(PN533_DEVICE_PN532_AUTOPOLL,
PN533_PROTO_REQ_ACK_RESP,
pn532, &uart_phy_ops, NULL,
&pn532->serdev->dev);
--
2.23.0.rc1
^ permalink raw reply related
* Re: [PATCH REPOST 1/2] can: flexcan: fix deadlock when using self wakeup
From: Sean Nyekjaer @ 2019-08-20 11:55 UTC (permalink / raw)
To: Joakim Zhang, mkl@pengutronix.de, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org, dl-linux-imx,
Martin Hundebøll
In-Reply-To: <DB7PR04MB4618A1F984F2281C66959B06E6AB0@DB7PR04MB4618.eurprd04.prod.outlook.com>
>> Unfortunatly it's still possible to reproduce the deadlock with this patch...
>>
>> [ 689.921717] flexcan: probe of 2094000.flexcan failed with error -110
>>
>> My test setup:
>> PC with CAN-USB dongle connected to can0 and can1.
>>
>> PC:
>> $ while true; do cansend can0 '123#DEADBEEF'; done
>>
>> iMX6ull:
>> root@iwg26:~# systemctl suspend
>>
>>
>> [ 365.858054] systemd[1]: Reached target Sleep.
>> root@iwg26:~# [ 365.939826] systemd[1]: Starting Suspend...
>> [ 366.115839] systemd-sleep[248]: Suspending system...
>> [ 366.517949] dpm_run_callback(): platform_pm_suspend+0x0/0x5c returns
>> -110 [ 366.518249] PM: Device 2094000.flexcan failed to suspend: error -110
>> [ 366.518406] PM: Some devices failed to suspend, or early wake event
>> detected [ 366.732162] dpm_run_callback():
>> platform_pm_suspend+0x0/0x5c returns -110 [ 366.732285] PM: Device
>> 2090000.flexcan failed to suspend: error -110 [ 366.732330] PM: Some
>> devices failed to suspend, or early wake event detected [ 366.890637]
>> systemd-sleep[248]: System resumed.
>
> CAN1, CAN0 suspended failed, then CAN0, CAN1 resumed back, so CAN0/CAN1 can work fine.
>
>> [ 366.923062] systemd[1]: Started Suspend.
>> [ 366.942819] systemd[1]: sleep.target: Unit not needed anymore. Stopping.
>> [ 366.954791] systemd[1]: Stopped target Sleep.
>> [ 366.962402] systemd[1]: Reached target Suspend.
>> [ 366.977546] systemd-logind[135]: Operation 'sleep' finished.
>> [ 366.979194] systemd[1]: suspend.target: Unit not needed anymore.
>> Stopping.
>> [ 366.993831] systemd[1]: Stopped target Suspend.
>> [ 367.139972] systemd-networkd[220]: usb0: Lost carrier [ 367.294077]
>> systemd-networkd[220]: usb0: Gained carrier
>>
>> root@iwg26:~# candump can0 | head -n 2
>>
>> can0 123 [4] DE AD BE EF
>> can0 123 [4] DE AD BE EF
>> root@iwg26:~# candump can1 | head -n 2
>>
>> can1 123 [4] DE AD BE EF
>> can1 123 [4] DE AD BE EF
>> root@iwg26:~# systemctl suspend
>>
>> root@iwg26:~# [ 385.106658] systemd[1]: Reached target Sleep.
>> [ 385.147602] systemd[1]: Starting Suspend...
>> [ 385.246421] systemd-sleep[260]: Suspending system...
>> [ 385.634733] dpm_run_callback(): platform_pm_suspend+0x0/0x5c returns
>> -110 [ 385.634855] PM: Device 2090000.flexcan failed to suspend: error -110
>> [ 385.634897] PM: Some devices failed to suspend, or early wake event
>> detected [ 385.856251] PM: noirq suspend of devices failed [ 385.998364]
>> systemd-sleep[260]: System resumed.
>
> CAN0 suspended failed, CAN1 noirq suspended failed, then CAN1, CAN0 resumed back, so CAN0/CAN1 can work fine.
>
>> [ 386.023390] systemd[1]: Started Suspend.
>> [ 386.031570] systemd[1]: sleep.target: Unit not needed anymore. Stopping.
>> [ 386.055886] systemd[1]: Stopped target Sleep.
>> [ 386.061430] systemd[1]: Reached target Suspend.
>> [ 386.066142] systemd[1]: suspend.target: Unit not needed anymore.
>> Stopping.
>> [ 386.112575] systemd-networkd[220]: usb0: Lost carrier [ 386.116797]
>> systemd-logind[135]: Operation 'sleep' finished.
>> [ 386.146161] systemd[1]: Stopped target Suspend.
>> [ 386.260866] systemd-networkd[220]: usb0: Gained carrier root@iwg26:~#
>> candump can0 | head -n 2
>> can0 123 [4] DE AD BE EF
>> can0 123 [4] DE AD BE EF
>> root@iwg26:~# candump can1 | head -n 2
>>
>> can1 123 [4] DE AD BE EF
>> can1 123 [4] DE AD BE EF
>> root@iwg26:~# systemctl suspend
>>
>> [ 396.919303] systemd[1]: Reached target Sleep.
>> root@iwg26:~# [ 396.964722] systemd[1]: Starting Suspend...
>> [ 397.067336] systemd-sleep[268]: Suspending system...
>> [ 397.574571] PM: noirq suspend of devices failed [ 397.834731] PM: noirq
>> suspend of devices failed [ 397.807996] systemd-networkd[220]: usb0: Lost
>> carrier [ 398.156295] dpm_run_callback(): platform_pm_suspend+0x0/0x5c
>> returns -110 [ 398.156339] PM: Device 2094000.flexcan failed to suspend:
>> error -110 [ 398.156509] PM: Some devices failed to suspend, or early wake
>> event detected [ 398.053555] systemd-sleep[268]: Failed to write
>> /sys/power/state:
>> Device or resource busy
>
> But the log here is very strange and chaotic, it looks like CAN0 suspended failed, then resumed back, so CAN0 can work fine.
> CAN1 noirq suspend failed, but have not resumed back, so CAN1 still in stop mode, cannot work. I think this may be other device noirq suspend failed
> broke the resume of CAN1.
>
> Could you do more debug to help locate the issue?
>
>> [ 398.074751] systemd[1]: systemd-suspend.service: Main process exited,
>> code=exited, status=1/FAILURE [ 398.076779] systemd[1]:
>
>> systemd-suspend.service: Failed with result 'exit-code'.
>> [ 398.109255] systemd[1]: Failed to start Suspend.
>> [ 398.118704] systemd[1]: Dependency failed for Suspend.
>> [ 398.136283] systemd-logind[135]: Operation 'sleep' finished.
>> [ 398.137770] systemd[1]: suspend.target: Job suspend.target/start failed
>> with result 'dependency'.
>> [ 398.139105] systemd[1]: sleep.target: Unit not needed anymore. Stopping.
>> [ 398.167590] systemd[1]: Stopped target Sleep.
>> [ 398.201558] systemd-networkd[220]: usb0: Gained carrier
>
> Log here also strange.
>
> Best Regards,
> Joakim Zhang
>> root@iwg26:~# candump can0 | head -n 2
>> can0 123 [4] DE AD BE EF
>> can0 123 [4] DE AD BE EF
>> root@iwg26:~# candump can1 | head -n 2
>>
>> nothing on can1 anymore :-(
>>
>> root@iwg26:~# rmmod flexcan
>> [ 622.884746] systemd-networkd[220]: can1: Lost carrier [ 623.046766]
>> systemd-networkd[220]: can0: Lost carrier root@iwg26:~# insmod
>> /mnt/flexcan.ko [ 628.323981] flexcan 2094000.flexcan: registering netdev
>> failed
>>
>> and can1 fails to register with:
>> [ 628.347485] flexcan: probe of 2094000.flexcan failed with error -110
>>
>> /Sean
I have added some more debug, same test setup:
https://gist.github.com/sknsean/81208714de23aa3639d3e31dccb2f3e0
root@iwg26:~# systemctl suspend
...
https://gist.github.com/sknsean/2a786f1543305056d4de03d387872403
/Sean
^ permalink raw reply
* RE: [PATCH REPOST 1/2] can: flexcan: fix deadlock when using self wakeup
From: Joakim Zhang @ 2019-08-20 11:56 UTC (permalink / raw)
To: Sean Nyekjaer, mkl@pengutronix.de, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org, dl-linux-imx,
Martin Hundebøll
In-Reply-To: <DB7PR04MB4618A1F984F2281C66959B06E6AB0@DB7PR04MB4618.eurprd04.prod.outlook.com>
> -----Original Message-----
> From: Joakim Zhang
> Sent: 2019年8月20日 19:25
> To: Sean Nyekjaer <sean@geanix.com>; mkl@pengutronix.de;
> linux-can@vger.kernel.org
> Cc: wg@grandegger.com; netdev@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>; Martin Hundebøll <martin@geanix.com>
> Subject: RE: [PATCH REPOST 1/2] can: flexcan: fix deadlock when using self
> wakeup
>
>
> > -----Original Message-----
> > From: Sean Nyekjaer <sean@geanix.com>
> > Sent: 2019年8月20日 18:25
> > To: Joakim Zhang <qiangqing.zhang@nxp.com>; mkl@pengutronix.de;
> > linux-can@vger.kernel.org
> > Cc: wg@grandegger.com; netdev@vger.kernel.org; dl-linux-imx
> > <linux-imx@nxp.com>; Martin Hundebøll <martin@geanix.com>
> > Subject: Re: [PATCH REPOST 1/2] can: flexcan: fix deadlock when using
> > self wakeup
> >
> >
> >
> > On 16/08/2019 10.20, Joakim Zhang wrote:
> > > As reproted by Sean Nyekjaer below:
> > > When suspending, when there is still can traffic on the interfaces
> > > the flexcan immediately wakes the platform again. As it should :-).
> > > But it throws this error msg:
> > > [ 3169.378661] PM: noirq suspend of devices failed
> > >
> > > On the way down to suspend the interface that throws the error
> > > message does call flexcan_suspend but fails to call flexcan_noirq_suspend.
> > > That means the flexcan_enter_stop_mode is called, but on the way out
> > > of suspend the driver only calls flexcan_resume and skips
> > > flexcan_noirq_resume, thus it doesn't call flexcan_exit_stop_mode.
> > > This leaves the flexcan in stop mode, and with the current driver it
> > > can't recover from this even with a soft reboot, it requires a hard reboot.
> > >
> > > The best way to exit stop mode is in Wake Up interrupt context, and
> > > then
> > > suspend() and resume() functions can be symmetric. However, stop
> > > mode request and ack will be controlled by SCU(System Control Unit)
> > > firmware(manage clock,power,stop mode, etc. by Cortex-M4 core) in
> > > coming i.MX8(QM/QXP). And SCU firmware interface can't be available
> > > in
> > interrupt context.
> > >
> > > For compatibillity, the wake up mechanism can't be symmetric, so we
> > > need in_stop_mode hack.
> > >
> > > Fixes: de3578c198c6 ("can: flexcan: add self wakeup support")
> > > Reported-by: Sean Nyekjaer <sean@geanix.com>
> > > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > >
> >
> > Unfortunatly it's still possible to reproduce the deadlock with this patch...
> >
> > [ 689.921717] flexcan: probe of 2094000.flexcan failed with error
> > -110
> >
> > My test setup:
> > PC with CAN-USB dongle connected to can0 and can1.
> >
> > PC:
> > $ while true; do cansend can0 '123#DEADBEEF'; done
> >
> > iMX6ull:
> > root@iwg26:~# systemctl suspend
> >
> >
> > [ 365.858054] systemd[1]: Reached target Sleep.
> > root@iwg26:~# [ 365.939826] systemd[1]: Starting Suspend...
> > [ 366.115839] systemd-sleep[248]: Suspending system...
> > [ 366.517949] dpm_run_callback(): platform_pm_suspend+0x0/0x5c
> > returns
> > -110 [ 366.518249] PM: Device 2094000.flexcan failed to suspend:
> > error -110 [ 366.518406] PM: Some devices failed to suspend, or early
> > wake event detected [ 366.732162] dpm_run_callback():
> > platform_pm_suspend+0x0/0x5c returns -110 [ 366.732285] PM: Device
> > 2090000.flexcan failed to suspend: error -110 [ 366.732330] PM: Some
> > devices failed to suspend, or early wake event detected [ 366.890637]
> > systemd-sleep[248]: System resumed.
>
> CAN1, CAN0 suspended failed, then CAN0, CAN1 resumed back, so
> CAN0/CAN1 can work fine.
>
> > [ 366.923062] systemd[1]: Started Suspend.
> > [ 366.942819] systemd[1]: sleep.target: Unit not needed anymore.
> Stopping.
> > [ 366.954791] systemd[1]: Stopped target Sleep.
> > [ 366.962402] systemd[1]: Reached target Suspend.
> > [ 366.977546] systemd-logind[135]: Operation 'sleep' finished.
> > [ 366.979194] systemd[1]: suspend.target: Unit not needed anymore.
> > Stopping.
> > [ 366.993831] systemd[1]: Stopped target Suspend.
> > [ 367.139972] systemd-networkd[220]: usb0: Lost carrier [
> > 367.294077]
> > systemd-networkd[220]: usb0: Gained carrier
> >
> > root@iwg26:~# candump can0 | head -n 2
> >
> > can0 123 [4] DE AD BE EF
> > can0 123 [4] DE AD BE EF
> > root@iwg26:~# candump can1 | head -n 2
> >
> > can1 123 [4] DE AD BE EF
> > can1 123 [4] DE AD BE EF
> > root@iwg26:~# systemctl suspend
> >
> > root@iwg26:~# [ 385.106658] systemd[1]: Reached target Sleep.
> > [ 385.147602] systemd[1]: Starting Suspend...
> > [ 385.246421] systemd-sleep[260]: Suspending system...
> > [ 385.634733] dpm_run_callback(): platform_pm_suspend+0x0/0x5c
> > returns
> > -110 [ 385.634855] PM: Device 2090000.flexcan failed to suspend:
> > error -110 [ 385.634897] PM: Some devices failed to suspend, or early
> > wake event detected [ 385.856251] PM: noirq suspend of devices failed
> > [ 385.998364]
> > systemd-sleep[260]: System resumed.
>
> CAN0 suspended failed, CAN1 noirq suspended failed, then CAN1, CAN0
> resumed back, so CAN0/CAN1 can work fine.
If CAN0 suspended failed, should system resumed after suspended all devices, should not enter noirq suspend,
why it here printed "PM: noirq suspend of devices failed"?
> > [ 386.023390] systemd[1]: Started Suspend.
> > [ 386.031570] systemd[1]: sleep.target: Unit not needed anymore.
> Stopping.
> > [ 386.055886] systemd[1]: Stopped target Sleep.
> > [ 386.061430] systemd[1]: Reached target Suspend.
> > [ 386.066142] systemd[1]: suspend.target: Unit not needed anymore.
> > Stopping.
> > [ 386.112575] systemd-networkd[220]: usb0: Lost carrier [
> > 386.116797]
> > systemd-logind[135]: Operation 'sleep' finished.
> > [ 386.146161] systemd[1]: Stopped target Suspend.
> > [ 386.260866] systemd-networkd[220]: usb0: Gained carrier
> > root@iwg26:~# candump can0 | head -n 2
> > can0 123 [4] DE AD BE EF
> > can0 123 [4] DE AD BE EF
> > root@iwg26:~# candump can1 | head -n 2
> >
> > can1 123 [4] DE AD BE EF
> > can1 123 [4] DE AD BE EF
> > root@iwg26:~# systemctl suspend
> >
> > [ 396.919303] systemd[1]: Reached target Sleep.
> > root@iwg26:~# [ 396.964722] systemd[1]: Starting Suspend...
> > [ 397.067336] systemd-sleep[268]: Suspending system...
> > [ 397.574571] PM: noirq suspend of devices failed [ 397.834731] PM:
> > noirq suspend of devices failed [ 397.807996] systemd-networkd[220]:
> > usb0: Lost carrier [ 398.156295] dpm_run_callback():
> > platform_pm_suspend+0x0/0x5c returns -110 [ 398.156339] PM: Device
> 2094000.flexcan failed to suspend:
> > error -110 [ 398.156509] PM: Some devices failed to suspend, or early
> > wake event detected [ 398.053555] systemd-sleep[268]: Failed to write
> > /sys/power/state:
> > Device or resource busy
>
> But the log here is very strange and chaotic, it looks like CAN0 suspended failed,
> then resumed back, so CAN0 can work fine.
> CAN1 noirq suspend failed, but have not resumed back, so CAN1 still in stop
> mode, cannot work. I think this may be other device noirq suspend failed broke
> the resume of CAN1.
>
> Could you do more debug to help locate the issue?
More strange, why here first enter noirq suspend?
Best Regards,
Joakim Zhang
> > [ 398.074751] systemd[1]: systemd-suspend.service: Main process
> > exited, code=exited, status=1/FAILURE [ 398.076779] systemd[1]:
>
> > systemd-suspend.service: Failed with result 'exit-code'.
> > [ 398.109255] systemd[1]: Failed to start Suspend.
> > [ 398.118704] systemd[1]: Dependency failed for Suspend.
> > [ 398.136283] systemd-logind[135]: Operation 'sleep' finished.
> > [ 398.137770] systemd[1]: suspend.target: Job suspend.target/start
> > failed with result 'dependency'.
> > [ 398.139105] systemd[1]: sleep.target: Unit not needed anymore.
> Stopping.
> > [ 398.167590] systemd[1]: Stopped target Sleep.
> > [ 398.201558] systemd-networkd[220]: usb0: Gained carrier
>
> Log here also strange.
>
> Best Regards,
> Joakim Zhang
> > root@iwg26:~# candump can0 | head -n 2
> > can0 123 [4] DE AD BE EF
> > can0 123 [4] DE AD BE EF
> > root@iwg26:~# candump can1 | head -n 2
> >
> > nothing on can1 anymore :-(
> >
> > root@iwg26:~# rmmod flexcan
> > [ 622.884746] systemd-networkd[220]: can1: Lost carrier [
> > 623.046766]
> > systemd-networkd[220]: can0: Lost carrier root@iwg26:~# insmod
> > /mnt/flexcan.ko [ 628.323981] flexcan 2094000.flexcan: registering
> > netdev failed
> >
> > and can1 fails to register with:
> > [ 628.347485] flexcan: probe of 2094000.flexcan failed with error
> > -110
> >
> > /Sean
^ 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