[ovs-dev] [PATCH 3/3] tunnel: set skb mark for IPsec tunnel packets

Pravin Shelar pshelar at nicira.com
Thu Feb 14 21:13:29 UTC 2013


On Thu, Feb 14, 2013 at 11:50 AM, Ansis Atteka <aatteka at nicira.com> wrote:
> The new ovs-monitor-ipsec implementation will use skb marks in
> IPsec policies. This patch will configure datapath to use these
> skb marks for IPsec tunnel packets.
>
> Issue: 14870
> Signed-off-by: Ansis Atteka <aatteka at nicira.com>
> ---
>  lib/odp-util.c         |   12 +++++++++---
>  lib/odp-util.h         |    4 ++--
>  ofproto/ofproto-dpif.c |   11 +++++++----
>  ofproto/tunnel.c       |    9 +++++++--
>  ofproto/tunnel.h       |    5 ++++-
>  5 files changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 7e48981..755bdd6 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -2043,11 +2043,17 @@ odp_put_userspace_action(uint32_t pid, const union user_action_cookie *cookie,
>
>  void
>  odp_put_tunnel_action(const struct flow_tnl *tunnel,
> -                      struct ofpbuf *odp_actions)
> +                      struct ofpbuf *odp_actions, uint32_t skb_mark)
>  {
>      size_t offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_SET);
>      tun_key_to_attr(odp_actions, tunnel);
>      nl_msg_end_nested(odp_actions, offset);
> +
> +    if (skb_mark) {
> +        offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_SET);
> +        nl_msg_put_u32(odp_actions, OVS_KEY_ATTR_SKB_MARK, skb_mark);
> +        nl_msg_end_nested(odp_actions, offset);
> +    }
>  }
>
why not use one available in flow->skb_mark? So that existing set
skb-mark action will take care of generating action for you.
Am I missing something?

>  /* The commit_odp_actions() function and its helpers. */
> @@ -2069,7 +2075,7 @@ commit_set_action(struct ofpbuf *odp_actions, enum ovs_key_attr key_type,
>   * only on tunneling information. */
>  void
>  commit_odp_tunnel_action(const struct flow *flow, struct flow *base,
> -                         struct ofpbuf *odp_actions)
> +                         struct ofpbuf *odp_actions, uint32_t skb_mark)
>  {
>      if (!memcmp(&base->tunnel, &flow->tunnel, sizeof base->tunnel)) {
>          return;
> @@ -2078,7 +2084,7 @@ commit_odp_tunnel_action(const struct flow *flow, struct flow *base,
>
>      /* A valid IPV4_TUNNEL must have non-zero ip_dst. */
>      if (flow->tunnel.ip_dst) {
> -        odp_put_tunnel_action(&base->tunnel, odp_actions);
> +        odp_put_tunnel_action(&base->tunnel, odp_actions, skb_mark);
>      } else {
>          commit_set_action(odp_actions, OVS_KEY_ATTR_TUN_ID,
>                            &base->tunnel.tun_id, sizeof base->tunnel.tun_id);
> diff --git a/lib/odp-util.h b/lib/odp-util.h
> index ccf6c2a..decc497 100644
> --- a/lib/odp-util.h
> +++ b/lib/odp-util.h
> @@ -115,7 +115,7 @@ enum odp_key_fitness odp_flow_key_to_flow(const struct nlattr *, size_t,
>  const char *odp_key_fitness_to_string(enum odp_key_fitness);
>
>  void commit_odp_tunnel_action(const struct flow *, struct flow *base,
> -                              struct ofpbuf *odp_actions);
> +                              struct ofpbuf *odp_actions, uint32_t skb_mark);
>  void commit_odp_actions(const struct flow *, struct flow *base,
>                          struct ofpbuf *odp_actions);
>
> @@ -155,7 +155,7 @@ size_t odp_put_userspace_action(uint32_t pid,
>                                  const union user_action_cookie *,
>                                  struct ofpbuf *odp_actions);
>  void odp_put_tunnel_action(const struct flow_tnl *tunnel,
> -                           struct ofpbuf *odp_actions);
> +                           struct ofpbuf *odp_actions, uint32_t skb_mark);
>
>  /* Reasons why a subfacet might not be fast-pathable. */
>  enum slow_path_reason {
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index b1ec3fb..3891315 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -5512,15 +5512,16 @@ send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
>
>      if (ofport->tnl_port) {
>          struct dpif_flow_stats stats;
> +        uint32_t skb_mark;
>
> -        odp_port = tnl_port_send(ofport->tnl_port, &flow);
> +        odp_port = tnl_port_send(ofport->tnl_port, &flow, &skb_mark);
>          if (odp_port == OVSP_NONE) {
>              return ENODEV;
>          }
>
>          dpif_flow_stats_extract(&flow, packet, time_msec(), &stats);
>          netdev_vport_inc_tx(ofport->up.netdev, &stats);
> -        odp_put_tunnel_action(&flow.tunnel, &odp_actions);
> +        odp_put_tunnel_action(&flow.tunnel, &odp_actions, skb_mark);
>      } else {
>          odp_port = vsp_realdev_to_vlandev(ofproto, ofport->odp_port,
>                                            flow.vlan_tci);
> @@ -5770,7 +5771,9 @@ compose_output_action__(struct action_xlate_ctx *ctx, uint16_t ofp_port,
>
>      odp_port = ofp_port_to_odp_port(ctx->ofproto, ofp_port);
>      if (ofport->tnl_port) {
> -        odp_port = tnl_port_send(ofport->tnl_port, &ctx->flow);
> +        uint32_t skb_mark;
> +
> +        odp_port = tnl_port_send(ofport->tnl_port, &ctx->flow, &skb_mark);
>          if (odp_port == OVSP_NONE) {
>              xlate_report(ctx, "Tunneling decided against output");
>              return;
> @@ -5781,7 +5784,7 @@ compose_output_action__(struct action_xlate_ctx *ctx, uint16_t ofp_port,
>          }
>          out_port = odp_port;
>          commit_odp_tunnel_action(&ctx->flow, &ctx->base_flow,
> -                                 ctx->odp_actions);
> +                                 ctx->odp_actions, skb_mark);
>      } else {
>          out_port = vsp_realdev_to_vlandev(ctx->ofproto, odp_port,
>                                            ctx->flow.vlan_tci);
> diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> index 5a4607e..ee73525 100644
> --- a/ofproto/tunnel.c
> +++ b/ofproto/tunnel.c
> @@ -34,7 +34,6 @@
>   *
>   * Ability to generate actions on input for ECN
>   * Ability to generate metadata for packet-outs
> - * IPsec using skb mark.
>   * VXLAN.
>   * Multicast group management (possibly).
>   * Disallow netdevs with names like "gre64_system" to prevent collisions. */
> @@ -47,6 +46,7 @@ struct tnl_match {
>      ovs_be32 ip_dst;
>      uint32_t odp_port;
>      bool in_key_flow;
> +    uint32_t skb_mark;
>  };
>
>  struct tnl_port {
> @@ -94,6 +94,7 @@ tnl_port_add__(const struct ofport *ofport, uint32_t odp_port,
>      tnl_port->match.in_key = cfg->in_key;
>      tnl_port->match.ip_src = cfg->ip_src;
>      tnl_port->match.ip_dst = cfg->ip_dst;
> +    tnl_port->match.skb_mark = cfg->ipsec ? IPSEC_MARK : 0;
>      tnl_port->match.in_key_flow = cfg->in_key_flow;
>      tnl_port->match.odp_port = odp_port;
>
> @@ -183,6 +184,7 @@ tnl_port_receive(struct flow *flow)
>      match.ip_src = flow->tunnel.ip_dst;
>      match.ip_dst = flow->tunnel.ip_src;
>      match.in_key = flow->tunnel.tun_id;
> +    match.skb_mark = flow->skb_mark;
>
>      tnl_port = tnl_find(&match);
>      if (!tnl_port) {
> @@ -230,7 +232,8 @@ tnl_port_receive(struct flow *flow)
>   * port that the output should happen on.  May return OVSP_NONE if the output
>   * shouldn't occur. */
>  uint32_t
> -tnl_port_send(const struct tnl_port *tnl_port, struct flow *flow)
> +tnl_port_send(const struct tnl_port *tnl_port, struct flow *flow,
> +              uint32_t *skb_mark)
>  {
>      const struct netdev_tunnel_config *cfg;
>      char *pre_flow_str = NULL;
> @@ -248,6 +251,7 @@ tnl_port_send(const struct tnl_port *tnl_port, struct flow *flow)
>
>      flow->tunnel.ip_src = tnl_port->match.ip_src;
>      flow->tunnel.ip_dst = tnl_port->match.ip_dst;
> +    *skb_mark = tnl_port->match.skb_mark;
>
>      if (!cfg->out_key_flow) {
>          flow->tunnel.tun_id = cfg->out_key;
> @@ -393,6 +397,7 @@ tnl_match_fmt(const struct tnl_match *match, struct ds *ds)
>      }
>
>      ds_put_format(ds, ", dp port=%"PRIu32, match->odp_port);
> +    ds_put_format(ds, ", skb mark=%"PRIu32, match->skb_mark);

I would prefer to print ipsec rather than skb_mark, since skb_mark can
have only have two values zero or one.
>  }
>
>  static void
> diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h
> index acb69a8..38c7bec 100644
> --- a/ofproto/tunnel.h
> +++ b/ofproto/tunnel.h
> @@ -25,6 +25,9 @@
>   * These functions emulate tunnel virtual ports based on the outer
>   * header information from the kernel. */
>
> +/* skb mark used for IPsec tunnel packets */
> +#define IPSEC_MARK 1
> +
>  struct ofport;
>  struct tnl_port;
>
> @@ -35,7 +38,7 @@ struct tnl_port *tnl_port_add(const struct ofport *, uint32_t odp_port);
>  void tnl_port_del(struct tnl_port *);
>
>  const struct ofport *tnl_port_receive(struct flow *);
> -uint32_t tnl_port_send(const struct tnl_port *, struct flow *);
> +uint32_t tnl_port_send(const struct tnl_port *, struct flow *, uint32_t *);
>
>  /* Returns true if 'flow' should be submitted to tnl_port_receive(). */
>  static inline bool
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list