diff mbox series

[ovs-dev,v3,3/3] ovn-trace: Handle IPv6 packets for tcp_reset action.

Message ID 20201019072712.280142-1-numans@ovn.org
State Accepted
Headers show
Series Optimize logical flow generation for reject ACLs. | expand

Commit Message

Numan Siddique Oct. 19, 2020, 7:27 a.m. UTC 
From: Numan Siddique <numans@ovn.org>

tcp_reset action can be used for both IPv4 and IPv6 TCP packets, but ovn-trace
was not handling IPv6.

Reported-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
---
 utilities/ovn-trace.c | 57 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 5 deletions(-)

Comments

Dumitru Ceara Oct. 19, 2020, 11:07 a.m. UTC | #1 
On 10/19/20 9:27 AM, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> tcp_reset action can be used for both IPv4 and IPv6 TCP packets, but ovn-trace
> was not handling IPv6.
> 
> Reported-by: Dumitru Ceara <dceara@redhat.com>
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>  utilities/ovn-trace.c | 57 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 52 insertions(+), 5 deletions(-)
> 
> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
> index 89b93774d7..70d47fd1aa 100644
> --- a/utilities/ovn-trace.c
> +++ b/utilities/ovn-trace.c
> @@ -1700,11 +1700,11 @@ execute_icmp6(const struct ovnact_nest *on,
>  }
>  
>  static void
> -execute_tcp_reset(const struct ovnact_nest *on,
> -                  const struct ovntrace_datapath *dp,
> -                  const struct flow *uflow, uint8_t table_id,
> -                  bool lookback, enum ovnact_pipeline pipeline,
> -                  struct ovs_list *super)
> +execute_tcp4_reset(const struct ovnact_nest *on,
> +                   const struct ovntrace_datapath *dp,
> +                   const struct flow *uflow, uint8_t table_id,
> +                   bool lookback, enum ovnact_pipeline pipeline,
> +                   struct ovs_list *super)
>  {
>      struct flow tcp_flow = *uflow;
>  
> @@ -1733,6 +1733,53 @@ execute_tcp_reset(const struct ovnact_nest *on,
>                    table_id, pipeline, &node->subs);
>  }
>  
> +static void
> +execute_tcp6_reset(const struct ovnact_nest *on,
> +                   const struct ovntrace_datapath *dp,
> +                   const struct flow *uflow, uint8_t table_id,
> +                   bool lookback, enum ovnact_pipeline pipeline,

Typo: s/lookback/loopback

> +                   struct ovs_list *super)
> +{
> +    struct flow tcp_flow = *uflow;
> +
> +    /* Update fields for TCP segment. */
> +    if (lookback) {
> +        tcp_flow.dl_dst = uflow->dl_src;
> +        tcp_flow.dl_src = uflow->dl_dst;
> +        tcp_flow.ipv6_dst = uflow->ipv6_src;
> +        tcp_flow.ipv6_src = uflow->ipv6_dst;
> +    } else {
> +        tcp_flow.dl_dst = uflow->dl_dst;
> +        tcp_flow.dl_src = uflow->dl_src;
> +        tcp_flow.ipv6_dst = uflow->ipv6_dst;
> +        tcp_flow.ipv6_src = uflow->ipv6_src;
> +    }
> +    tcp_flow.nw_proto = IPPROTO_TCP;
> +    tcp_flow.nw_ttl = 255;
> +    tcp_flow.tp_src = uflow->tp_src;
> +    tcp_flow.tp_dst = uflow->tp_dst;
> +    tcp_flow.tcp_flags = htons(TCP_RST);
> +
> +    struct ovntrace_node *node = ovntrace_node_append(
> +        super, OVNTRACE_NODE_TRANSFORMATION, "tcp_reset");
> +
> +    trace_actions(on->nested, on->nested_len, dp, &tcp_flow,
> +                  table_id, pipeline, &node->subs);
> +}
> +
> +static void
> +execute_tcp_reset(const struct ovnact_nest *on,
> +                  const struct ovntrace_datapath *dp,
> +                  const struct flow *uflow, uint8_t table_id,
> +                  bool lookback, enum ovnact_pipeline pipeline,
> +                  struct ovs_list *super)

Typo: s/lookback/loopback

With the typos fixed:

Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks,
Dumitru
Numan Siddique Oct. 20, 2020, 6:14 a.m. UTC | #2 
On Mon, Oct 19, 2020 at 4:38 PM Dumitru Ceara <dceara@redhat.com> wrote:
>
> On 10/19/20 9:27 AM, numans@ovn.org wrote:
> > From: Numan Siddique <numans@ovn.org>
> >
> > tcp_reset action can be used for both IPv4 and IPv6 TCP packets, but ovn-trace
> > was not handling IPv6.
> >
> > Reported-by: Dumitru Ceara <dceara@redhat.com>
> > Signed-off-by: Numan Siddique <numans@ovn.org>
> > ---
> >  utilities/ovn-trace.c | 57 +++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 52 insertions(+), 5 deletions(-)
> >
> > diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
> > index 89b93774d7..70d47fd1aa 100644
> > --- a/utilities/ovn-trace.c
> > +++ b/utilities/ovn-trace.c
> > @@ -1700,11 +1700,11 @@ execute_icmp6(const struct ovnact_nest *on,
> >  }
> >
> >  static void
> > -execute_tcp_reset(const struct ovnact_nest *on,
> > -                  const struct ovntrace_datapath *dp,
> > -                  const struct flow *uflow, uint8_t table_id,
> > -                  bool lookback, enum ovnact_pipeline pipeline,
> > -                  struct ovs_list *super)
> > +execute_tcp4_reset(const struct ovnact_nest *on,
> > +                   const struct ovntrace_datapath *dp,
> > +                   const struct flow *uflow, uint8_t table_id,
> > +                   bool lookback, enum ovnact_pipeline pipeline,
> > +                   struct ovs_list *super)
> >  {
> >      struct flow tcp_flow = *uflow;
> >
> > @@ -1733,6 +1733,53 @@ execute_tcp_reset(const struct ovnact_nest *on,
> >                    table_id, pipeline, &node->subs);
> >  }
> >
> > +static void
> > +execute_tcp6_reset(const struct ovnact_nest *on,
> > +                   const struct ovntrace_datapath *dp,
> > +                   const struct flow *uflow, uint8_t table_id,
> > +                   bool lookback, enum ovnact_pipeline pipeline,
>
> Typo: s/lookback/loopback
>
> > +                   struct ovs_list *super)
> > +{
> > +    struct flow tcp_flow = *uflow;
> > +
> > +    /* Update fields for TCP segment. */
> > +    if (lookback) {
> > +        tcp_flow.dl_dst = uflow->dl_src;
> > +        tcp_flow.dl_src = uflow->dl_dst;
> > +        tcp_flow.ipv6_dst = uflow->ipv6_src;
> > +        tcp_flow.ipv6_src = uflow->ipv6_dst;
> > +    } else {
> > +        tcp_flow.dl_dst = uflow->dl_dst;
> > +        tcp_flow.dl_src = uflow->dl_src;
> > +        tcp_flow.ipv6_dst = uflow->ipv6_dst;
> > +        tcp_flow.ipv6_src = uflow->ipv6_src;
> > +    }
> > +    tcp_flow.nw_proto = IPPROTO_TCP;
> > +    tcp_flow.nw_ttl = 255;
> > +    tcp_flow.tp_src = uflow->tp_src;
> > +    tcp_flow.tp_dst = uflow->tp_dst;
> > +    tcp_flow.tcp_flags = htons(TCP_RST);
> > +
> > +    struct ovntrace_node *node = ovntrace_node_append(
> > +        super, OVNTRACE_NODE_TRANSFORMATION, "tcp_reset");
> > +
> > +    trace_actions(on->nested, on->nested_len, dp, &tcp_flow,
> > +                  table_id, pipeline, &node->subs);
> > +}
> > +
> > +static void
> > +execute_tcp_reset(const struct ovnact_nest *on,
> > +                  const struct ovntrace_datapath *dp,
> > +                  const struct flow *uflow, uint8_t table_id,
> > +                  bool lookback, enum ovnact_pipeline pipeline,
> > +                  struct ovs_list *super)
>
> Typo: s/lookback/loopback
>
> With the typos fixed:
>
> Acked-by: Dumitru Ceara <dceara@redhat.com>

Hi Dumitru,

I just realized that I missed fixing these typos before  committing
the patch series. My apologies.
I have sent a follow up patch -
https://patchwork.ozlabs.org/project/ovn/patch/20201020061202.1051704-1-numans@ovn.org/
Request to take a look.

Numan

>
> Thanks,
> Dumitru
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
index 89b93774d7..70d47fd1aa 100644
--- a/utilities/ovn-trace.c
+++ b/utilities/ovn-trace.c
@@ -1700,11 +1700,11 @@  execute_icmp6(const struct ovnact_nest *on,
 }
 
 static void
-execute_tcp_reset(const struct ovnact_nest *on,
-                  const struct ovntrace_datapath *dp,
-                  const struct flow *uflow, uint8_t table_id,
-                  bool lookback, enum ovnact_pipeline pipeline,
-                  struct ovs_list *super)
+execute_tcp4_reset(const struct ovnact_nest *on,
+                   const struct ovntrace_datapath *dp,
+                   const struct flow *uflow, uint8_t table_id,
+                   bool lookback, enum ovnact_pipeline pipeline,
+                   struct ovs_list *super)
 {
     struct flow tcp_flow = *uflow;
 
@@ -1733,6 +1733,53 @@  execute_tcp_reset(const struct ovnact_nest *on,
                   table_id, pipeline, &node->subs);
 }
 
+static void
+execute_tcp6_reset(const struct ovnact_nest *on,
+                   const struct ovntrace_datapath *dp,
+                   const struct flow *uflow, uint8_t table_id,
+                   bool lookback, enum ovnact_pipeline pipeline,
+                   struct ovs_list *super)
+{
+    struct flow tcp_flow = *uflow;
+
+    /* Update fields for TCP segment. */
+    if (lookback) {
+        tcp_flow.dl_dst = uflow->dl_src;
+        tcp_flow.dl_src = uflow->dl_dst;
+        tcp_flow.ipv6_dst = uflow->ipv6_src;
+        tcp_flow.ipv6_src = uflow->ipv6_dst;
+    } else {
+        tcp_flow.dl_dst = uflow->dl_dst;
+        tcp_flow.dl_src = uflow->dl_src;
+        tcp_flow.ipv6_dst = uflow->ipv6_dst;
+        tcp_flow.ipv6_src = uflow->ipv6_src;
+    }
+    tcp_flow.nw_proto = IPPROTO_TCP;
+    tcp_flow.nw_ttl = 255;
+    tcp_flow.tp_src = uflow->tp_src;
+    tcp_flow.tp_dst = uflow->tp_dst;
+    tcp_flow.tcp_flags = htons(TCP_RST);
+
+    struct ovntrace_node *node = ovntrace_node_append(
+        super, OVNTRACE_NODE_TRANSFORMATION, "tcp_reset");
+
+    trace_actions(on->nested, on->nested_len, dp, &tcp_flow,
+                  table_id, pipeline, &node->subs);
+}
+
+static void
+execute_tcp_reset(const struct ovnact_nest *on,
+                  const struct ovntrace_datapath *dp,
+                  const struct flow *uflow, uint8_t table_id,
+                  bool lookback, enum ovnact_pipeline pipeline,
+                  struct ovs_list *super)
+{
+    if (get_dl_type(uflow) == htons(ETH_TYPE_IP)) {
+        execute_tcp4_reset(on, dp, uflow, table_id, lookback, pipeline, super);
+    } else {
+        execute_tcp6_reset(on, dp, uflow, table_id, lookback, pipeline, super);
+    }
+}
 static void
 execute_reject(const struct ovnact_nest *on,
                const struct ovntrace_datapath *dp,