* Re: [Intel-wired-lan] [RFC PATCH next 2/2] i40e: add support for macvlan hardware offload
From: Shannon Nelson @ 2017-10-17 23:12 UTC (permalink / raw)
To: Alexander Duyck; +Cc: intel-wired-lan, Jeff Kirsher, Netdev
In-Reply-To: <CAKgT0Uft_kCyct==am7W0TGUo-YzUhSVkJir83uGx_XwMuxsHg@mail.gmail.com>
On 10/17/2017 2:32 PM, Alexander Duyck wrote:
>
> So the select_queue function being needed is the deal breaker on all
> of this as far as I am concerned. We aren't allowed to use it under
> other cases so why should macvlan be an exception to the rule?
I realize that the stack is pretty good at chosing the "right" queue,
which is my understanding as to why we shouldn't use select_queue(), but
it doesn't know how to use the accel_priv context associated with the
macvlan offload.
I saw DaveM's guidance to the HiNIC folks when they tried to add
select_queue(): "do not implement this function unless you absolutely
need to do something custom in your driver". I can see where this might
be the exception.
When originally thinking about how to do this, I wanted to use the
accel_priv as a pointer to the VSI to be used for the offload, then we
could have multiple queues and use all the VSI specific tuning
operations that XL710 has available. It can work when selecting the
queue, but by the time you get to start_xmit(), you no longer have that
context and only have the queue number. You can't do any fancy encoding
in the queue number because the value has to be within
dev->num_tx_queues. Maybe we can add accel_priv to the start_xmit
interface? (I can hear the groans already...)
However... for our case, you might be right anyway. If the stack is
doing its job at keeping the conversation on the one queue/irq/cpu
combination, any Tx following the offloaded Rx might already be headed
for the right Tx queue. I'll check on that.
> I think we should probably look at a different approach for this. For
> example why is it we need to use a different transmit path for a
> macvlan packet vs any other packet? On the Rx side we get the
> advantage of avoiding the software hashing and demux. What do we get
> for reserving queues for transmit?
There are a couple of reasons I can think of to keep the Tx on the
specific queue pair:
- Keep the Tx traffic on the same CPU and irq as the Rx traffic
- Don't let the flow get interrupted, slowed, or otherwise perturbed by
other traffic flows.
- Allow for adding hardware assisted bandwidth constraints to the
offloaded flow without bothering the rest of the NIC's traffic
Are these enough to want to guarantee the Tx queue?
> My plan for this is to go back and "fix" ixgbe so we can get it away
> from having to use the select_queue call for the macvlan offload and
> then maybe look at proving a few select NDO operations for allowing
> macvlans that are being offloaded to make specific calls into the
> hardware to perform tasks as needed.
The ixgbe implementation can certainly be improved. I think its biggest
failing is that the rest of the general traffic gets constrained to a
single queue - no more RSS for load balancing.
sln
^ permalink raw reply
* [PATCH] net: mac80211: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2017-10-17 23:14 UTC (permalink / raw)
To: Johannes Berg, David S. Miller
Cc: linux-wireless, netdev, linux-kernel, Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Notice that in some cases I replaced "fall through on else" and
"otherwise fall through" comments with just a "fall through" comment,
which is what GCC is expecting to find.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify that the actual intention of the code is to fall through.
net/mac80211/cfg.c | 3 +++
net/mac80211/ht.c | 1 +
net/mac80211/iface.c | 2 +-
net/mac80211/mesh.c | 2 ++
net/mac80211/mesh_hwmp.c | 1 +
net/mac80211/mesh_plink.c | 2 +-
net/mac80211/mlme.c | 1 +
net/mac80211/offchannel.c | 4 ++--
net/mac80211/tdls.c | 1 +
net/mac80211/wme.c | 1 +
10 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a354f19..9bd8bef 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -573,10 +573,12 @@ static int ieee80211_get_key(struct wiphy *wiphy, struct net_device *dev,
case WLAN_CIPHER_SUITE_BIP_CMAC_256:
BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) !=
offsetof(typeof(kseq), aes_cmac));
+ /* fall through */
case WLAN_CIPHER_SUITE_BIP_GMAC_128:
case WLAN_CIPHER_SUITE_BIP_GMAC_256:
BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) !=
offsetof(typeof(kseq), aes_gmac));
+ /* fall through */
case WLAN_CIPHER_SUITE_GCMP:
case WLAN_CIPHER_SUITE_GCMP_256:
BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) !=
@@ -2205,6 +2207,7 @@ static int ieee80211_scan(struct wiphy *wiphy,
* for now fall through to allow scanning only when
* beaconing hasn't been configured yet
*/
+ /* fall through */
case NL80211_IFTYPE_AP:
/*
* If the scan has been forced (and the driver supports
diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
index 41f5e48..e55dabf 100644
--- a/net/mac80211/ht.c
+++ b/net/mac80211/ht.c
@@ -491,6 +491,7 @@ int ieee80211_send_smps_action(struct ieee80211_sub_if_data *sdata,
case IEEE80211_SMPS_AUTOMATIC:
case IEEE80211_SMPS_NUM_MODES:
WARN_ON(1);
+ /* fall through */
case IEEE80211_SMPS_OFF:
action_frame->u.action.u.ht_smps.smps_control =
WLAN_HT_SMPS_CONTROL_DISABLED;
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 13b16f9..435e735 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1633,7 +1633,7 @@ static void ieee80211_assign_perm_addr(struct ieee80211_local *local,
goto out_unlock;
}
}
- /* otherwise fall through */
+ /* fall through */
default:
/* assign a new address if possible -- try n_addresses first */
for (i = 0; i < local->hw.wiphy->n_addresses; i++) {
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 7a76c4a..d29a545 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -988,8 +988,10 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
switch (sdata->vif.bss_conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
sta_flags |= IEEE80211_STA_DISABLE_HT;
+ /* fall through */
case NL80211_CHAN_WIDTH_20:
sta_flags |= IEEE80211_STA_DISABLE_40MHZ;
+ /* fall through */
case NL80211_CHAN_WIDTH_40:
sta_flags |= IEEE80211_STA_DISABLE_VHT;
break;
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 146ec6c..0e75abf 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -1247,6 +1247,7 @@ void mesh_path_tx_root_frame(struct ieee80211_sub_if_data *sdata)
break;
case IEEE80211_PROACTIVE_PREQ_WITH_PREP:
flags |= IEEE80211_PREQ_PROACTIVE_PREP_FLAG;
+ /* fall through */
case IEEE80211_PROACTIVE_PREQ_NO_PREP:
interval = ifmsh->mshcfg.dot11MeshHWMPactivePathToRootTimeout;
target_flags |= IEEE80211_PREQ_TO_FLAG |
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index e2d00cc..0f6c9ca 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -672,7 +672,7 @@ void mesh_plink_timer(struct timer_list *t)
break;
}
reason = WLAN_REASON_MESH_MAX_RETRIES;
- /* fall through on else */
+ /* fall through */
case NL80211_PLINK_CNF_RCVD:
/* confirm timer */
if (!reason)
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index e4ededa..f5f300fc4 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -473,6 +473,7 @@ static void ieee80211_add_ht_ie(struct ieee80211_sub_if_data *sdata,
case IEEE80211_SMPS_AUTOMATIC:
case IEEE80211_SMPS_NUM_MODES:
WARN_ON(1);
+ /* fall through */
case IEEE80211_SMPS_OFF:
cap |= WLAN_HT_CAP_SM_PS_DISABLED <<
IEEE80211_HT_CAP_SM_PS_SHIFT;
diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index faf4f60..f1d40b6 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -801,14 +801,14 @@ int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
case NL80211_IFTYPE_ADHOC:
if (!sdata->vif.bss_conf.ibss_joined)
need_offchan = true;
- /* fall through */
#ifdef CONFIG_MAC80211_MESH
+ /* fall through */
case NL80211_IFTYPE_MESH_POINT:
if (ieee80211_vif_is_mesh(&sdata->vif) &&
!sdata->u.mesh.mesh_id_len)
need_offchan = true;
- /* fall through */
#endif
+ /* fall through */
case NL80211_IFTYPE_AP:
case NL80211_IFTYPE_AP_VLAN:
case NL80211_IFTYPE_P2P_GO:
diff --git a/net/mac80211/tdls.c b/net/mac80211/tdls.c
index 91093d4..96d4fb9 100644
--- a/net/mac80211/tdls.c
+++ b/net/mac80211/tdls.c
@@ -236,6 +236,7 @@ static enum ieee80211_ac_numbers ieee80211_ac_from_wmm(int ac)
switch (ac) {
default:
WARN_ON_ONCE(1);
+ /* fall through */
case 0:
return IEEE80211_AC_BE;
case 1:
diff --git a/net/mac80211/wme.c b/net/mac80211/wme.c
index 3e3d301..5f7c963 100644
--- a/net/mac80211/wme.c
+++ b/net/mac80211/wme.c
@@ -165,6 +165,7 @@ u16 ieee80211_select_queue(struct ieee80211_sub_if_data *sdata,
qos = sta->sta.wme;
break;
}
+ /* fall through */
case NL80211_IFTYPE_AP:
ra = skb->data;
break;
--
2.7.4
^ permalink raw reply related
* [PATCH] dql: make dql_init return void
From: Stephen Hemminger @ 2017-10-18 0:16 UTC (permalink / raw)
To: Tom Herbert, Hiroaki SHIMODA, davem; +Cc: netdev, Stephen Hemminger
dql_init always returned 0, and the only place that uses it
in network core code didn't care about the return value anyway.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
include/linux/dynamic_queue_limits.h | 2 +-
lib/dynamic_queue_limits.c | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/include/linux/dynamic_queue_limits.h b/include/linux/dynamic_queue_limits.h
index a4be70398ce1..f69f98541953 100644
--- a/include/linux/dynamic_queue_limits.h
+++ b/include/linux/dynamic_queue_limits.h
@@ -98,7 +98,7 @@ void dql_completed(struct dql *dql, unsigned int count);
void dql_reset(struct dql *dql);
/* Initialize dql state */
-int dql_init(struct dql *dql, unsigned hold_time);
+void dql_init(struct dql *dql, unsigned int hold_time);
#endif /* _KERNEL_ */
diff --git a/lib/dynamic_queue_limits.c b/lib/dynamic_queue_limits.c
index f346715e2255..dbe61c4c2a97 100644
--- a/lib/dynamic_queue_limits.c
+++ b/lib/dynamic_queue_limits.c
@@ -127,12 +127,11 @@ void dql_reset(struct dql *dql)
}
EXPORT_SYMBOL(dql_reset);
-int dql_init(struct dql *dql, unsigned hold_time)
+void dql_init(struct dql *dql, unsigned int hold_time)
{
dql->max_limit = DQL_MAX_LIMIT;
dql->min_limit = 0;
dql->slack_hold_time = hold_time;
dql_reset(dql);
- return 0;
}
EXPORT_SYMBOL(dql_init);
--
2.11.0
^ permalink raw reply related
* Re: RFC(v2): Audit Kernel Container IDs
From: Steve Grubb @ 2017-10-18 0:23 UTC (permalink / raw)
To: James Bottomley
Cc: mszeredi-H+wXaHxf7aLQT0dZR+AlfA, Simo Sorce,
jlayton-H+wXaHxf7aLQT0dZR+AlfA, Carlos O'Donell, Linux API,
Linux Containers, Linux Kernel, Al Viro, David Howells,
Linux FS Devel, linux-audit-H+wXaHxf7aLQT0dZR+AlfA,
Eric W. Biederman, Andy Lutomirski,
cgroups-u79uwXL29TY76Z2rM5mHXA, Casey Schaufler, Eric Paris,
Linux Network Development, trondmy-7I+n7zu2hftEKMMhf/gKZA
In-Reply-To: <1508263063.3129.35.camel-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>
On Tuesday, October 17, 2017 1:57:43 PM EDT James Bottomley wrote:
> > > > The idea is that processes spawned into a container would be
> > > > labelled by the container orchestration system. It's unclear
> > > > what should happen to processes using nsenter after the fact, but
> > > > policy for that should be up to the orchestration system.
> > >
> > > I'm fine with that. The user space policy can be anything y'all
> > > like.
> >
> > I think there should be a login event.
>
> I thought you wanted this for containers? Container creation doesn't
> have login events. In an unprivileged orchestration system it may be
> hard to synthetically manufacture them.
I realize this. This work is very similar to problems we've solved 12 years
ago. We'll figure out what the right name is for it down the road. But the
concept is the same. If something enters a container, we need to know about
it. It needs to get tagged and be associated with the container. The way this
was solved for the loginuid problem was to add a session identifier so that
new logins of the same loginuid can coexist and we can trace actions back to a
specific login. I'd think we can apply lessons learned from a while back to
make container identification act similarly.
-Steve
^ permalink raw reply
* Re: [PATCH v3 net-next] tcp: Remove use of daddr_cache in tracepoint
From: Song Liu @ 2017-10-18 0:25 UTC (permalink / raw)
To: netdev; +Cc: eric.dumazet, xiyou.wangcong, dsahern, Song Liu
In-Reply-To: <1508276678.31614.118.camel.edumazet-glaptop3.roam.corp.google.com>
> > Remove use of ipv6_pinfo in favor of data in sock_common.
> >
> > Fixes: e086101b150a ("tcp: add a tracepoint for tcp retransmission")
> > Signed-off-by: David Ahern <dsahern@gmail.com>
> > ---
>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Thanks David !
Tested-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply
* RE: [patch net v3 2/4] net/sched: Use action array instead of action list as parameter
From: Chris Mi @ 2017-10-18 0:58 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Lucas Bates,
Jiri Pirko, David Miller
In-Reply-To: <CAM_iQpUz4bKhb2uUiBBeXL6f6BfdAND7sW+az5MjbjbifRnmhw@mail.gmail.com>
> -----Original Message-----
> From: Cong Wang [mailto:xiyou.wangcong@gmail.com]
> Sent: Wednesday, October 18, 2017 12:56 AM
> To: Chris Mi <chrism@mellanox.com>
> Cc: Linux Kernel Network Developers <netdev@vger.kernel.org>; Jamal Hadi
> Salim <jhs@mojatatu.com>; Lucas Bates <lucasb@mojatatu.com>; Jiri Pirko
> <jiri@resnulli.us>; David Miller <davem@davemloft.net>
> Subject: Re: [patch net v3 2/4] net/sched: Use action array instead of action
> list as parameter
>
> On Mon, Oct 16, 2017 at 6:20 PM, Chris Mi <chrism@mellanox.com> wrote:
> > When destroying filters, actions should be destroyed first.
> > The pointers of each action are saved in an array. TC doesn't use the
> > array directly, but put all actions in a doubly linked list and use
> > that list as parameter.
> >
> > There is no problem if each filter has its own actions. But if some
> > filters share the same action, when these filters are destroyed, RCU
> > callback fl_destroy_filter() may be called at the same time. That
> > means the same action's 'struct list_head list'
> > could be manipulated at the same time. It may point to an invalid
> > address so that system will panic.
>
> So if we remove these RCU callbacks (by adding a sychronize_rcu) this is not a
> problem, right?
Maybe you are right. But do you think it will cause performance issue, I mean it takes
longer time to destroy filters if using synchronize_rcu()?
Or is there any other races than RCU callbacks?
We haven't found them. This is the only one we found.
>
>
> >
> > This patch uses the action array directly to fix this issue.
> >
> > Fixes commit in pre-git era.
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>
> This is wrong too. RCU callbacks were introduced very late.
^ permalink raw reply
* RE: [patch net v2 1/4] net/sched: Change tc_action refcnt and bindcnt to atomic
From: Chris Mi @ 2017-10-18 1:03 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Lucas Bates,
Jiri Pirko, David Miller
In-Reply-To: <CAM_iQpWbfjCH0EFb8RYnOuZQtVtg-Mw-Kzv3GWq81qTVErN6hA@mail.gmail.com>
> -----Original Message-----
> From: Cong Wang [mailto:xiyou.wangcong@gmail.com]
> Sent: Tuesday, October 17, 2017 11:53 PM
> To: Chris Mi <chrism@mellanox.com>
> Cc: Linux Kernel Network Developers <netdev@vger.kernel.org>; Jamal Hadi
> Salim <jhs@mojatatu.com>; Lucas Bates <lucasb@mojatatu.com>; Jiri Pirko
> <jiri@resnulli.us>; David Miller <davem@davemloft.net>
> Subject: Re: [patch net v2 1/4] net/sched: Change tc_action refcnt and
> bindcnt to atomic
>
> On Mon, Oct 16, 2017 at 6:14 PM, Chris Mi <chrism@mellanox.com> wrote:
> > I don't think this bug were introduced by above two commits only.
> > Actually, this bug were introduced by several commits, at least the
> following:
> > 1. refcnt and bindcnt are not atomic
>
> Nope, it is perfectly okay with non-atomic as long as no parallel, and without
> RCU callback they are perfectly serialized by RTNL.
Agree.
>
>
> > 2. passing actions using list instead of arrays (I think initially we
> > are using arrays)
>
> We are discussing patch 1/4, this is patch 2/4, so irrelevant.
Agree.
>
>
> > 3. using RCU callbacks
>
> This introduces problem 1.
I think this patch set only fixes one problem, that's the race and the panic.
What do you mean by problem 1.
>
>
> > So instead of blaming the latest commit, it is better to say it is a pre-git error.
>
> You are wrong.
OK, you are right. But could I know what's your suggestion for this patch set?
1. reject it?
2. change the "Fixes" as you suggested?
3. something else?
Thanks,
Chris
^ permalink raw reply
* RE: [PATCH net v2 2/2] net: fec: Let fec_ptp have its own interrupt routine
From: Andy Duan @ 2017-10-18 2:30 UTC (permalink / raw)
To: Troy Kisky, shawn.guo@linaro.org, netdev@vger.kernel.org,
davem@davemloft.net
Cc: Fabio Estevam, lznuaa@gmail.com, andrew@lunn.ch
In-Reply-To: <542a126c-d7ff-7232-9dde-e5c6f1e1f55f@boundarydevices.com>
From: Troy Kisky <troy.kisky@boundarydevices.com> Sent: Wednesday, October 18, 2017 5:34 AM
>>> This is better for code locality and should slightly speed up normal
>interrupts.
>>>
>>> This also allows PPS clock output to start working for i.mx7. This is
>>> because
>>> i.mx7 was already using the limit of 3 interrupts, and needed another.
>>>
>>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>>>
>>> ---
>>>
>>> v2: made this change independent of any devicetree change so that old
>>> dtbs continue to work.
>>>
>>> Continue to register ptp clock if interrupt is not found.
>>> ---
>>> drivers/net/ethernet/freescale/fec.h | 3 +-
>>> drivers/net/ethernet/freescale/fec_main.c | 25 ++++++----
>>> drivers/net/ethernet/freescale/fec_ptp.c | 82
>>> ++++++++++++++++++--------
>>> -----
>>> 3 files changed, 65 insertions(+), 45 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/freescale/fec.h
>>> b/drivers/net/ethernet/freescale/fec.h
>>> index ede1876a9a19..be56ac1f1ac4 100644
>>> --- a/drivers/net/ethernet/freescale/fec.h
>>> +++ b/drivers/net/ethernet/freescale/fec.h
>>> @@ -582,12 +582,11 @@ struct fec_enet_private {
>>> u64 ethtool_stats[0];
>>> };
>>>
>>> -void fec_ptp_init(struct platform_device *pdev);
>>> +void fec_ptp_init(struct platform_device *pdev, int irq_index);
>>> void fec_ptp_stop(struct platform_device *pdev); void
>>> fec_ptp_start_cyclecounter(struct net_device *ndev); int
>>> fec_ptp_set(struct net_device *ndev, struct ifreq *ifr); int
>>> fec_ptp_get(struct net_device *ndev, struct ifreq *ifr); -uint
>>> fec_ptp_check_pps_event(struct fec_enet_private *fep);
>>>
>>>
>>>
>/**********************************************************
>>> ******************/
>>> #endif /* FEC_H */
>>> diff --git a/drivers/net/ethernet/freescale/fec_main.c
>>> b/drivers/net/ethernet/freescale/fec_main.c
>>> index 3dc2d771a222..21afabbc560f 100644
>>> --- a/drivers/net/ethernet/freescale/fec_main.c
>>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>>> @@ -1602,10 +1602,6 @@ fec_enet_interrupt(int irq, void *dev_id)
>>> ret = IRQ_HANDLED;
>>> complete(&fep->mdio_done);
>>> }
>>> -
>>> - if (fep->ptp_clock)
>>> - if (fec_ptp_check_pps_event(fep))
>>> - ret = IRQ_HANDLED;
>>> return ret;
>>> }
>>>
>>> @@ -3325,6 +3321,8 @@ fec_probe(struct platform_device *pdev)
>>> struct device_node *np = pdev->dev.of_node, *phy_node;
>>> int num_tx_qs;
>>> int num_rx_qs;
>>> + char irq_name[8];
>>> + int irq_cnt;
>>>
>>> fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs);
>>>
>>> @@ -3465,18 +3463,27 @@ fec_probe(struct platform_device *pdev)
>>> if (ret)
>>> goto failed_reset;
>>>
>>> + irq_cnt = platform_irq_count(pdev);
>>> + if (irq_cnt > FEC_IRQ_NUM)
>>> + irq_cnt = FEC_IRQ_NUM; /* last for ptp */
>>> + else if (irq_cnt == 2)
>>> + irq_cnt = 1; /* last for ptp */
>>> + else if (irq_cnt <= 0)
>>> + irq_cnt = 1; /* Let the for loop fail */
>>
>> Don't do like this. Don't suppose pps interrupt is the last one.
>
>
>I don't. If the pps interrupt is named, the named interrupt will be used. If it is
>NOT named, the last interrupt is used, if 2 interrupts, or >3 interrupt are
>provided.
>Otherwise, no pps interrupt is assumed.
>Fortunately this seems to be true currently.
>
If pps interrupt is not named, then it limit the last one is pps.
We cannot get the pps interrupt based on current chip interrupt define, we never know the future chip how to define interrupt.
Although your current implementation can work with current chips, but it is not really good solution.
>
>> And if irq_cnt is 1 like imx28/imx5x, the patch will break fec interrupt
>function.
>
>How ? fec_ptp_init will not be called as bufdesc_ex is 0.
>
Imx28 also support enhanced buffer descriptor, if define the ptp clock in dts then bufdesc_ex also can be 1.
I still suggest to use v1 logic check pps interrupt that need to check irq name.
>
>Also, if only 1 interrupt is provided, it is assumed there is no unnamed pps
>interrupt.
>
>
>>
>> I suggest to use .platform_get_irq_byname() to get pps(ptp) interrupt like
>your v1 logic check.
>>
>>> +
>>> if (fep->bufdesc_ex)
>>> - fec_ptp_init(pdev);
>>> + fec_ptp_init(pdev, irq_cnt);
>>>
>>> ret = fec_enet_init(ndev);
>>> if (ret)
>>> goto failed_init;
>>>
>>> - for (i = 0; i < FEC_IRQ_NUM; i++) {
>>> - irq = platform_get_irq(pdev, i);
>>> + for (i = 0; i < irq_cnt; i++) {
>>> + sprintf(irq_name, "int%d", i);
>>> + irq = platform_get_irq_byname(pdev, irq_name);
>>> + if (irq < 0)
>>> + irq = platform_get_irq(pdev, i);
>>> if (irq < 0) {
>>> - if (i)
>>> - break;
>>> ret = irq;
>>> goto failed_irq;
>>> }
>>> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
>>> b/drivers/net/ethernet/freescale/fec_ptp.c
>>> index 6ebad3fac81d..3abeee0d16dd 100644
>>> --- a/drivers/net/ethernet/freescale/fec_ptp.c
>>> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
>>> @@ -549,6 +549,37 @@ static void fec_time_keep(struct work_struct
>*work)
>>> schedule_delayed_work(&fep->time_keep, HZ); }
>>>
>>> +/* This function checks the pps event and reloads the timer compare
>>> +counter. */ static irqreturn_t fec_ptp_interrupt(int irq, void
>>> +*dev_id) {
>>> + struct net_device *ndev = dev_id;
>>> + struct fec_enet_private *fep = netdev_priv(ndev);
>>> + u32 val;
>>> + u8 channel = fep->pps_channel;
>>> + struct ptp_clock_event event;
>>> +
>>> + val = readl(fep->hwp + FEC_TCSR(channel));
>>> + if (val & FEC_T_TF_MASK) {
>>> + /* Write the next next compare(not the next according the
>>> spec)
>>> + * value to the register
>>> + */
>>> + writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
>>> + do {
>>> + writel(val, fep->hwp + FEC_TCSR(channel));
>>> + } while (readl(fep->hwp + FEC_TCSR(channel)) &
>>> FEC_T_TF_MASK);
>>> +
>>> + /* Update the counter; */
>>> + fep->next_counter = (fep->next_counter + fep-
>>>> reload_period) &
>>> + fep->cc.mask;
>>> +
>>> + event.type = PTP_CLOCK_PPS;
>>> + ptp_clock_event(fep->ptp_clock, &event);
>>> + return IRQ_HANDLED;
>>> + }
>>> +
>>> + return IRQ_NONE;
>>> +}
>>> +
>>> /**
>>> * fec_ptp_init
>>> * @ndev: The FEC network adapter
>>> @@ -558,10 +589,12 @@ static void fec_time_keep(struct work_struct
>>> *work)
>>> * cyclecounter init routine and exits.
>>> */
>>>
>>> -void fec_ptp_init(struct platform_device *pdev)
>>> +void fec_ptp_init(struct platform_device *pdev, int irq_index)
>>> {
>>> struct net_device *ndev = platform_get_drvdata(pdev);
>>> struct fec_enet_private *fep = netdev_priv(ndev);
>>> + int irq;
>>> + int ret;
>>>
>>> fep->ptp_caps.owner = THIS_MODULE;
>>> snprintf(fep->ptp_caps.name, 16, "fec ptp"); @@ -587,6 +620,20 @@
>>> void fec_ptp_init(struct platform_device *pdev)
>>>
>>> INIT_DELAYED_WORK(&fep->time_keep, fec_time_keep);
>>>
>>> + irq = platform_get_irq_byname(pdev, "ptp");
>>> + if (irq < 0)
>>> + irq = platform_get_irq(pdev, irq_index);
>>> + /* Failure to get an irq is not fatal,
>>> + * only the PTP_CLOCK_PPS clock events should stop
>>> + */
>>> + if (irq >= 0) {
>>> + ret = devm_request_irq(&pdev->dev, irq, fec_ptp_interrupt,
>>> + 0, pdev->name, ndev);
>>> + if (ret < 0)
>>> + dev_warn(&pdev->dev, "request for ptp irq
>>> failed(%d)\n",
>>> + ret);
>>> + }
>>> +
>>> fep->ptp_clock = ptp_clock_register(&fep->ptp_caps, &pdev->dev);
>>> if (IS_ERR(fep->ptp_clock)) {
>>> fep->ptp_clock = NULL;
>>> @@ -605,36 +652,3 @@ void fec_ptp_stop(struct platform_device *pdev)
>>> if (fep->ptp_clock)
>>> ptp_clock_unregister(fep->ptp_clock);
>>> }
>>> -
>>> -/**
>>> - * fec_ptp_check_pps_event
>>> - * @fep: the fec_enet_private structure handle
>>> - *
>>> - * This function check the pps event and reload the timer compare
>counter.
>>> - */
>>> -uint fec_ptp_check_pps_event(struct fec_enet_private *fep) -{
>>> - u32 val;
>>> - u8 channel = fep->pps_channel;
>>> - struct ptp_clock_event event;
>>> -
>>> - val = readl(fep->hwp + FEC_TCSR(channel));
>>> - if (val & FEC_T_TF_MASK) {
>>> - /* Write the next next compare(not the next according the
>>> spec)
>>> - * value to the register
>>> - */
>>> - writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
>>> - do {
>>> - writel(val, fep->hwp + FEC_TCSR(channel));
>>> - } while (readl(fep->hwp + FEC_TCSR(channel)) &
>>> FEC_T_TF_MASK);
>>> -
>>> - /* Update the counter; */
>>> - fep->next_counter = (fep->next_counter + fep-
>>>> reload_period) & fep->cc.mask;
>>> -
>>> - event.type = PTP_CLOCK_PPS;
>>> - ptp_clock_event(fep->ptp_clock, &event);
>>> - return 1;
>>> - }
>>> -
>>> - return 0;
>>> -}
>>> --
>>> 2.11.0
>>
>>
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH] [v2] i40e: avoid 64-bit division where possible
From: Nambiar, Amritha @ 2017-10-18 5:37 UTC (permalink / raw)
To: Alexander Duyck, Arnd Bergmann
Cc: Netdev, Mitch Williams, linux-kernel@vger.kernel.org,
intel-wired-lan, Björn Töpel, Filip Sadowski,
David S. Miller
In-Reply-To: <CAKgT0UfeDJomnen_2PhgnHSi27YROjTAtT=OYEj0JNVD8fzcOw@mail.gmail.com>
On 10/17/2017 10:33 AM, Alexander Duyck wrote:
> On Tue, Oct 17, 2017 at 8:49 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> The new bandwidth calculation caused a link error on 32-bit
>> architectures, like
>>
>> ERROR: "__aeabi_uldivmod" [drivers/net/ethernet/intel/i40e/i40e.ko] undefined!
>>
>> The problem is the max_tx_rate calculation that uses 64-bit integers.
>> This is not really necessary since the numbers are in MBit/s so
>> they won't be higher than 40000 for the highest support rate, and
>> are guaranteed to not exceed 2^32 in future generations either.
>>
>> Another patch from Alan Brady fixed the link error by adding
>> many calls to do_div(), which makes the code less efficent and
>> less readable than necessary.
>>
>> This changes the representation to 'u32' when dealing with MBit/s
>> and uses div_u64() to convert from u64 numbers in byte/s, reverting
>> parts of Alan's earlier fix that have become obsolete now.
>>
This patch breaks the functionality while converting the rates in
bytes/s provided by tc-layer into the Mbit/s in the driver.
I40E_BW_MBPS_DIVISOR defined in Alan's patch should be used for the
conversion, and not I40E_BW_CREDIT_DIVISOR which does the incorrect
math. I40E_BW_CREDIT_DIVISOR is in place because the device uses credit
rates in values of 50Mbps.
>> Fixes: 2027d4deacb1 ("i40e: Add support setting TC max bandwidth rates")
>> Fixes: 73983b5ae011 ("i40e: fix u64 division usage")
>> Cc: Alan Brady <alan.brady@intel.com>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> So this patch looks good to me, we just need to test it to verify it
> doesn't break existing functionality. In the meantime if Alan's patch
> has gone through testing we should probably push "i40e: fix u64
> division usage" to Dave so that we can at least fix the linking issues
> on ARM and i386.
>
> Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com>
>
>> ---
>> drivers/net/ethernet/intel/i40e/i40e.h | 4 +-
>> drivers/net/ethernet/intel/i40e/i40e_main.c | 70 +++++++++++------------------
>> 2 files changed, 27 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
>> index c3f13120f3ce..c7aa0c982273 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>> @@ -407,7 +407,7 @@ struct i40e_channel {
>> u8 enabled_tc;
>> struct i40e_aqc_vsi_properties_data info;
>>
>> - u64 max_tx_rate;
>> + u32 max_tx_rate; /* in Mbits/s */
>>
>> /* track this channel belongs to which VSI */
>> struct i40e_vsi *parent_vsi;
>> @@ -1101,5 +1101,5 @@ static inline bool i40e_enabled_xdp_vsi(struct i40e_vsi *vsi)
>> }
>>
>> int i40e_create_queue_channel(struct i40e_vsi *vsi, struct i40e_channel *ch);
>> -int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate);
>> +int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u32 max_tx_rate);
>> #endif /* _I40E_H_ */
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index 3ceda140170d..57682cc78508 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> @@ -5448,17 +5448,16 @@ int i40e_get_link_speed(struct i40e_vsi *vsi)
>> *
>> * Helper function to set BW limit for a given VSI
>> **/
>> -int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate)
>> +int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u32 max_tx_rate)
>> {
>> struct i40e_pf *pf = vsi->back;
>> - u64 credits = 0;
>> int speed = 0;
>> int ret = 0;
>>
>> speed = i40e_get_link_speed(vsi);
>> if (max_tx_rate > speed) {
>> dev_err(&pf->pdev->dev,
>> - "Invalid max tx rate %llu specified for VSI seid %d.",
>> + "Invalid max tx rate %u specified for VSI seid %d.",
>> max_tx_rate, seid);
>> return -EINVAL;
>> }
>> @@ -5469,13 +5468,12 @@ int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate)
>> }
>>
>> /* Tx rate credits are in values of 50Mbps, 0 is disabled */
>> - credits = max_tx_rate;
>> - do_div(credits, I40E_BW_CREDIT_DIVISOR);
>> - ret = i40e_aq_config_vsi_bw_limit(&pf->hw, seid, credits,
>> + ret = i40e_aq_config_vsi_bw_limit(&pf->hw, seid,
>> + max_tx_rate / I40E_BW_CREDIT_DIVISOR,
>> I40E_MAX_BW_INACTIVE_ACCUM, NULL);
>> if (ret)
>> dev_err(&pf->pdev->dev,
>> - "Failed set tx rate (%llu Mbps) for vsi->seid %u, err %s aq_err %s\n",
>> + "Failed set tx rate (%u Mbps) for vsi->seid %u, err %s aq_err %s\n",
>> max_tx_rate, seid, i40e_stat_str(&pf->hw, ret),
>> i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status));
>> return ret;
>> @@ -6158,17 +6156,13 @@ int i40e_create_queue_channel(struct i40e_vsi *vsi,
>>
>> /* configure VSI for BW limit */
>> if (ch->max_tx_rate) {
>> - u64 credits = ch->max_tx_rate;
>> -
>> if (i40e_set_bw_limit(vsi, ch->seid, ch->max_tx_rate))
>> return -EINVAL;
>>
>> - do_div(credits, I40E_BW_CREDIT_DIVISOR);
>> dev_dbg(&pf->pdev->dev,
>> - "Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
>> + "Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
>> ch->max_tx_rate,
>> - credits,
>> - ch->seid);
>> + ch->max_tx_rate / I40E_BW_CREDIT_DIVISOR, ch->seid);
>> }
>>
>> /* in case of VF, this will be main SRIOV VSI */
>> @@ -6189,7 +6183,6 @@ int i40e_create_queue_channel(struct i40e_vsi *vsi,
>> static int i40e_configure_queue_channels(struct i40e_vsi *vsi)
>> {
>> struct i40e_channel *ch;
>> - u64 max_rate = 0;
>> int ret = 0, i;
>>
>> /* Create app vsi with the TCs. Main VSI with TC0 is already set up */
>> @@ -6211,9 +6204,8 @@ static int i40e_configure_queue_channels(struct i40e_vsi *vsi)
>> /* Bandwidth limit through tc interface is in bytes/s,
>> * change to Mbit/s
>> */
>> - max_rate = vsi->mqprio_qopt.max_rate[i];
>> - do_div(max_rate, I40E_BW_MBPS_DIVISOR);
>> - ch->max_tx_rate = max_rate;
>> + ch->max_tx_rate = div_u64(vsi->mqprio_qopt.max_rate[i],
>> + I40E_BW_CREDIT_DIVISOR);
>>
Use I40E_BW_MBPS_DIVISOR for the conversion math here.
>> list_add_tail(&ch->list, &vsi->ch_list);
>>
>> @@ -6643,7 +6635,6 @@ static int i40e_validate_mqprio_qopt(struct i40e_vsi *vsi,
>> struct tc_mqprio_qopt_offload *mqprio_qopt)
>> {
>> u64 sum_max_rate = 0;
>> - u64 max_rate = 0;
>> int i;
>>
>> if (mqprio_qopt->qopt.offset[0] != 0 ||
>> @@ -6658,9 +6649,8 @@ static int i40e_validate_mqprio_qopt(struct i40e_vsi *vsi,
>> "Invalid min tx rate (greater than 0) specified\n");
>> return -EINVAL;
>> }
>> - max_rate = mqprio_qopt->max_rate[i];
>> - do_div(max_rate, I40E_BW_MBPS_DIVISOR);
>> - sum_max_rate += max_rate;
>> + sum_max_rate += div_u64(mqprio_qopt->max_rate[i],
>> + I40E_BW_CREDIT_DIVISOR);
Use I40E_BW_MBPS_DIVISOR here.
>>
>> if (i >= mqprio_qopt->qopt.num_tc - 1)
>> break;
>> @@ -6804,18 +6794,14 @@ static int i40e_setup_tc(struct net_device *netdev, void *type_data)
>>
>> if (pf->flags & I40E_FLAG_TC_MQPRIO) {
>> if (vsi->mqprio_qopt.max_rate[0]) {
>> - u64 max_tx_rate = vsi->mqprio_qopt.max_rate[0];
>> -
>> - do_div(max_tx_rate, I40E_BW_MBPS_DIVISOR);
>> + u32 max_tx_rate = div_u64(vsi->mqprio_qopt.max_rate[0],
>> + I40E_BW_CREDIT_DIVISOR);
Use I40E_BW_MBPS_DIVISOR here.
>> ret = i40e_set_bw_limit(vsi, vsi->seid, max_tx_rate);
>> if (!ret) {
>> - u64 credits = max_tx_rate;
>> -
>> - do_div(credits, I40E_BW_CREDIT_DIVISOR);
>> dev_dbg(&vsi->back->pdev->dev,
>> - "Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
>> + "Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
>> max_tx_rate,
>> - credits,
>> + max_tx_rate / I40E_BW_CREDIT_DIVISOR,
>> vsi->seid);
>> } else {
>> need_reset = true;
>> @@ -9024,15 +9010,12 @@ static int i40e_rebuild_channels(struct i40e_vsi *vsi)
>> return ret;
>> }
>> if (ch->max_tx_rate) {
>> - u64 credits = ch->max_tx_rate;
>> -
>> if (i40e_set_bw_limit(vsi, ch->seid,
>> ch->max_tx_rate))
>> return -EINVAL;
>>
>> - do_div(credits, I40E_BW_CREDIT_DIVISOR);
>> dev_dbg(&vsi->back->pdev->dev,
>> - "Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
>> + "Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
>> ch->max_tx_rate,
>> ch->max_tx_rate / I40E_BW_CREDIT_DIVISOR,
>> ch->seid);
>> @@ -9314,21 +9297,18 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
>> }
>>
>> if (vsi->mqprio_qopt.max_rate[0]) {
>> - u64 max_tx_rate = vsi->mqprio_qopt.max_rate[0];
>> - u64 credits = 0;
>> + u32 max_tx_rate = div_u64(vsi->mqprio_qopt.max_rate[0],
>> + I40E_BW_CREDIT_DIVISOR);
Use I40E_BW_MBPS_DIVISOR here.
>>
>> - do_div(max_tx_rate, I40E_BW_MBPS_DIVISOR);
>> ret = i40e_set_bw_limit(vsi, vsi->seid, max_tx_rate);
>> - if (ret)
>> + if (!ret)
>> + dev_dbg(&vsi->back->pdev->dev,
>> + "Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
>> + max_tx_rate,
>> + max_tx_rate / I40E_BW_CREDIT_DIVISOR,
>> + vsi->seid);
>> + else
>> goto end_unlock;
>> -
>> - credits = max_tx_rate;
>> - do_div(credits, I40E_BW_CREDIT_DIVISOR);
>> - dev_dbg(&vsi->back->pdev->dev,
>> - "Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
>> - max_tx_rate,
>> - credits,
>> - vsi->seid);
>> }
>>
>> ret = i40e_rebuild_cloud_filters(vsi, vsi->seid);
>> --
>> 2.9.0
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>
^ permalink raw reply
* Re: [PATCH v9 00/20] simplify crypto wait for async op
From: Gilad Ben-Yossef @ 2017-10-18 5:44 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer,
device-mapper development, Steve French, Theodore Y. Ts'o,
Jaegeuk Kim, Steffen Klassert, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Mimi Zohar
In-Reply-To: <20171017140629.GO20805@n2100.armlinux.org.uk>
On Tue, Oct 17, 2017 at 5:06 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Sun, Oct 15, 2017 at 10:19:45AM +0100, Gilad Ben-Yossef wrote:
>> Many users of kernel async. crypto services have a pattern of
>> starting an async. crypto op and than using a completion
>> to wait for it to end.
>>
>> This patch set simplifies this common use case in two ways:
>>
>> First, by separating the return codes of the case where a
>> request is queued to a backlog due to the provider being
>> busy (-EBUSY) from the case the request has failed due
>> to the provider being busy and backlogging is not enabled
>> (-EAGAIN).
>>
>> Next, this change is than built on to create a generic API
>> to wait for a async. crypto operation to complete.
>>
>> The end result is a smaller code base and an API that is
>> easier to use and more difficult to get wrong.
>>
>> The patch set was boot tested on x86_64 and arm64 which
>> at the very least tests the crypto users via testmgr and
>> tcrypt but I do note that I do not have access to some
>> of the HW whose drivers are modified nor do I claim I was
>> able to test all of the corner cases.
>>
>> The patch set is based upon linux-next release tagged
>> next-20171013.
>
> Has there been any performance impact analysis of these changes? I
> ended up with patches for one of the crypto drivers which converted
> its interrupt handling to threaded interrupts being reverted because
> it caused a performance degredation.
>
> Moving code to latest APIs to simplify it is not always beneficial.
I agree with the sentiment but I believe this one is justified.
This patch set basically does 3 things:
1. Replace one immediate value (-EBUSY) by another (-EAGAIN). Mostly it's just
s/EBUSY/EAGAIN/g. In very few places this resulted very trivial code
changes. I don't
foresee this having any effect on performance.
2. Removal of some conditions and/or conditional jumps that were used to discern
between two different cases which are now now easily tested for by the
different return
value. If at all, this will be an increase in performance, although I
don't expect it to be
noticeable.
3. Replacing a whole bunch of open coded code and data structures
which were pretty much
cut and pasted from the Documentation and therefore identical, with a
single copy thereof.
Every place that I found that deviated slightly from the identical
pattern, it turned out to be
a bug of some sorts and patches for those were sent and accepted already.
So, we might be losing a few inline optimization opportunities but
we're gaining better
cache utilization. Again, I don't expect any of this to have a
noticeable effect to either
direction.
I did run the changed code as best I could and did not notice any
performance changes and
none of the testers and maintainers that ACKed mentioned any.
Having said that, it's a big change that touches many places,
sub-systems and drivers. I do
not claim to have thoroughly tested for performance all the changes in
person. In some cases,
I don't even have access to the specialized hardware. I did get a
reasonable amount of review
and testers I believe - would always love to see more :-)
Many thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply
* Re: [PATCH 00/58] networking: Convert timers to use timer_setup()
From: Kalle Valo @ 2017-10-18 5:44 UTC (permalink / raw)
To: Kees Cook
Cc: David S. Miller, Network Development, Thomas Gleixner, LKML,
linux-wireless
In-Reply-To: <CAGXu5j+fcARk=yWh9Bbkn0C-4PHd+pKSPGV+5qPx88H6prvUoA@mail.gmail.com>
Kees Cook <keescook@chromium.org> writes:
> On Tue, Oct 17, 2017 at 7:18 AM, Kalle Valo <kvalo@codeaurora.org> wrote:
>> + linux-wireless
>>
>> Hi Kees,
>>
>> Kees Cook <keescook@chromium.org> writes:
>>
>>> This is the current set of outstanding networking patches to perform
>>> conversions to the new timer interface (rebased to -next). This is not
>>> all expected conversions, but it contains everything needed in networking
>>> to eliminate init_timer(), and all the non-standard setup_*_timer() uses.
>>
>> So this also includes patches which I had queued for
>> wireless-drivers-next:
>>
>> https://patchwork.kernel.org/patch/9986253/
>> https://patchwork.kernel.org/patch/9986245/
>>
>> And looking at patchwork[1] I have even more timer_setup() related
>> patches from you. It would be really helpful if you could clearly
>> document to which tree you want the patches to be applied. I don't care
>
> Hi! Sorry about that. It's been a bit tricky to juggle everything.
Yeah, I understand.
>> if it's net-next or wireless-drivers-next as long as it's not the both
>> (meaning that both Dave and me apply the same patch, which would be
>> bad). The thing is that I really do not have time to figure out for
>> every patch via which tree it's supposed to go.
>
> Which split is preferred? I had been trying to separate wireless from
> the rest of net (but missed some cases).
So what we try to follow is that I apply all patches for
drivers/net/wireless to my wireless-drivers trees, with exception of
Johannes taking mac80211_hwsim.c patches to his mac80211 trees. And
Johannes of course takes all patches for net/wireless and net/mac80211.
So in general I prefer that I take all drivers/net/wireless patches and
make it obvious for Dave that he can ignore those patches (not mix
wireless-drivers and net patches into same set etc). But like I said,
it's ok to push API changes like these via Dave's net trees as well if
you want (and if Dave is ok with that). The chances of conflicts is low,
and if there are be any those would be easy to fix either by me or Dave.
>> For now I'll just drop all your timer_setup() related patches from my
>> queue and I'll assume Dave will take those. Ok?
>>
>> [1] https://patchwork.kernel.org/project/linux-wireless/list/
>
> I guess I'll wait to see what Dave says.
Ok, I don't drop the patches from my queue quite yet then.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH net-next 0/3] net: sh_eth: add R-Car Gen[12] fallback compatibility strings
From: Simon Horman @ 2017-10-18 6:49 UTC (permalink / raw)
To: David Miller, Sergei Shtylyov; +Cc: Magnus Damm, netdev, linux-renesas-soc
In-Reply-To: <20171017074747.24159-1-horms+renesas@verge.net.au>
On Tue, Oct 17, 2017 at 09:47:44AM +0200, Simon Horman wrote:
> Add fallback compatibility strings for R-Car Gen 1 and 2.
>
> In the case of Renesas R-Car hardware we know that there are generations of
> SoCs, f.e. Gen 1 and 2. But beyond that its not clear what the relationship
> between IP blocks might be. For example, I believe that r8a7790 is older
> than r8a7791 but that doesn't imply that the latter is a descendant of the
> former or vice versa.
>
> We can, however, by examining the documentation and behaviour of the
> hardware at run-time observe that the current driver implementation appears
> to be compatible with the IP blocks on SoCs within a given generation.
>
> For the above reasons and convenience when enabling new SoCs a
> per-generation fallback compatibility string scheme being adopted for
> drivers for Renesas SoCs.
>
> Simon Horman (3):
> dt-bindings: net: sh_eth: add R-Car Gen[12] fallback compatibility
> strings
> net: sh_eth: rename name structures as rcar_gen[12]_*
> net: sh_eth: implement R-Car Gen[12] fallback compatibility strings
There have been a few comments on this series, I will post v2.
^ permalink raw reply
* Re: using verifier to ensure a BPF program uses certain metadata?
From: Johannes Berg @ 2017-10-18 6:56 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: netdev, Daniel Borkmann, linux-wireless
In-Reply-To: <20171017225806.b5xubolkyocfgnjc@ast-mbp>
Hi Alexei,
> > https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next
> > .git/log/?h=bpf
>
> bpf bits looks pretty straightforward.
Thanks for looking at this!
> attach looks fine too. I'm assuming there is some rtnl or other lock,
> so multiple assigns cannot race?
Yes.
> It's missing query interface though.
> Please add support to return prog_id.
Good point, this is about half a year old, so ... :)
[...]
> > Now, I realize that people could trivially just work around this in
> > their program if they wanted, but I think most will take the
> > reminder
> > and just implement
> >
> > if (ctx->is_data_ethernet)
> > return DROP_FRAME;
> >
> > instead, since mostly data frames will not be very relevant to
> > them.
> >
> > What do you think?
>
> sounds fine and considering new verifier ops after Jakub refactoring
> a check that is_data_ethernet was accessed would fit nicely.
> Without void** hack.
Ok, thanks! I'll have to check what Jakub is doing there, do you have a
pointer to that refactoring?
johannes
^ permalink raw reply
* Re: [patch net-next 01/20] net: sched: add block bind/unbind notif. and extended block_get/put
From: Jiri Pirko @ 2017-10-18 6:59 UTC (permalink / raw)
To: Duyck, Alexander H
Cc: netdev@vger.kernel.org, andrew@lunn.ch, leonro@mellanox.com,
idosch@mellanox.com, daniel@iogearbox.net, ast@kernel.org,
jhs@mojatatu.com, saeedm@mellanox.com, mlxsw@mellanox.com,
john.hurley@netronome.com, f.fainelli@gmail.com,
xiyou.wangcong@gmail.com, jakub.kicinski@netronome.com,
ganeshgr@chelsio.com, simon.horman@netronome.com,
michael.chan@broadcom.com
In-Reply-To: <1508271733.11655.3.camel@intel.com>
Tue, Oct 17, 2017 at 10:22:16PM CEST, alexander.h.duyck@intel.com wrote:
>On Tue, 2017-10-17 at 22:05 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Introduce new type of ndo_setup_tc message to propage binding/unbinding
>> of a block to driver. Call this ndo whenever qdisc gets/puts a block.
>> Alongside with this, there's need to propagate binder type from qdisc
>> code down to the notifier. So introduce extended variants of
>> block_get/put in order to pass this info.
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> include/linux/netdevice.h | 1 +
>> include/net/pkt_cls.h | 40 +++++++++++++++++++++++++++++++++
>> net/sched/cls_api.c | 56 ++++++++++++++++++++++++++++++++++++++++++++---
>> 3 files changed, 94 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 31bb301..062a4f5 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -771,6 +771,7 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>>
>> enum tc_setup_type {
>> TC_SETUP_MQPRIO,
>> + TC_SETUP_BLOCK,
>> TC_SETUP_CLSU32,
>> TC_SETUP_CLSFLOWER,
>> TC_SETUP_CLSMATCHALL,
>
>I'm not a big fan of adding this to the middle of the enum. It will
>make it harder for people that have to backport changes and such since
>it is reordering values that are passed as a part of the kabi between
>drivers and the kernel.
I put it where I think it fits. I never think about backport while
working on upstream kernel. I believe it is a bad practise if you do so.
But in this case, the backport is trivial, I don't see any problem with
that.
>
>Also does this patch set really need to be 20 patches long? Seems like
>you could have done this as a set of 8 and another of 12 since you need
>about 8 patches to get to the point where you start pulling the code
>out of the drivers.
Yeah, basically I can cut the patchset in any place. But I wanted to
show the whole change. If there was more drivers using this, I would
have to add patches. Cutting it in half just because of the amount feels
wrong, it's half way through there :/ If you insist, I can cut it. I can
even squash multiple patches to one. But that also feels wrong :/
^ permalink raw reply
* [PATCH v10 00/20] simplify crypto wait for async op
From: Gilad Ben-Yossef @ 2017-10-18 7:00 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Steffen Klassert,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin <dmitry.ka
Cc: Ofir Drang, linux-crypto, linux-doc, linux-kernel, keyrings,
linux-arm-kernel, linux-mediatek, linux-cifs, samba-technical,
linux-fscrypt, netdev, linux-ima-devel, linux-ima-user,
linux-security-module
Many users of kernel async. crypto services have a pattern of
starting an async. crypto op and than using a completion
to wait for it to end.
This patch set simplifies this common use case in two ways:
First, by separating the return codes of the case where a
request is queued to a backlog due to the provider being
busy (-EBUSY) from the case the request has failed due
to the provider being busy and backlogging is not enabled
(-ENOSPC).
Next, this change is than built on to create a generic API
to wait for a async. crypto operation to complete.
The end result is a smaller code base and an API that is
easier to use and more difficult to get wrong.
The patch set was boot tested on x86_64 and arm64 which
at the very least tests the crypto users via testmgr and
tcrypt but I do note that I do not have access to some
of the HW whose drivers are modified nor do I claim I was
able to test all of the corner cases.
The patch set is based upon linux-next release tagged
next-20171017.
Changes from v9:
- s/EAGAIN/ENOSPC/g so as to not confuse networking
indication of a signal delivery with tfm provider
running out of processing resources.
Changes from v8:
- Remove the translation of EAGAIN return code to the
previous return code of EBUSY for the user space
interface of algif as no one seems to rely on it as
requested by Herbert Xu.
Changes from v7:
- Turn -EBUSY to -EAGAIN also in crypto using net
code which I missed before, as has been pointed
out by Harsh Jain.
Changes from v6:
- Fix brown paper bag compile error on marvell/cesa
code.
Changes from v5:
- Remove redundant new line as spotted by Jonathan
Cameron.
- Reworded dm-verity change commit message to better
clarify potential issue averted by change as
pointed out by Mikulas Patocka.
Changes from v4:
- Rebase on top of latest algif changes from Stephan
Mueller.
- Fix typo in ccp patch title.
Changes from v3:
- Instead of changing the return code to indicate
backlog queueing, change the return code to indicate
transient busy state, as suggested by Herbert Xu.
Changes from v2:
- Patch title changed from "introduce crypto wait for
async op" to better reflect the current state.
- Rebase on top of latest linux-next.
- Add a new return code of -EIOCBQUEUED for backlog
queueing, as suggested by Herbert Xu.
- Transform more users to the new API.
- Update the drbg change to account for new init as
indicated by Stephan Muller.
Changes from v1:
- Address review comments from Eric Biggers.
- Separated out bug fixes of existing code and rebase
on top of that patch set.
- Rename 'ecr' to 'wait' in fscrypto code.
- Split patch introducing the new API from the change
moving over the algif code which it originated from
to the new API.
- Inline crypto_wait_req().
- Some code indentation fixes.
Gilad Ben-Yossef (20):
crypto: change transient busy return code to -ENOSPC
crypto: ccp: use -ENOSPC for transient busy indication
net: use -ENOSPC for transient busy indication
crypto: remove redundant backlog checks on EBUSY
crypto: marvell/cesa: remove redundant backlog checks on EBUSY
crypto: introduce crypto wait for async op
crypto: move algif to generic async completion
crypto: move pub key to generic async completion
crypto: move drbg to generic async completion
crypto: move gcm to generic async completion
crypto: move testmgr to generic async completion
fscrypt: move to generic async completion
dm: move dm-verity to generic async completion
cifs: move to generic async completion
ima: move to generic async completion
crypto: tcrypt: move to generic async completion
crypto: talitos: move to generic async completion
crypto: qce: move to generic async completion
crypto: mediatek: move to generic async completion
crypto: adapt api sample to use async. op wait
Documentation/crypto/api-samples.rst | 52 ++-------
crypto/af_alg.c | 27 -----
crypto/ahash.c | 12 +--
crypto/algapi.c | 6 +-
crypto/algif_aead.c | 8 +-
crypto/algif_hash.c | 30 +++---
crypto/algif_skcipher.c | 9 +-
crypto/api.c | 13 +++
crypto/asymmetric_keys/public_key.c | 28 +----
crypto/cryptd.c | 4 +-
crypto/cts.c | 6 +-
crypto/drbg.c | 36 ++-----
crypto/gcm.c | 32 ++----
crypto/lrw.c | 8 +-
crypto/rsa-pkcs1pad.c | 16 +--
crypto/tcrypt.c | 84 +++++----------
crypto/testmgr.c | 204 ++++++++++++-----------------------
crypto/xts.c | 8 +-
drivers/crypto/ccp/ccp-crypto-main.c | 8 +-
drivers/crypto/ccp/ccp-dev.c | 7 +-
drivers/crypto/marvell/cesa.c | 3 +-
drivers/crypto/marvell/cesa.h | 2 +-
drivers/crypto/mediatek/mtk-aes.c | 31 +-----
drivers/crypto/qce/sha.c | 30 +-----
drivers/crypto/talitos.c | 38 +------
drivers/md/dm-verity-target.c | 81 ++++----------
drivers/md/dm-verity.h | 5 -
fs/cifs/smb2ops.c | 30 +-----
fs/crypto/crypto.c | 28 +----
fs/crypto/fname.c | 36 ++-----
fs/crypto/fscrypt_private.h | 10 --
fs/crypto/keyinfo.c | 21 +---
include/crypto/drbg.h | 3 +-
include/crypto/if_alg.h | 15 +--
include/linux/crypto.h | 40 +++++++
net/ipv4/ah4.c | 2 +-
net/ipv4/esp4.c | 2 +-
net/ipv6/ah6.c | 2 +-
net/ipv6/esp6.c | 2 +-
security/integrity/ima/ima_crypto.c | 56 +++-------
40 files changed, 297 insertions(+), 738 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v10 01/20] crypto: change transient busy return code to -ENOSPC
From: Gilad Ben-Yossef @ 2017-10-18 7:00 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Steffen Klassert,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin <dmitry.ka
Cc: Ofir Drang, linux-crypto, linux-doc, linux-kernel, keyrings,
linux-arm-kernel, linux-mediatek, linux-cifs, samba-technical,
linux-fscrypt, netdev, linux-ima-devel, linux-ima-user,
linux-security-module
In-Reply-To: <1508310057-25369-1-git-send-email-gilad@benyossef.com>
The crypto API was using the -EBUSY return value to indicate
both a hard failure to submit a crypto operation into a
transformation provider when the latter was busy and the backlog
mechanism was not enabled as well as a notification that the
operation was queued into the backlog when the backlog mechanism
was enabled.
Having the same return code indicate two very different conditions
depending on a flag is both error prone and requires extra runtime
check like the following to discern between the cases:
if (err == -EINPROGRESS ||
(err == -EBUSY && (ahash_request_flags(req) &
CRYPTO_TFM_REQ_MAY_BACKLOG)))
This patch changes the return code used to indicate a crypto op
failed due to the transformation provider being transiently busy
to -ENOSPC.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
crypto/algapi.c | 6 ++++--
crypto/cryptd.c | 4 +---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/crypto/algapi.c b/crypto/algapi.c
index aa699ff..60d7366 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -897,9 +897,11 @@ int crypto_enqueue_request(struct crypto_queue *queue,
int err = -EINPROGRESS;
if (unlikely(queue->qlen >= queue->max_qlen)) {
- err = -EBUSY;
- if (!(request->flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+ if (!(request->flags & CRYPTO_TFM_REQ_MAY_BACKLOG)) {
+ err = -ENOSPC;
goto out;
+ }
+ err = -EBUSY;
if (queue->backlog == &queue->list)
queue->backlog = &request->list;
}
diff --git a/crypto/cryptd.c b/crypto/cryptd.c
index 0508c48..bd43cf5 100644
--- a/crypto/cryptd.c
+++ b/crypto/cryptd.c
@@ -137,16 +137,14 @@ static int cryptd_enqueue_request(struct cryptd_queue *queue,
int cpu, err;
struct cryptd_cpu_queue *cpu_queue;
atomic_t *refcnt;
- bool may_backlog;
cpu = get_cpu();
cpu_queue = this_cpu_ptr(queue->cpu_queue);
err = crypto_enqueue_request(&cpu_queue->queue, request);
refcnt = crypto_tfm_ctx(request->tfm);
- may_backlog = request->flags & CRYPTO_TFM_REQ_MAY_BACKLOG;
- if (err == -EBUSY && !may_backlog)
+ if (err == -ENOSPC)
goto out_put_cpu;
queue_work_on(cpu, kcrypto_wq, &cpu_queue->work);
--
2.7.4
^ permalink raw reply related
* [PATCH v10 02/20] crypto: ccp: use -ENOSPC for transient busy indication
From: Gilad Ben-Yossef @ 2017-10-18 7:00 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Steffen Klassert,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin <dmitry.ka
Cc: Ofir Drang, linux-crypto, linux-doc, linux-kernel, keyrings,
linux-arm-kernel, linux-mediatek, linux-cifs, samba-technical,
linux-fscrypt, netdev, linux-ima-devel, linux-ima-user,
linux-security-module
In-Reply-To: <1508310057-25369-1-git-send-email-gilad@benyossef.com>
Replace -EBUSY with -ENOSPC when reporting transient busy
indication in the absence of backlog.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Reviewed-by: Gary R Hook <gary.hook@amd.com>
---
Please squash this patch with the previous one when merging upstream.
drivers/crypto/ccp/ccp-crypto-main.c | 8 +++-----
drivers/crypto/ccp/ccp-dev.c | 7 +++++--
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/crypto/ccp/ccp-crypto-main.c b/drivers/crypto/ccp/ccp-crypto-main.c
index 35a9de7..b95d199 100644
--- a/drivers/crypto/ccp/ccp-crypto-main.c
+++ b/drivers/crypto/ccp/ccp-crypto-main.c
@@ -222,9 +222,10 @@ static int ccp_crypto_enqueue_cmd(struct ccp_crypto_cmd *crypto_cmd)
/* Check if the cmd can/should be queued */
if (req_queue.cmd_count >= CCP_CRYPTO_MAX_QLEN) {
- ret = -EBUSY;
- if (!(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG))
+ if (!(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG)) {
+ ret = -ENOSPC;
goto e_lock;
+ }
}
/* Look for an entry with the same tfm. If there is a cmd
@@ -243,9 +244,6 @@ static int ccp_crypto_enqueue_cmd(struct ccp_crypto_cmd *crypto_cmd)
ret = ccp_enqueue_cmd(crypto_cmd->cmd);
if (!ccp_crypto_success(ret))
goto e_lock; /* Error, don't queue it */
- if ((ret == -EBUSY) &&
- !(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG))
- goto e_lock; /* Not backlogging, don't queue it */
}
if (req_queue.cmd_count >= CCP_CRYPTO_MAX_QLEN) {
diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c
index 4e029b1..1b5035d 100644
--- a/drivers/crypto/ccp/ccp-dev.c
+++ b/drivers/crypto/ccp/ccp-dev.c
@@ -292,9 +292,12 @@ int ccp_enqueue_cmd(struct ccp_cmd *cmd)
i = ccp->cmd_q_count;
if (ccp->cmd_count >= MAX_CMD_QLEN) {
- ret = -EBUSY;
- if (cmd->flags & CCP_CMD_MAY_BACKLOG)
+ if (cmd->flags & CCP_CMD_MAY_BACKLOG) {
+ ret = -EBUSY;
list_add_tail(&cmd->entry, &ccp->backlog);
+ } else {
+ ret = -ENOSPC;
+ }
} else {
ret = -EINPROGRESS;
ccp->cmd_count++;
--
2.7.4
^ permalink raw reply related
* [PATCH v10 03/20] net: use -ENOSPC for transient busy indication
From: Gilad Ben-Yossef @ 2017-10-18 7:00 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Steffen Klassert,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin <dmitry.ka
Cc: Ofir Drang, linux-crypto, linux-doc, linux-kernel, keyrings,
linux-arm-kernel, linux-mediatek, linux-cifs, samba-technical,
linux-fscrypt, netdev, linux-ima-devel, linux-ima-user,
linux-security-module
In-Reply-To: <1508310057-25369-1-git-send-email-gilad@benyossef.com>
Replace -EBUSY with -ENOSPC when handling transient busy
indication in the absence of backlog.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
Please squash this patch with the previous one when merging upstream.
net/ipv4/ah4.c | 2 +-
net/ipv4/esp4.c | 2 +-
net/ipv6/ah6.c | 2 +-
net/ipv6/esp6.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index 37db44f..4dd95cd 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c
@@ -240,7 +240,7 @@ static int ah_output(struct xfrm_state *x, struct sk_buff *skb)
if (err == -EINPROGRESS)
goto out;
- if (err == -EBUSY)
+ if (err == -ENOSPC)
err = NET_XMIT_DROP;
goto out_free;
}
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index b00e4a4..d57aa64 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -432,7 +432,7 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
case -EINPROGRESS:
goto error;
- case -EBUSY:
+ case -ENOSPC:
err = NET_XMIT_DROP;
break;
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index 7802b72..3bd9d80 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -443,7 +443,7 @@ static int ah6_output(struct xfrm_state *x, struct sk_buff *skb)
if (err == -EINPROGRESS)
goto out;
- if (err == -EBUSY)
+ if (err == -ENOSPC)
err = NET_XMIT_DROP;
goto out_free;
}
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 89910e2..c04d995 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -396,7 +396,7 @@ int esp6_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info
case -EINPROGRESS:
goto error;
- case -EBUSY:
+ case -ENOSPC:
err = NET_XMIT_DROP;
break;
--
2.7.4
^ permalink raw reply related
* [PATCH v10 04/20] crypto: remove redundant backlog checks on EBUSY
From: Gilad Ben-Yossef @ 2017-10-18 7:00 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Steffen Klassert,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn
Cc: linux-cifs, linux-doc, linux-ima-user, netdev, samba-technical,
linux-kernel, linux-fscrypt, keyrings, linux-mediatek,
linux-crypto, linux-security-module, linux-ima-devel,
linux-arm-kernel, Ofir Drang
In-Reply-To: <1508310057-25369-1-git-send-email-gilad@benyossef.com>
Now that -EBUSY return code only indicates backlog queueing
we can safely remove the now redundant check for the
CRYPTO_TFM_REQ_MAY_BACKLOG flag when -EBUSY is returned.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
crypto/ahash.c | 12 +++---------
crypto/cts.c | 6 ++----
crypto/lrw.c | 8 ++------
crypto/rsa-pkcs1pad.c | 16 ++++------------
crypto/xts.c | 8 ++------
5 files changed, 13 insertions(+), 37 deletions(-)
diff --git a/crypto/ahash.c b/crypto/ahash.c
index 5e8666e..3a35d67 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -334,9 +334,7 @@ static int ahash_op_unaligned(struct ahash_request *req,
return err;
err = op(req);
- if (err == -EINPROGRESS ||
- (err == -EBUSY && (ahash_request_flags(req) &
- CRYPTO_TFM_REQ_MAY_BACKLOG)))
+ if (err == -EINPROGRESS || err == -EBUSY)
return err;
ahash_restore_req(req, err);
@@ -394,9 +392,7 @@ static int ahash_def_finup_finish1(struct ahash_request *req, int err)
req->base.complete = ahash_def_finup_done2;
err = crypto_ahash_reqtfm(req)->final(req);
- if (err == -EINPROGRESS ||
- (err == -EBUSY && (ahash_request_flags(req) &
- CRYPTO_TFM_REQ_MAY_BACKLOG)))
+ if (err == -EINPROGRESS || err == -EBUSY)
return err;
out:
@@ -432,9 +428,7 @@ static int ahash_def_finup(struct ahash_request *req)
return err;
err = tfm->update(req);
- if (err == -EINPROGRESS ||
- (err == -EBUSY && (ahash_request_flags(req) &
- CRYPTO_TFM_REQ_MAY_BACKLOG)))
+ if (err == -EINPROGRESS || err == -EBUSY)
return err;
return ahash_def_finup_finish1(req, err);
diff --git a/crypto/cts.c b/crypto/cts.c
index 243f591..4773c18 100644
--- a/crypto/cts.c
+++ b/crypto/cts.c
@@ -136,8 +136,7 @@ static void crypto_cts_encrypt_done(struct crypto_async_request *areq, int err)
goto out;
err = cts_cbc_encrypt(req);
- if (err == -EINPROGRESS ||
- (err == -EBUSY && req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+ if (err == -EINPROGRESS || err == -EBUSY)
return;
out:
@@ -229,8 +228,7 @@ static void crypto_cts_decrypt_done(struct crypto_async_request *areq, int err)
goto out;
err = cts_cbc_decrypt(req);
- if (err == -EINPROGRESS ||
- (err == -EBUSY && req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+ if (err == -EINPROGRESS || err == -EBUSY)
return;
out:
diff --git a/crypto/lrw.c b/crypto/lrw.c
index 92df312..cbbd7c5 100644
--- a/crypto/lrw.c
+++ b/crypto/lrw.c
@@ -328,9 +328,7 @@ static int do_encrypt(struct skcipher_request *req, int err)
crypto_skcipher_encrypt(subreq) ?:
post_crypt(req);
- if (err == -EINPROGRESS ||
- (err == -EBUSY &&
- req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+ if (err == -EINPROGRESS || err == -EBUSY)
return err;
}
@@ -380,9 +378,7 @@ static int do_decrypt(struct skcipher_request *req, int err)
crypto_skcipher_decrypt(subreq) ?:
post_crypt(req);
- if (err == -EINPROGRESS ||
- (err == -EBUSY &&
- req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+ if (err == -EINPROGRESS || err == -EBUSY)
return err;
}
diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index 407c64b..2908f93 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -279,9 +279,7 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
req->dst, ctx->key_size - 1, req->dst_len);
err = crypto_akcipher_encrypt(&req_ctx->child_req);
- if (err != -EINPROGRESS &&
- (err != -EBUSY ||
- !(req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)))
+ if (err != -EINPROGRESS && err != -EBUSY)
return pkcs1pad_encrypt_sign_complete(req, err);
return err;
@@ -383,9 +381,7 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
ctx->key_size);
err = crypto_akcipher_decrypt(&req_ctx->child_req);
- if (err != -EINPROGRESS &&
- (err != -EBUSY ||
- !(req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)))
+ if (err != -EINPROGRESS && err != -EBUSY)
return pkcs1pad_decrypt_complete(req, err);
return err;
@@ -440,9 +436,7 @@ static int pkcs1pad_sign(struct akcipher_request *req)
req->dst, ctx->key_size - 1, req->dst_len);
err = crypto_akcipher_sign(&req_ctx->child_req);
- if (err != -EINPROGRESS &&
- (err != -EBUSY ||
- !(req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)))
+ if (err != -EINPROGRESS && err != -EBUSY)
return pkcs1pad_encrypt_sign_complete(req, err);
return err;
@@ -561,9 +555,7 @@ static int pkcs1pad_verify(struct akcipher_request *req)
ctx->key_size);
err = crypto_akcipher_verify(&req_ctx->child_req);
- if (err != -EINPROGRESS &&
- (err != -EBUSY ||
- !(req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)))
+ if (err != -EINPROGRESS && err != -EBUSY)
return pkcs1pad_verify_complete(req, err);
return err;
diff --git a/crypto/xts.c b/crypto/xts.c
index e31828e..f317c48 100644
--- a/crypto/xts.c
+++ b/crypto/xts.c
@@ -269,9 +269,7 @@ static int do_encrypt(struct skcipher_request *req, int err)
crypto_skcipher_encrypt(subreq) ?:
post_crypt(req);
- if (err == -EINPROGRESS ||
- (err == -EBUSY &&
- req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+ if (err == -EINPROGRESS || err == -EBUSY)
return err;
}
@@ -321,9 +319,7 @@ static int do_decrypt(struct skcipher_request *req, int err)
crypto_skcipher_decrypt(subreq) ?:
post_crypt(req);
- if (err == -EINPROGRESS ||
- (err == -EBUSY &&
- req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+ if (err == -EINPROGRESS || err == -EBUSY)
return err;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v10 06/20] crypto: introduce crypto wait for async op
From: Gilad Ben-Yossef @ 2017-10-18 7:00 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Steffen Klassert,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin <dmitry.ka
Cc: Ofir Drang, Eric Biggers, Jonathan Cameron, linux-crypto,
linux-doc, linux-kernel, keyrings, linux-arm-kernel,
linux-mediatek, linux-cifs, samba-technical, linux-fscrypt,
netdev, linux-ima-devel, linux-ima-user, linux-security-module
In-Reply-To: <1508310057-25369-1-git-send-email-gilad@benyossef.com>
Invoking a possibly async. crypto op and waiting for completion
while correctly handling backlog processing is a common task
in the crypto API implementation and outside users of it.
This patch adds a generic implementation for doing so in
preparation for using it across the board instead of hand
rolled versions.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Eric Biggers <ebiggers3@gmail.com>
CC: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
crypto/api.c | 13 +++++++++++++
include/linux/crypto.h | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+)
diff --git a/crypto/api.c b/crypto/api.c
index 941cd4c..2a2479d 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -24,6 +24,7 @@
#include <linux/sched/signal.h>
#include <linux/slab.h>
#include <linux/string.h>
+#include <linux/completion.h>
#include "internal.h"
LIST_HEAD(crypto_alg_list);
@@ -595,5 +596,17 @@ int crypto_has_alg(const char *name, u32 type, u32 mask)
}
EXPORT_SYMBOL_GPL(crypto_has_alg);
+void crypto_req_done(struct crypto_async_request *req, int err)
+{
+ struct crypto_wait *wait = req->data;
+
+ if (err == -EINPROGRESS)
+ return;
+
+ wait->err = err;
+ complete(&wait->completion);
+}
+EXPORT_SYMBOL_GPL(crypto_req_done);
+
MODULE_DESCRIPTION("Cryptographic core API");
MODULE_LICENSE("GPL");
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 84da997..78508ca 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -24,6 +24,7 @@
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/uaccess.h>
+#include <linux/completion.h>
/*
* Autoloaded crypto modules should only use a prefixed name to avoid allowing
@@ -468,6 +469,45 @@ struct crypto_alg {
} CRYPTO_MINALIGN_ATTR;
/*
+ * A helper struct for waiting for completion of async crypto ops
+ */
+struct crypto_wait {
+ struct completion completion;
+ int err;
+};
+
+/*
+ * Macro for declaring a crypto op async wait object on stack
+ */
+#define DECLARE_CRYPTO_WAIT(_wait) \
+ struct crypto_wait _wait = { \
+ COMPLETION_INITIALIZER_ONSTACK((_wait).completion), 0 }
+
+/*
+ * Async ops completion helper functioons
+ */
+void crypto_req_done(struct crypto_async_request *req, int err);
+
+static inline int crypto_wait_req(int err, struct crypto_wait *wait)
+{
+ switch (err) {
+ case -EINPROGRESS:
+ case -EBUSY:
+ wait_for_completion(&wait->completion);
+ reinit_completion(&wait->completion);
+ err = wait->err;
+ break;
+ };
+
+ return err;
+}
+
+static inline void crypto_init_wait(struct crypto_wait *wait)
+{
+ init_completion(&wait->completion);
+}
+
+/*
* Algorithm registration interface.
*/
int crypto_register_alg(struct crypto_alg *alg);
--
2.7.4
^ permalink raw reply related
* [PATCH v10 07/20] crypto: move algif to generic async completion
From: Gilad Ben-Yossef @ 2017-10-18 7:00 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Steffen Klassert,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin <dmitry.ka
Cc: Ofir Drang, linux-crypto, linux-doc, linux-kernel, keyrings,
linux-arm-kernel, linux-mediatek, linux-cifs, samba-technical,
linux-fscrypt, netdev, linux-ima-devel, linux-ima-user,
linux-security-module
In-Reply-To: <1508310057-25369-1-git-send-email-gilad@benyossef.com>
algif starts several async crypto ops and waits for their completion.
Move it over to generic code doing the same.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
crypto/af_alg.c | 27 ---------------------------
crypto/algif_aead.c | 8 ++++----
crypto/algif_hash.c | 30 ++++++++++++++----------------
crypto/algif_skcipher.c | 9 ++++-----
include/crypto/if_alg.h | 15 +--------------
5 files changed, 23 insertions(+), 66 deletions(-)
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 337cf38..85cea9d 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -481,33 +481,6 @@ int af_alg_cmsg_send(struct msghdr *msg, struct af_alg_control *con)
}
EXPORT_SYMBOL_GPL(af_alg_cmsg_send);
-int af_alg_wait_for_completion(int err, struct af_alg_completion *completion)
-{
- switch (err) {
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&completion->completion);
- reinit_completion(&completion->completion);
- err = completion->err;
- break;
- };
-
- return err;
-}
-EXPORT_SYMBOL_GPL(af_alg_wait_for_completion);
-
-void af_alg_complete(struct crypto_async_request *req, int err)
-{
- struct af_alg_completion *completion = req->data;
-
- if (err == -EINPROGRESS)
- return;
-
- completion->err = err;
- complete(&completion->completion);
-}
-EXPORT_SYMBOL_GPL(af_alg_complete);
-
/**
* af_alg_alloc_tsgl - allocate the TX SGL
*
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 516b38c..aacae08 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -278,11 +278,11 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
/* Synchronous operation */
aead_request_set_callback(&areq->cra_u.aead_req,
CRYPTO_TFM_REQ_MAY_BACKLOG,
- af_alg_complete, &ctx->completion);
- err = af_alg_wait_for_completion(ctx->enc ?
+ crypto_req_done, &ctx->wait);
+ err = crypto_wait_req(ctx->enc ?
crypto_aead_encrypt(&areq->cra_u.aead_req) :
crypto_aead_decrypt(&areq->cra_u.aead_req),
- &ctx->completion);
+ &ctx->wait);
}
/* AIO operation in progress */
@@ -554,7 +554,7 @@ static int aead_accept_parent_nokey(void *private, struct sock *sk)
ctx->merge = 0;
ctx->enc = 0;
ctx->aead_assoclen = 0;
- af_alg_init_completion(&ctx->completion);
+ crypto_init_wait(&ctx->wait);
ask->private = ctx;
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 5e92bd2..76d2e71 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -26,7 +26,7 @@ struct hash_ctx {
u8 *result;
- struct af_alg_completion completion;
+ struct crypto_wait wait;
unsigned int len;
bool more;
@@ -88,8 +88,7 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg,
if ((msg->msg_flags & MSG_MORE))
hash_free_result(sk, ctx);
- err = af_alg_wait_for_completion(crypto_ahash_init(&ctx->req),
- &ctx->completion);
+ err = crypto_wait_req(crypto_ahash_init(&ctx->req), &ctx->wait);
if (err)
goto unlock;
}
@@ -110,8 +109,8 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg,
ahash_request_set_crypt(&ctx->req, ctx->sgl.sg, NULL, len);
- err = af_alg_wait_for_completion(crypto_ahash_update(&ctx->req),
- &ctx->completion);
+ err = crypto_wait_req(crypto_ahash_update(&ctx->req),
+ &ctx->wait);
af_alg_free_sg(&ctx->sgl);
if (err)
goto unlock;
@@ -129,8 +128,8 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg,
goto unlock;
ahash_request_set_crypt(&ctx->req, NULL, ctx->result, 0);
- err = af_alg_wait_for_completion(crypto_ahash_final(&ctx->req),
- &ctx->completion);
+ err = crypto_wait_req(crypto_ahash_final(&ctx->req),
+ &ctx->wait);
}
unlock:
@@ -171,7 +170,7 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
} else {
if (!ctx->more) {
err = crypto_ahash_init(&ctx->req);
- err = af_alg_wait_for_completion(err, &ctx->completion);
+ err = crypto_wait_req(err, &ctx->wait);
if (err)
goto unlock;
}
@@ -179,7 +178,7 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
err = crypto_ahash_update(&ctx->req);
}
- err = af_alg_wait_for_completion(err, &ctx->completion);
+ err = crypto_wait_req(err, &ctx->wait);
if (err)
goto unlock;
@@ -215,17 +214,16 @@ static int hash_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
ahash_request_set_crypt(&ctx->req, NULL, ctx->result, 0);
if (!result && !ctx->more) {
- err = af_alg_wait_for_completion(
- crypto_ahash_init(&ctx->req),
- &ctx->completion);
+ err = crypto_wait_req(crypto_ahash_init(&ctx->req),
+ &ctx->wait);
if (err)
goto unlock;
}
if (!result || ctx->more) {
ctx->more = 0;
- err = af_alg_wait_for_completion(crypto_ahash_final(&ctx->req),
- &ctx->completion);
+ err = crypto_wait_req(crypto_ahash_final(&ctx->req),
+ &ctx->wait);
if (err)
goto unlock;
}
@@ -476,13 +474,13 @@ static int hash_accept_parent_nokey(void *private, struct sock *sk)
ctx->result = NULL;
ctx->len = len;
ctx->more = 0;
- af_alg_init_completion(&ctx->completion);
+ crypto_init_wait(&ctx->wait);
ask->private = ctx;
ahash_request_set_tfm(&ctx->req, hash);
ahash_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- af_alg_complete, &ctx->completion);
+ crypto_req_done, &ctx->wait);
sk->sk_destruct = hash_sock_destruct;
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 8ae4170..9954b07 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -129,12 +129,11 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
skcipher_request_set_callback(&areq->cra_u.skcipher_req,
CRYPTO_TFM_REQ_MAY_SLEEP |
CRYPTO_TFM_REQ_MAY_BACKLOG,
- af_alg_complete,
- &ctx->completion);
- err = af_alg_wait_for_completion(ctx->enc ?
+ crypto_req_done, &ctx->wait);
+ err = crypto_wait_req(ctx->enc ?
crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) :
crypto_skcipher_decrypt(&areq->cra_u.skcipher_req),
- &ctx->completion);
+ &ctx->wait);
}
/* AIO operation in progress */
@@ -388,7 +387,7 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
ctx->more = 0;
ctx->merge = 0;
ctx->enc = 0;
- af_alg_init_completion(&ctx->completion);
+ crypto_init_wait(&ctx->wait);
ask->private = ctx;
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 75ec9c6..6abf0a3 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -40,11 +40,6 @@ struct alg_sock {
void *private;
};
-struct af_alg_completion {
- struct completion completion;
- int err;
-};
-
struct af_alg_control {
struct af_alg_iv *iv;
int op;
@@ -152,7 +147,7 @@ struct af_alg_ctx {
void *iv;
size_t aead_assoclen;
- struct af_alg_completion completion;
+ struct crypto_wait wait;
size_t used;
size_t rcvused;
@@ -177,19 +172,11 @@ void af_alg_link_sg(struct af_alg_sgl *sgl_prev, struct af_alg_sgl *sgl_new);
int af_alg_cmsg_send(struct msghdr *msg, struct af_alg_control *con);
-int af_alg_wait_for_completion(int err, struct af_alg_completion *completion);
-void af_alg_complete(struct crypto_async_request *req, int err);
-
static inline struct alg_sock *alg_sk(struct sock *sk)
{
return (struct alg_sock *)sk;
}
-static inline void af_alg_init_completion(struct af_alg_completion *completion)
-{
- init_completion(&completion->completion);
-}
-
/**
* Size of available buffer for sending data from user space to kernel.
*
--
2.7.4
^ permalink raw reply related
* [PATCH v10 09/20] crypto: move drbg to generic async completion
From: Gilad Ben-Yossef @ 2017-10-18 7:00 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Steffen Klassert,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin <dmitry.ka
Cc: Ofir Drang, linux-crypto, linux-doc, linux-kernel, keyrings,
linux-arm-kernel, linux-mediatek, linux-cifs, samba-technical,
linux-fscrypt, netdev, linux-ima-devel, linux-ima-user,
linux-security-module
In-Reply-To: <1508310057-25369-1-git-send-email-gilad@benyossef.com>
DRBG is starting an async. crypto op and waiting for it complete.
Move it over to generic code doing the same.
The code now also passes CRYPTO_TFM_REQ_MAY_SLEEP flag indicating
crypto request memory allocation may use GFP_KERNEL which should
be perfectly fine as the code is obviously sleeping for the
completion of the request any way.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
crypto/drbg.c | 36 +++++++++---------------------------
include/crypto/drbg.h | 3 +--
2 files changed, 10 insertions(+), 29 deletions(-)
diff --git a/crypto/drbg.c b/crypto/drbg.c
index 7001839..4faa278 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1651,16 +1651,6 @@ static int drbg_fini_sym_kernel(struct drbg_state *drbg)
return 0;
}
-static void drbg_skcipher_cb(struct crypto_async_request *req, int error)
-{
- struct drbg_state *drbg = req->data;
-
- if (error == -EINPROGRESS)
- return;
- drbg->ctr_async_err = error;
- complete(&drbg->ctr_completion);
-}
-
static int drbg_init_sym_kernel(struct drbg_state *drbg)
{
struct crypto_cipher *tfm;
@@ -1691,7 +1681,7 @@ static int drbg_init_sym_kernel(struct drbg_state *drbg)
return PTR_ERR(sk_tfm);
}
drbg->ctr_handle = sk_tfm;
- init_completion(&drbg->ctr_completion);
+ crypto_init_wait(&drbg->ctr_wait);
req = skcipher_request_alloc(sk_tfm, GFP_KERNEL);
if (!req) {
@@ -1700,8 +1690,9 @@ static int drbg_init_sym_kernel(struct drbg_state *drbg)
return -ENOMEM;
}
drbg->ctr_req = req;
- skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- drbg_skcipher_cb, drbg);
+ skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
+ CRYPTO_TFM_REQ_MAY_SLEEP,
+ crypto_req_done, &drbg->ctr_wait);
alignmask = crypto_skcipher_alignmask(sk_tfm);
drbg->ctr_null_value_buf = kzalloc(DRBG_CTR_NULL_LEN + alignmask,
@@ -1762,21 +1753,12 @@ static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
/* Output buffer may not be valid for SGL, use scratchpad */
skcipher_request_set_crypt(drbg->ctr_req, &sg_in, &sg_out,
cryptlen, drbg->V);
- ret = crypto_skcipher_encrypt(drbg->ctr_req);
- switch (ret) {
- case 0:
- break;
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&drbg->ctr_completion);
- if (!drbg->ctr_async_err) {
- reinit_completion(&drbg->ctr_completion);
- break;
- }
- default:
+ ret = crypto_wait_req(crypto_skcipher_encrypt(drbg->ctr_req),
+ &drbg->ctr_wait);
+ if (ret)
goto out;
- }
- init_completion(&drbg->ctr_completion);
+
+ crypto_init_wait(&drbg->ctr_wait);
memcpy(outbuf, drbg->outscratchpad, cryptlen);
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index 22f884c..8f94110 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -126,8 +126,7 @@ struct drbg_state {
__u8 *ctr_null_value; /* CTR mode aligned zero buf */
__u8 *outscratchpadbuf; /* CTR mode output scratchpad */
__u8 *outscratchpad; /* CTR mode aligned outbuf */
- struct completion ctr_completion; /* CTR mode async handler */
- int ctr_async_err; /* CTR mode async error */
+ struct crypto_wait ctr_wait; /* CTR mode async wait obj */
bool seeded; /* DRBG fully seeded? */
bool pr; /* Prediction resistance enabled? */
--
2.7.4
^ permalink raw reply related
* [PATCH v10 10/20] crypto: move gcm to generic async completion
From: Gilad Ben-Yossef @ 2017-10-18 7:00 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Steffen Klassert,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin <dmitry.ka
Cc: Ofir Drang, linux-crypto, linux-doc, linux-kernel, keyrings,
linux-arm-kernel, linux-mediatek, linux-cifs, samba-technical,
linux-fscrypt, netdev, linux-ima-devel, linux-ima-user,
linux-security-module
In-Reply-To: <1508310057-25369-1-git-send-email-gilad@benyossef.com>
gcm is starting an async. crypto op and waiting for it complete.
Move it over to generic code doing the same.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
crypto/gcm.c | 32 ++++++--------------------------
1 file changed, 6 insertions(+), 26 deletions(-)
diff --git a/crypto/gcm.c b/crypto/gcm.c
index 80cf6cf..8589681 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -17,7 +17,6 @@
#include <crypto/gcm.h>
#include <crypto/hash.h>
#include "internal.h"
-#include <linux/completion.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/kernel.h>
@@ -79,11 +78,6 @@ struct crypto_gcm_req_priv_ctx {
} u;
};
-struct crypto_gcm_setkey_result {
- int err;
- struct completion completion;
-};
-
static struct {
u8 buf[16];
struct scatterlist sg;
@@ -99,17 +93,6 @@ static inline struct crypto_gcm_req_priv_ctx *crypto_gcm_reqctx(
return (void *)PTR_ALIGN((u8 *)aead_request_ctx(req), align + 1);
}
-static void crypto_gcm_setkey_done(struct crypto_async_request *req, int err)
-{
- struct crypto_gcm_setkey_result *result = req->data;
-
- if (err == -EINPROGRESS)
- return;
-
- result->err = err;
- complete(&result->completion);
-}
-
static int crypto_gcm_setkey(struct crypto_aead *aead, const u8 *key,
unsigned int keylen)
{
@@ -120,7 +103,7 @@ static int crypto_gcm_setkey(struct crypto_aead *aead, const u8 *key,
be128 hash;
u8 iv[16];
- struct crypto_gcm_setkey_result result;
+ struct crypto_wait wait;
struct scatterlist sg[1];
struct skcipher_request req;
@@ -141,21 +124,18 @@ static int crypto_gcm_setkey(struct crypto_aead *aead, const u8 *key,
if (!data)
return -ENOMEM;
- init_completion(&data->result.completion);
+ crypto_init_wait(&data->wait);
sg_init_one(data->sg, &data->hash, sizeof(data->hash));
skcipher_request_set_tfm(&data->req, ctr);
skcipher_request_set_callback(&data->req, CRYPTO_TFM_REQ_MAY_SLEEP |
CRYPTO_TFM_REQ_MAY_BACKLOG,
- crypto_gcm_setkey_done,
- &data->result);
+ crypto_req_done,
+ &data->wait);
skcipher_request_set_crypt(&data->req, data->sg, data->sg,
sizeof(data->hash), data->iv);
- err = crypto_skcipher_encrypt(&data->req);
- if (err == -EINPROGRESS || err == -EBUSY) {
- wait_for_completion(&data->result.completion);
- err = data->result.err;
- }
+ err = crypto_wait_req(crypto_skcipher_encrypt(&data->req),
+ &data->wait);
if (err)
goto out;
--
2.7.4
^ permalink raw reply related
* [PATCH v10 11/20] crypto: move testmgr to generic async completion
From: Gilad Ben-Yossef @ 2017-10-18 7:00 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Steffen Klassert,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin <dmitry.ka
Cc: Ofir Drang, linux-crypto, linux-doc, linux-kernel, keyrings,
linux-arm-kernel, linux-mediatek, linux-cifs, samba-technical,
linux-fscrypt, netdev, linux-ima-devel, linux-ima-user,
linux-security-module
In-Reply-To: <1508310057-25369-1-git-send-email-gilad@benyossef.com>
testmgr is starting async. crypto ops and waiting for them to complete.
Move it over to generic code doing the same.
This also provides a test of the generic crypto async. wait code.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
crypto/testmgr.c | 204 ++++++++++++++++++-------------------------------------
1 file changed, 66 insertions(+), 138 deletions(-)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index dd9c7f1..3996fd5 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -76,11 +76,6 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
#define ENCRYPT 1
#define DECRYPT 0
-struct tcrypt_result {
- struct completion completion;
- int err;
-};
-
struct aead_test_suite {
struct {
const struct aead_testvec *vecs;
@@ -155,17 +150,6 @@ static void hexdump(unsigned char *buf, unsigned int len)
buf, len, false);
}
-static void tcrypt_complete(struct crypto_async_request *req, int err)
-{
- struct tcrypt_result *res = req->data;
-
- if (err == -EINPROGRESS)
- return;
-
- res->err = err;
- complete(&res->completion);
-}
-
static int testmgr_alloc_buf(char *buf[XBUFSIZE])
{
int i;
@@ -193,20 +177,10 @@ static void testmgr_free_buf(char *buf[XBUFSIZE])
free_page((unsigned long)buf[i]);
}
-static int wait_async_op(struct tcrypt_result *tr, int ret)
-{
- if (ret == -EINPROGRESS || ret == -EBUSY) {
- wait_for_completion(&tr->completion);
- reinit_completion(&tr->completion);
- ret = tr->err;
- }
- return ret;
-}
-
static int ahash_partial_update(struct ahash_request **preq,
struct crypto_ahash *tfm, const struct hash_testvec *template,
void *hash_buff, int k, int temp, struct scatterlist *sg,
- const char *algo, char *result, struct tcrypt_result *tresult)
+ const char *algo, char *result, struct crypto_wait *wait)
{
char *state;
struct ahash_request *req;
@@ -236,7 +210,7 @@ static int ahash_partial_update(struct ahash_request **preq,
}
ahash_request_set_callback(req,
CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, tresult);
+ crypto_req_done, wait);
memcpy(hash_buff, template->plaintext + temp,
template->tap[k]);
@@ -247,7 +221,7 @@ static int ahash_partial_update(struct ahash_request **preq,
pr_err("alg: hash: Failed to import() for %s\n", algo);
goto out;
}
- ret = wait_async_op(tresult, crypto_ahash_update(req));
+ ret = crypto_wait_req(crypto_ahash_update(req), wait);
if (ret)
goto out;
*preq = req;
@@ -272,7 +246,7 @@ static int __test_hash(struct crypto_ahash *tfm,
char *result;
char *key;
struct ahash_request *req;
- struct tcrypt_result tresult;
+ struct crypto_wait wait;
void *hash_buff;
char *xbuf[XBUFSIZE];
int ret = -ENOMEM;
@@ -286,7 +260,7 @@ static int __test_hash(struct crypto_ahash *tfm,
if (testmgr_alloc_buf(xbuf))
goto out_nobuf;
- init_completion(&tresult.completion);
+ crypto_init_wait(&wait);
req = ahash_request_alloc(tfm, GFP_KERNEL);
if (!req) {
@@ -295,7 +269,7 @@ static int __test_hash(struct crypto_ahash *tfm,
goto out_noreq;
}
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &tresult);
+ crypto_req_done, &wait);
j = 0;
for (i = 0; i < tcount; i++) {
@@ -335,26 +309,26 @@ static int __test_hash(struct crypto_ahash *tfm,
ahash_request_set_crypt(req, sg, result, template[i].psize);
if (use_digest) {
- ret = wait_async_op(&tresult, crypto_ahash_digest(req));
+ ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
if (ret) {
pr_err("alg: hash: digest failed on test %d "
"for %s: ret=%d\n", j, algo, -ret);
goto out;
}
} else {
- ret = wait_async_op(&tresult, crypto_ahash_init(req));
+ ret = crypto_wait_req(crypto_ahash_init(req), &wait);
if (ret) {
pr_err("alg: hash: init failed on test %d "
"for %s: ret=%d\n", j, algo, -ret);
goto out;
}
- ret = wait_async_op(&tresult, crypto_ahash_update(req));
+ ret = crypto_wait_req(crypto_ahash_update(req), &wait);
if (ret) {
pr_err("alg: hash: update failed on test %d "
"for %s: ret=%d\n", j, algo, -ret);
goto out;
}
- ret = wait_async_op(&tresult, crypto_ahash_final(req));
+ ret = crypto_wait_req(crypto_ahash_final(req), &wait);
if (ret) {
pr_err("alg: hash: final failed on test %d "
"for %s: ret=%d\n", j, algo, -ret);
@@ -420,22 +394,10 @@ static int __test_hash(struct crypto_ahash *tfm,
}
ahash_request_set_crypt(req, sg, result, template[i].psize);
- ret = crypto_ahash_digest(req);
- switch (ret) {
- case 0:
- break;
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&tresult.completion);
- reinit_completion(&tresult.completion);
- ret = tresult.err;
- if (!ret)
- break;
- /* fall through */
- default:
- printk(KERN_ERR "alg: hash: digest failed "
- "on chunking test %d for %s: "
- "ret=%d\n", j, algo, -ret);
+ ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
+ if (ret) {
+ pr_err("alg: hash: digest failed on chunking test %d for %s: ret=%d\n",
+ j, algo, -ret);
goto out;
}
@@ -486,13 +448,13 @@ static int __test_hash(struct crypto_ahash *tfm,
}
ahash_request_set_crypt(req, sg, result, template[i].tap[0]);
- ret = wait_async_op(&tresult, crypto_ahash_init(req));
+ ret = crypto_wait_req(crypto_ahash_init(req), &wait);
if (ret) {
pr_err("alg: hash: init failed on test %d for %s: ret=%d\n",
j, algo, -ret);
goto out;
}
- ret = wait_async_op(&tresult, crypto_ahash_update(req));
+ ret = crypto_wait_req(crypto_ahash_update(req), &wait);
if (ret) {
pr_err("alg: hash: update failed on test %d for %s: ret=%d\n",
j, algo, -ret);
@@ -503,7 +465,7 @@ static int __test_hash(struct crypto_ahash *tfm,
for (k = 1; k < template[i].np; k++) {
ret = ahash_partial_update(&req, tfm, &template[i],
hash_buff, k, temp, &sg[0], algo, result,
- &tresult);
+ &wait);
if (ret) {
pr_err("alg: hash: partial update failed on test %d for %s: ret=%d\n",
j, algo, -ret);
@@ -511,7 +473,7 @@ static int __test_hash(struct crypto_ahash *tfm,
}
temp += template[i].tap[k];
}
- ret = wait_async_op(&tresult, crypto_ahash_final(req));
+ ret = crypto_wait_req(crypto_ahash_final(req), &wait);
if (ret) {
pr_err("alg: hash: final failed on test %d for %s: ret=%d\n",
j, algo, -ret);
@@ -580,7 +542,7 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
struct scatterlist *sg;
struct scatterlist *sgout;
const char *e, *d;
- struct tcrypt_result result;
+ struct crypto_wait wait;
unsigned int authsize, iv_len;
void *input;
void *output;
@@ -619,7 +581,7 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
else
e = "decryption";
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
req = aead_request_alloc(tfm, GFP_KERNEL);
if (!req) {
@@ -629,7 +591,7 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
}
aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
+ crypto_req_done, &wait);
iv_len = crypto_aead_ivsize(tfm);
@@ -709,7 +671,8 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
aead_request_set_ad(req, template[i].alen);
- ret = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
+ ret = crypto_wait_req(enc ? crypto_aead_encrypt(req)
+ : crypto_aead_decrypt(req), &wait);
switch (ret) {
case 0:
@@ -722,13 +685,6 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
goto out;
}
break;
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&result.completion);
- reinit_completion(&result.completion);
- ret = result.err;
- if (!ret)
- break;
case -EBADMSG:
if (template[i].novrfy)
/* verification failure was expected */
@@ -866,7 +822,8 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
aead_request_set_ad(req, template[i].alen);
- ret = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
+ ret = crypto_wait_req(enc ? crypto_aead_encrypt(req)
+ : crypto_aead_decrypt(req), &wait);
switch (ret) {
case 0:
@@ -879,13 +836,6 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
goto out;
}
break;
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&result.completion);
- reinit_completion(&result.completion);
- ret = result.err;
- if (!ret)
- break;
case -EBADMSG:
if (template[i].novrfy)
/* verification failure was expected */
@@ -1083,7 +1033,7 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
struct scatterlist sg[8];
struct scatterlist sgout[8];
const char *e, *d;
- struct tcrypt_result result;
+ struct crypto_wait wait;
void *data;
char iv[MAX_IVLEN];
char *xbuf[XBUFSIZE];
@@ -1107,7 +1057,7 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
else
e = "decryption";
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
req = skcipher_request_alloc(tfm, GFP_KERNEL);
if (!req) {
@@ -1117,7 +1067,7 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
}
skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
+ crypto_req_done, &wait);
j = 0;
for (i = 0; i < tcount; i++) {
@@ -1164,21 +1114,10 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
skcipher_request_set_crypt(req, sg, (diff_dst) ? sgout : sg,
template[i].ilen, iv);
- ret = enc ? crypto_skcipher_encrypt(req) :
- crypto_skcipher_decrypt(req);
+ ret = crypto_wait_req(enc ? crypto_skcipher_encrypt(req) :
+ crypto_skcipher_decrypt(req), &wait);
- switch (ret) {
- case 0:
- break;
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&result.completion);
- reinit_completion(&result.completion);
- ret = result.err;
- if (!ret)
- break;
- /* fall through */
- default:
+ if (ret) {
pr_err("alg: skcipher%s: %s failed on test %d for %s: ret=%d\n",
d, e, j, algo, -ret);
goto out;
@@ -1272,21 +1211,10 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
skcipher_request_set_crypt(req, sg, (diff_dst) ? sgout : sg,
template[i].ilen, iv);
- ret = enc ? crypto_skcipher_encrypt(req) :
- crypto_skcipher_decrypt(req);
+ ret = crypto_wait_req(enc ? crypto_skcipher_encrypt(req) :
+ crypto_skcipher_decrypt(req), &wait);
- switch (ret) {
- case 0:
- break;
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&result.completion);
- reinit_completion(&result.completion);
- ret = result.err;
- if (!ret)
- break;
- /* fall through */
- default:
+ if (ret) {
pr_err("alg: skcipher%s: %s failed on chunk test %d for %s: ret=%d\n",
d, e, j, algo, -ret);
goto out;
@@ -1462,7 +1390,7 @@ static int test_acomp(struct crypto_acomp *tfm,
int ret;
struct scatterlist src, dst;
struct acomp_req *req;
- struct tcrypt_result result;
+ struct crypto_wait wait;
output = kmalloc(COMP_BUF_SIZE, GFP_KERNEL);
if (!output)
@@ -1486,7 +1414,7 @@ static int test_acomp(struct crypto_acomp *tfm,
}
memset(output, 0, dlen);
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
sg_init_one(&src, input_vec, ilen);
sg_init_one(&dst, output, dlen);
@@ -1501,9 +1429,9 @@ static int test_acomp(struct crypto_acomp *tfm,
acomp_request_set_params(req, &src, &dst, ilen, dlen);
acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
+ crypto_req_done, &wait);
- ret = wait_async_op(&result, crypto_acomp_compress(req));
+ ret = crypto_wait_req(crypto_acomp_compress(req), &wait);
if (ret) {
pr_err("alg: acomp: compression failed on test %d for %s: ret=%d\n",
i + 1, algo, -ret);
@@ -1516,10 +1444,10 @@ static int test_acomp(struct crypto_acomp *tfm,
dlen = COMP_BUF_SIZE;
sg_init_one(&src, output, ilen);
sg_init_one(&dst, decomp_out, dlen);
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
acomp_request_set_params(req, &src, &dst, ilen, dlen);
- ret = wait_async_op(&result, crypto_acomp_decompress(req));
+ ret = crypto_wait_req(crypto_acomp_decompress(req), &wait);
if (ret) {
pr_err("alg: acomp: compression failed on test %d for %s: ret=%d\n",
i + 1, algo, -ret);
@@ -1563,7 +1491,7 @@ static int test_acomp(struct crypto_acomp *tfm,
}
memset(output, 0, dlen);
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
sg_init_one(&src, input_vec, ilen);
sg_init_one(&dst, output, dlen);
@@ -1578,9 +1506,9 @@ static int test_acomp(struct crypto_acomp *tfm,
acomp_request_set_params(req, &src, &dst, ilen, dlen);
acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
+ crypto_req_done, &wait);
- ret = wait_async_op(&result, crypto_acomp_decompress(req));
+ ret = crypto_wait_req(crypto_acomp_decompress(req), &wait);
if (ret) {
pr_err("alg: acomp: decompression failed on test %d for %s: ret=%d\n",
i + 1, algo, -ret);
@@ -2000,7 +1928,7 @@ static int do_test_kpp(struct crypto_kpp *tfm, const struct kpp_testvec *vec,
void *a_public = NULL;
void *a_ss = NULL;
void *shared_secret = NULL;
- struct tcrypt_result result;
+ struct crypto_wait wait;
unsigned int out_len_max;
int err = -ENOMEM;
struct scatterlist src, dst;
@@ -2009,7 +1937,7 @@ static int do_test_kpp(struct crypto_kpp *tfm, const struct kpp_testvec *vec,
if (!req)
return err;
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
err = crypto_kpp_set_secret(tfm, vec->secret, vec->secret_size);
if (err < 0)
@@ -2027,10 +1955,10 @@ static int do_test_kpp(struct crypto_kpp *tfm, const struct kpp_testvec *vec,
sg_init_one(&dst, output_buf, out_len_max);
kpp_request_set_output(req, &dst, out_len_max);
kpp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
+ crypto_req_done, &wait);
/* Compute party A's public key */
- err = wait_async_op(&result, crypto_kpp_generate_public_key(req));
+ err = crypto_wait_req(crypto_kpp_generate_public_key(req), &wait);
if (err) {
pr_err("alg: %s: Party A: generate public key test failed. err %d\n",
alg, err);
@@ -2069,8 +1997,8 @@ static int do_test_kpp(struct crypto_kpp *tfm, const struct kpp_testvec *vec,
kpp_request_set_input(req, &src, vec->b_public_size);
kpp_request_set_output(req, &dst, out_len_max);
kpp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
- err = wait_async_op(&result, crypto_kpp_compute_shared_secret(req));
+ crypto_req_done, &wait);
+ err = crypto_wait_req(crypto_kpp_compute_shared_secret(req), &wait);
if (err) {
pr_err("alg: %s: Party A: compute shared secret test failed. err %d\n",
alg, err);
@@ -2100,9 +2028,9 @@ static int do_test_kpp(struct crypto_kpp *tfm, const struct kpp_testvec *vec,
kpp_request_set_input(req, &src, vec->expected_a_public_size);
kpp_request_set_output(req, &dst, out_len_max);
kpp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
- err = wait_async_op(&result,
- crypto_kpp_compute_shared_secret(req));
+ crypto_req_done, &wait);
+ err = crypto_wait_req(crypto_kpp_compute_shared_secret(req),
+ &wait);
if (err) {
pr_err("alg: %s: Party B: compute shared secret failed. err %d\n",
alg, err);
@@ -2179,7 +2107,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm,
struct akcipher_request *req;
void *outbuf_enc = NULL;
void *outbuf_dec = NULL;
- struct tcrypt_result result;
+ struct crypto_wait wait;
unsigned int out_len_max, out_len = 0;
int err = -ENOMEM;
struct scatterlist src, dst, src_tab[2];
@@ -2191,7 +2119,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm,
if (!req)
goto free_xbuf;
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
if (vecs->public_key_vec)
err = crypto_akcipher_set_pub_key(tfm, vecs->key,
@@ -2220,13 +2148,13 @@ static int test_akcipher_one(struct crypto_akcipher *tfm,
akcipher_request_set_crypt(req, src_tab, &dst, vecs->m_size,
out_len_max);
akcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
+ crypto_req_done, &wait);
- err = wait_async_op(&result, vecs->siggen_sigver_test ?
- /* Run asymmetric signature generation */
- crypto_akcipher_sign(req) :
- /* Run asymmetric encrypt */
- crypto_akcipher_encrypt(req));
+ err = crypto_wait_req(vecs->siggen_sigver_test ?
+ /* Run asymmetric signature generation */
+ crypto_akcipher_sign(req) :
+ /* Run asymmetric encrypt */
+ crypto_akcipher_encrypt(req), &wait);
if (err) {
pr_err("alg: akcipher: encrypt test failed. err %d\n", err);
goto free_all;
@@ -2261,14 +2189,14 @@ static int test_akcipher_one(struct crypto_akcipher *tfm,
sg_init_one(&src, xbuf[0], vecs->c_size);
sg_init_one(&dst, outbuf_dec, out_len_max);
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
akcipher_request_set_crypt(req, &src, &dst, vecs->c_size, out_len_max);
- err = wait_async_op(&result, vecs->siggen_sigver_test ?
- /* Run asymmetric signature verification */
- crypto_akcipher_verify(req) :
- /* Run asymmetric decrypt */
- crypto_akcipher_decrypt(req));
+ err = crypto_wait_req(vecs->siggen_sigver_test ?
+ /* Run asymmetric signature verification */
+ crypto_akcipher_verify(req) :
+ /* Run asymmetric decrypt */
+ crypto_akcipher_decrypt(req), &wait);
if (err) {
pr_err("alg: akcipher: decrypt test failed. err %d\n", err);
goto free_all;
--
2.7.4
^ permalink raw reply related
* [PATCH v10 13/20] dm: move dm-verity to generic async completion
From: Gilad Ben-Yossef @ 2017-10-18 7:00 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Steffen Klassert,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin <dmitry.ka
Cc: Ofir Drang, Mikulas Patocka, linux-crypto, linux-doc,
linux-kernel, keyrings, linux-arm-kernel, linux-mediatek,
linux-cifs, samba-technical, linux-fscrypt, netdev,
linux-ima-devel, linux-ima-user, linux-security-module
In-Reply-To: <1508310057-25369-1-git-send-email-gilad@benyossef.com>
dm-verity is starting async. crypto ops and waiting for them to complete.
Move it over to generic code doing the same.
This also avoids a future potential data coruption bug created
by the use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing, should this code ever move to a context
where signals are not masked.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Mikulas Patocka <mpatocka@redhat.com>
---
drivers/md/dm-verity-target.c | 81 +++++++++++--------------------------------
drivers/md/dm-verity.h | 5 ---
2 files changed, 20 insertions(+), 66 deletions(-)
diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index bda3cac..811ad28 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -92,74 +92,33 @@ static sector_t verity_position_at_level(struct dm_verity *v, sector_t block,
return block >> (level * v->hash_per_block_bits);
}
-/*
- * Callback function for asynchrnous crypto API completion notification
- */
-static void verity_op_done(struct crypto_async_request *base, int err)
-{
- struct verity_result *res = (struct verity_result *)base->data;
-
- if (err == -EINPROGRESS)
- return;
-
- res->err = err;
- complete(&res->completion);
-}
-
-/*
- * Wait for async crypto API callback
- */
-static inline int verity_complete_op(struct verity_result *res, int ret)
-{
- switch (ret) {
- case 0:
- break;
-
- case -EINPROGRESS:
- case -EBUSY:
- ret = wait_for_completion_interruptible(&res->completion);
- if (!ret)
- ret = res->err;
- reinit_completion(&res->completion);
- break;
-
- default:
- DMERR("verity_wait_hash: crypto op submission failed: %d", ret);
- }
-
- if (unlikely(ret < 0))
- DMERR("verity_wait_hash: crypto op failed: %d", ret);
-
- return ret;
-}
-
static int verity_hash_update(struct dm_verity *v, struct ahash_request *req,
const u8 *data, size_t len,
- struct verity_result *res)
+ struct crypto_wait *wait)
{
struct scatterlist sg;
sg_init_one(&sg, data, len);
ahash_request_set_crypt(req, &sg, NULL, len);
- return verity_complete_op(res, crypto_ahash_update(req));
+ return crypto_wait_req(crypto_ahash_update(req), wait);
}
/*
* Wrapper for crypto_ahash_init, which handles verity salting.
*/
static int verity_hash_init(struct dm_verity *v, struct ahash_request *req,
- struct verity_result *res)
+ struct crypto_wait *wait)
{
int r;
ahash_request_set_tfm(req, v->tfm);
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP |
CRYPTO_TFM_REQ_MAY_BACKLOG,
- verity_op_done, (void *)res);
- init_completion(&res->completion);
+ crypto_req_done, (void *)wait);
+ crypto_init_wait(wait);
- r = verity_complete_op(res, crypto_ahash_init(req));
+ r = crypto_wait_req(crypto_ahash_init(req), wait);
if (unlikely(r < 0)) {
DMERR("crypto_ahash_init failed: %d", r);
@@ -167,18 +126,18 @@ static int verity_hash_init(struct dm_verity *v, struct ahash_request *req,
}
if (likely(v->salt_size && (v->version >= 1)))
- r = verity_hash_update(v, req, v->salt, v->salt_size, res);
+ r = verity_hash_update(v, req, v->salt, v->salt_size, wait);
return r;
}
static int verity_hash_final(struct dm_verity *v, struct ahash_request *req,
- u8 *digest, struct verity_result *res)
+ u8 *digest, struct crypto_wait *wait)
{
int r;
if (unlikely(v->salt_size && (!v->version))) {
- r = verity_hash_update(v, req, v->salt, v->salt_size, res);
+ r = verity_hash_update(v, req, v->salt, v->salt_size, wait);
if (r < 0) {
DMERR("verity_hash_final failed updating salt: %d", r);
@@ -187,7 +146,7 @@ static int verity_hash_final(struct dm_verity *v, struct ahash_request *req,
}
ahash_request_set_crypt(req, NULL, digest, 0);
- r = verity_complete_op(res, crypto_ahash_final(req));
+ r = crypto_wait_req(crypto_ahash_final(req), wait);
out:
return r;
}
@@ -196,17 +155,17 @@ int verity_hash(struct dm_verity *v, struct ahash_request *req,
const u8 *data, size_t len, u8 *digest)
{
int r;
- struct verity_result res;
+ struct crypto_wait wait;
- r = verity_hash_init(v, req, &res);
+ r = verity_hash_init(v, req, &wait);
if (unlikely(r < 0))
goto out;
- r = verity_hash_update(v, req, data, len, &res);
+ r = verity_hash_update(v, req, data, len, &wait);
if (unlikely(r < 0))
goto out;
- r = verity_hash_final(v, req, digest, &res);
+ r = verity_hash_final(v, req, digest, &wait);
out:
return r;
@@ -389,7 +348,7 @@ int verity_hash_for_block(struct dm_verity *v, struct dm_verity_io *io,
* Calculates the digest for the given bio
*/
int verity_for_io_block(struct dm_verity *v, struct dm_verity_io *io,
- struct bvec_iter *iter, struct verity_result *res)
+ struct bvec_iter *iter, struct crypto_wait *wait)
{
unsigned int todo = 1 << v->data_dev_block_bits;
struct bio *bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size);
@@ -414,7 +373,7 @@ int verity_for_io_block(struct dm_verity *v, struct dm_verity_io *io,
*/
sg_set_page(&sg, bv.bv_page, len, bv.bv_offset);
ahash_request_set_crypt(req, &sg, NULL, len);
- r = verity_complete_op(res, crypto_ahash_update(req));
+ r = crypto_wait_req(crypto_ahash_update(req), wait);
if (unlikely(r < 0)) {
DMERR("verity_for_io_block crypto op failed: %d", r);
@@ -482,7 +441,7 @@ static int verity_verify_io(struct dm_verity_io *io)
struct dm_verity *v = io->v;
struct bvec_iter start;
unsigned b;
- struct verity_result res;
+ struct crypto_wait wait;
for (b = 0; b < io->n_blocks; b++) {
int r;
@@ -507,17 +466,17 @@ static int verity_verify_io(struct dm_verity_io *io)
continue;
}
- r = verity_hash_init(v, req, &res);
+ r = verity_hash_init(v, req, &wait);
if (unlikely(r < 0))
return r;
start = io->iter;
- r = verity_for_io_block(v, io, &io->iter, &res);
+ r = verity_for_io_block(v, io, &io->iter, &wait);
if (unlikely(r < 0))
return r;
r = verity_hash_final(v, req, verity_io_real_digest(v, io),
- &res);
+ &wait);
if (unlikely(r < 0))
return r;
diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h
index a59e0ad..b675bc0 100644
--- a/drivers/md/dm-verity.h
+++ b/drivers/md/dm-verity.h
@@ -90,11 +90,6 @@ struct dm_verity_io {
*/
};
-struct verity_result {
- struct completion completion;
- int err;
-};
-
static inline struct ahash_request *verity_io_hash_req(struct dm_verity *v,
struct dm_verity_io *io)
{
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox