diff mbox

[net-next,1/9] openvswitch: Scrub packet in ovs_vport_receive()

Message ID CANr6G5wRS1zg4oLdS5L4gM7iCS11BHb7+8uUN1fh5Oa9F4_BBw@mail.gmail.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Joe Stringer July 30, 2015, 11:16 p.m. UTC 
On 30 July 2015 at 11:40, Thomas Graf <tgraf@suug.ch> wrote:
> On 07/30/15 at 11:12am, Joe Stringer wrote:
>> Signed-off-by: Joe Stringer <joestringer@nicira.com>
>
> Can you write a few lines on why this is needed? I have flows which
> use the mark to communicate with netfilter through internal ports.

The problem I was seeing is when packets come from a different
namespace on the localhost, they still have conntrack data associated.
This doesn't make sense, so the intention is to perform nf_reset().
However, it seems like we should actually be doing a bit more - at
least the skb_dst_drop() and perhaps some of the other stuff in
skb_scrub_packet().

Do you want to retain the mark when transitioning between namespaces?

Perhaps something like the below incremental would be sufficient:

        stats = this_cpu_ptr(vport->percpu_stats);
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Thomas Graf July 31, 2015, 7:38 a.m. UTC | #1 
On 07/30/15 at 04:16pm, Joe Stringer wrote:
> On 30 July 2015 at 11:40, Thomas Graf <tgraf@suug.ch> wrote:
> > On 07/30/15 at 11:12am, Joe Stringer wrote:
> >> Signed-off-by: Joe Stringer <joestringer@nicira.com>
> >
> > Can you write a few lines on why this is needed? I have flows which
> > use the mark to communicate with netfilter through internal ports.
> 
> The problem I was seeing is when packets come from a different
> namespace on the localhost, they still have conntrack data associated.
> This doesn't make sense, so the intention is to perform nf_reset().
> However, it seems like we should actually be doing a bit more - at
> least the skb_dst_drop() and perhaps some of the other stuff in
> skb_scrub_packet().
> 
> Do you want to retain the mark when transitioning between namespaces?

Since we have retained it so far I think we should keep on doing
that. I'm pretty sure there are users of it out there besides me.
As you know, it's common to have tap devices in between OVS and the
guest in OpenStack and install netfilter rules there.

As for whether we should scrub it in between namespaces. Probably yes
but it's definitely tremendously useful to be able to transfer some
metadata (mark and dst metadata) between namespaces. The default
behaviour should probably be to scrub it with a flag to keep it. If
that flag is not set and nsid of port != bridge then we scrub the mark
and other metadata.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index 8a63df6..82844e6 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -475,7 +475,9 @@  void ovs_vport_receive(struct vport *vport, struct
sk_buff *skb,
        struct sw_flow_key key;
        int error;

-       if (!skb->sk || (sock_net(skb->sk) != read_pnet(&vport->dp->net)))
+       if (!skb->sk)
+               skb_scrub_packet(skb, false);
+       else if (sock_net(skb->sk) != read_pnet(&vport->dp->net))
                skb_scrub_packet(skb, true);