Re: [PATCH v3] core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors
On 20/03/14 22:22, Thomas Graf wrote:
On 03/20/2014 05:02 PM, Zoltan Kiss wrote:
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -464,7 +464,9 @@ static int queue_userspace_packet(struct datapath
*dp, struct sk_buff *skb,
}
nla->nla_len = nla_attr_size(skb->len);
-skb_zerocopy(user_skb, skb, skb->len, hlen);
+err = skb_zerocopy(user_skb, skb, skb->len, hlen);
+if (err)
+goto out;
/* Pad OVS_PACKET_ATTR_PACKET if linear copy was performed */
if (!(dp->user_features & OVS_DP_F_UNALIGNED)) {
@@ -478,6 +480,7 @@ static int queue_userspace_packet(struct datapath
*dp, struct sk_buff *skb,
err = genlmsg_unicast(ovs_dp_get_net(dp), user_skb,
upcall_info->portid);
out:
+skb_tx_error(skb);
Really sorry to bug you again. We'll get this right ;-)
Patch looks great except for this call to skb_tx_error() which is now
done in the error *and* success path. Make the call conditional on
if (err) and we're good.
Ah, sorry, I'm really sloppy with this patch. I've sent a fixed one.
Zoli
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors
On 20/03/14 22:22, Thomas Graf wrote:
On 03/20/2014 05:02 PM, Zoltan Kiss wrote:
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -464,7 +464,9 @@ static int queue_userspace_packet(struct datapath
*dp, struct sk_buff *skb,
}
nla-nla_len = nla_attr_size(skb-len);
-skb_zerocopy(user_skb, skb, skb-len, hlen);
+err = skb_zerocopy(user_skb, skb, skb-len, hlen);
+if (err)
+goto out;
/* Pad OVS_PACKET_ATTR_PACKET if linear copy was performed */
if (!(dp-user_features OVS_DP_F_UNALIGNED)) {
@@ -478,6 +480,7 @@ static int queue_userspace_packet(struct datapath
*dp, struct sk_buff *skb,
err = genlmsg_unicast(ovs_dp_get_net(dp), user_skb,
upcall_info-portid);
out:
+skb_tx_error(skb);
Really sorry to bug you again. We'll get this right ;-)
Patch looks great except for this call to skb_tx_error() which is now
done in the error *and* success path. Make the call conditional on
if (err) and we're good.
Ah, sorry, I'm really sloppy with this patch. I've sent a fixed one.
Zoli
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors
On 03/20/2014 05:02 PM, Zoltan Kiss wrote:
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -464,7 +464,9 @@ static int queue_userspace_packet(struct datapath *dp,
struct sk_buff *skb,
}
nla->nla_len = nla_attr_size(skb->len);
- skb_zerocopy(user_skb, skb, skb->len, hlen);
+ err = skb_zerocopy(user_skb, skb, skb->len, hlen);
+ if (err)
+ goto out;
/* Pad OVS_PACKET_ATTR_PACKET if linear copy was performed */
if (!(dp->user_features & OVS_DP_F_UNALIGNED)) {
@@ -478,6 +480,7 @@ static int queue_userspace_packet(struct datapath *dp,
struct sk_buff *skb,
err = genlmsg_unicast(ovs_dp_get_net(dp), user_skb,
upcall_info->portid);
out:
+ skb_tx_error(skb);
Really sorry to bug you again. We'll get this right ;-)
Patch looks great except for this call to skb_tx_error() which is now
done in the error *and* success path. Make the call conditional on
if (err) and we're good.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors
From: Zoltan Kiss Date: Thu, 20 Mar 2014 16:02:34 + > skb_zerocopy can copy elements of the frags array between skbs, but it doesn't > orphan them. Also, it doesn't handle errors, so this patch takes care of that > as well, and modify the callers accordingly. skb_tx_error() is also added to > the callers so they will signal the failed delivery towards the creator of the > skb. > > Signed-off-by: Zoltan Kiss I'll wait for Thomas to review this new version before applying. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors
From: Zoltan Kiss zoltan.k...@citrix.com Date: Thu, 20 Mar 2014 16:02:34 + skb_zerocopy can copy elements of the frags array between skbs, but it doesn't orphan them. Also, it doesn't handle errors, so this patch takes care of that as well, and modify the callers accordingly. skb_tx_error() is also added to the callers so they will signal the failed delivery towards the creator of the skb. Signed-off-by: Zoltan Kiss zoltan.k...@citrix.com I'll wait for Thomas to review this new version before applying. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors
On 03/20/2014 05:02 PM, Zoltan Kiss wrote:
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -464,7 +464,9 @@ static int queue_userspace_packet(struct datapath *dp,
struct sk_buff *skb,
}
nla-nla_len = nla_attr_size(skb-len);
- skb_zerocopy(user_skb, skb, skb-len, hlen);
+ err = skb_zerocopy(user_skb, skb, skb-len, hlen);
+ if (err)
+ goto out;
/* Pad OVS_PACKET_ATTR_PACKET if linear copy was performed */
if (!(dp-user_features OVS_DP_F_UNALIGNED)) {
@@ -478,6 +480,7 @@ static int queue_userspace_packet(struct datapath *dp,
struct sk_buff *skb,
err = genlmsg_unicast(ovs_dp_get_net(dp), user_skb,
upcall_info-portid);
out:
+ skb_tx_error(skb);
Really sorry to bug you again. We'll get this right ;-)
Patch looks great except for this call to skb_tx_error() which is now
done in the error *and* success path. Make the call conditional on
if (err) and we're good.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
