Re: [PATCH ipsec-next 1/1] xfrm: don't check offload_handle for nonzero

2018-07-22 Thread Steffen Klassert
On Tue, Jun 26, 2018 at 02:19:10PM -0700, Shannon Nelson wrote:
> The offload_handle should be an opaque data cookie for the driver
> to use, much like the data cookie for a timer or alarm callback.
> Thus, the XFRM stack should not be checking for non-zero, because
> the driver might use that to store an array reference, which could
> be zero, or some other zero but meaningful value.
> 
> We can remove the checks for non-zero because there are plenty
> other attributes also being checked to see if there is an offload
> in place for the SA in question.
> 
> Signed-off-by: Shannon Nelson 

Applied to ipsec-next, thanks Shannon!


Re: [PATCH ipsec-next 1/1] xfrm: don't check offload_handle for nonzero

2018-06-27 Thread Steffen Klassert
On Tue, Jun 26, 2018 at 02:19:10PM -0700, Shannon Nelson wrote:
> The offload_handle should be an opaque data cookie for the driver
> to use, much like the data cookie for a timer or alarm callback.
> Thus, the XFRM stack should not be checking for non-zero, because
> the driver might use that to store an array reference, which could
> be zero, or some other zero but meaningful value.
> 
> We can remove the checks for non-zero because there are plenty
> other attributes also being checked to see if there is an offload
> in place for the SA in question.
> 
> Signed-off-by: Shannon Nelson 

I don't have access to my hw offload testlab currently,
so I've queued this one until I'm back from my vacation.


[PATCH ipsec-next 1/1] xfrm: don't check offload_handle for nonzero

2018-06-26 Thread Shannon Nelson
The offload_handle should be an opaque data cookie for the driver
to use, much like the data cookie for a timer or alarm callback.
Thus, the XFRM stack should not be checking for non-zero, because
the driver might use that to store an array reference, which could
be zero, or some other zero but meaningful value.

We can remove the checks for non-zero because there are plenty
other attributes also being checked to see if there is an offload
in place for the SA in question.

Signed-off-by: Shannon Nelson 
---
 net/ipv4/esp4_offload.c | 6 ++
 net/ipv6/esp6_offload.c | 6 ++
 net/xfrm/xfrm_device.c  | 6 +++---
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index bbeecd1..58834a1 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -135,8 +135,7 @@ static struct sk_buff *esp4_gso_segment(struct sk_buff *skb,
 
skb->encap_hdr_csum = 1;
 
-   if (!(features & NETIF_F_HW_ESP) || !x->xso.offload_handle ||
-   (x->xso.dev != skb->dev))
+   if (!(features & NETIF_F_HW_ESP) || x->xso.dev != skb->dev)
esp_features = features & ~(NETIF_F_SG | NETIF_F_CSUM_MASK);
else if (!(features & NETIF_F_HW_ESP_TX_CSUM))
esp_features = features & ~NETIF_F_CSUM_MASK;
@@ -179,8 +178,7 @@ static int esp_xmit(struct xfrm_state *x, struct sk_buff 
*skb,  netdev_features_
if (!xo)
return -EINVAL;
 
-   if (!(features & NETIF_F_HW_ESP) || !x->xso.offload_handle ||
-   (x->xso.dev != skb->dev)) {
+   if (!(features & NETIF_F_HW_ESP) || x->xso.dev != skb->dev) {
xo->flags |= CRYPTO_FALLBACK;
hw_offload = false;
}
diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index ddfa533..6177e21 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -162,8 +162,7 @@ static struct sk_buff *esp6_gso_segment(struct sk_buff *skb,
 
skb->encap_hdr_csum = 1;
 
-   if (!(features & NETIF_F_HW_ESP) || !x->xso.offload_handle ||
-   (x->xso.dev != skb->dev))
+   if (!(features & NETIF_F_HW_ESP) || x->xso.dev != skb->dev)
esp_features = features & ~(NETIF_F_SG | NETIF_F_CSUM_MASK);
else if (!(features & NETIF_F_HW_ESP_TX_CSUM))
esp_features = features & ~NETIF_F_CSUM_MASK;
@@ -207,8 +206,7 @@ static int esp6_xmit(struct xfrm_state *x, struct sk_buff 
*skb,  netdev_features
if (!xo)
return -EINVAL;
 
-   if (!(features & NETIF_F_HW_ESP) || !x->xso.offload_handle ||
-   (x->xso.dev != skb->dev)) {
+   if (!(features & NETIF_F_HW_ESP) || x->xso.dev != skb->dev) {
xo->flags |= CRYPTO_FALLBACK;
hw_offload = false;
}
diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index 175941e..9265dd6 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -56,7 +56,7 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, 
netdev_features_t featur
if (skb_is_gso(skb)) {
struct net_device *dev = skb->dev;
 
-   if (unlikely(!x->xso.offload_handle || (x->xso.dev != dev))) {
+   if (unlikely(x->xso.dev != dev)) {
struct sk_buff *segs;
 
/* Packet got rerouted, fixup features and segment it. 
*/
@@ -210,8 +210,8 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct 
xfrm_state *x)
if (!x->type_offload || x->encap)
return false;
 
-   if ((!dev || (x->xso.offload_handle && (dev == 
xfrm_dst_path(dst)->dev))) &&
-(!xdst->child->xfrm && x->type->get_mtu)) {
+   if ((!dev || (dev == xfrm_dst_path(dst)->dev)) &&
+   (!xdst->child->xfrm && x->type->get_mtu)) {
mtu = x->type->get_mtu(x, xdst->child_mtu_cached);
 
if (skb->len <= mtu)
-- 
2.7.4