On 27/02/2019 20:28, John Hurley wrote:
> Offloading rules to a TC datapath only allows the creating of ingress hook
> qdiscs and the application of filters to these. However, there may be
> certain situations where an egress qdisc is more applicable (e.g. when
> offloading to TC rules applied to OvS internal ports).
> 
> Extend the TC API in OvS to allow the creation of egress qdisc and to add
> or interact with flower filters applied to these.
> 
> Signed-off-by: John Hurley <[email protected]>
> Reviewed-by: Simon Horman <[email protected]>
> ---
>  lib/netdev-linux.c       |  8 ++++----
>  lib/netdev-tc-offloads.c | 30 +++++++++++++++---------------
>  lib/tc.c                 | 45 ++++++++++++++++++++++++++++++---------------
>  lib/tc.h                 | 20 ++++++++++++++------
>  4 files changed, 63 insertions(+), 40 deletions(-)
> 
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 25d037c..8d37eb6 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -740,7 +740,7 @@ netdev_linux_update_lag(struct rtnetlink_change *change)
>  
>                  /* LAG master is linux netdev so add slave to same block. */
>                  error = tc_add_del_ingress_qdisc(change->if_index, true,
> -                                                 block_id);
> +                                                 block_id, false);
>                  if (error) {
>                      VLOG_WARN("failed to bind LAG slave to master's block");
>                      shash_delete(&lag_shash, lag->node);
> @@ -756,7 +756,7 @@ netdev_linux_update_lag(struct rtnetlink_change *change)
>  
>          if (lag) {
>              tc_add_del_ingress_qdisc(change->if_index, false,
> -                                     lag->block_id);
> +                                     lag->block_id, false);
>              shash_delete(&lag_shash, lag->node);
>              free(lag);
>          }
> @@ -2370,7 +2370,7 @@ netdev_linux_set_policing(struct netdev *netdev_,
>  
>      COVERAGE_INC(netdev_set_policing);
>      /* Remove any existing ingress qdisc. */
> -    error = tc_add_del_ingress_qdisc(ifindex, false, 0);
> +    error = tc_add_del_ingress_qdisc(ifindex, false, 0, false);
>      if (error) {
>          VLOG_WARN_RL(&rl, "%s: removing policing failed: %s",
>                       netdev_name, ovs_strerror(error));
> @@ -2378,7 +2378,7 @@ netdev_linux_set_policing(struct netdev *netdev_,
>      }
>  
>      if (kbits_rate) {
> -        error = tc_add_del_ingress_qdisc(ifindex, true, 0);
> +        error = tc_add_del_ingress_qdisc(ifindex, true, 0, false);
>          if (error) {
>              VLOG_WARN_RL(&rl, "%s: adding policing qdisc failed: %s",
>                           netdev_name, ovs_strerror(error));

you are missing a call in tc_del_matchall_policer 

lib/netdev-linux.c: In function ‘tc_del_matchall_policer’:
lib/netdev-linux.c:2426:11: error: too few arguments to function ‘tc_del_filter’
     err = tc_del_filter(ifindex, TC_RESERVED_PRIORITY_POLICE, 1, block_id);
           ^~~~~~~~~~~~~



> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index 36bce15..31ad9f4 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -182,7 +182,7 @@ del_filter_and_ufid_mapping(int ifindex, int prio, int 
> handle,
>  {
>      int err;
>  
> -    err = tc_del_filter(ifindex, prio, handle, block_id);
> +    err = tc_del_filter(ifindex, prio, handle, block_id, false);
>      del_ufid_tc_mapping(ufid);
>  
>      return err;
> @@ -350,7 +350,7 @@ netdev_tc_flow_flush(struct netdev *netdev)
>  
>      block_id = get_block_id_from_netdev(netdev);
>  
> -    return tc_flush(ifindex, block_id);
> +    return tc_flush(ifindex, block_id, false);
>  }
>  
>  int
> @@ -372,7 +372,7 @@ netdev_tc_flow_dump_create(struct netdev *netdev,
>      dump = xzalloc(sizeof *dump);
>      dump->nl_dump = xzalloc(sizeof *dump->nl_dump);
>      dump->netdev = netdev_ref(netdev);
> -    tc_dump_flower_start(ifindex, dump->nl_dump, block_id);
> +    tc_dump_flower_start(ifindex, dump->nl_dump, block_id, false);
>  
>      *dump_out = dump;
>  
> @@ -1347,7 +1347,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
> *match,
>      flower.act_cookie.data = ufid;
>      flower.act_cookie.len = sizeof *ufid;
>  
> -    err = tc_replace_flower(ifindex, prio, handle, &flower, block_id);
> +    err = tc_replace_flower(ifindex, prio, handle, &flower, block_id, false);
>      if (!err) {
>          add_ufid_tc_mapping(ufid, flower.prio, flower.handle, netdev, 
> ifindex);
>      }
> @@ -1390,7 +1390,7 @@ netdev_tc_flow_get(struct netdev *netdev OVS_UNUSED,
>      VLOG_DBG_RL(&rl, "flow get (dev %s prio %d handle %d)",
>                  netdev_get_name(dev), prio, handle);
>      block_id = get_block_id_from_netdev(netdev);
> -    err = tc_get_flower(ifindex, prio, handle, &flower, block_id);
> +    err = tc_get_flower(ifindex, prio, handle, &flower, block_id, false);
>      netdev_close(dev);
>      if (err) {
>          VLOG_ERR_RL(&error_rl, "flow get failed (dev %s prio %d handle %d): 
> %s",
> @@ -1437,7 +1437,7 @@ netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
>  
>      if (stats) {
>          memset(stats, 0, sizeof *stats);
> -        if (!tc_get_flower(ifindex, prio, handle, &flower, block_id)) {
> +        if (!tc_get_flower(ifindex, prio, handle, &flower, block_id, false)) 
> {
>              stats->n_packets = get_32aligned_u64(&flower.stats.n_packets);
>              stats->n_bytes = get_32aligned_u64(&flower.stats.n_bytes);
>              stats->used = flower.lastused;
> @@ -1458,7 +1458,7 @@ probe_multi_mask_per_prio(int ifindex)
>      int block_id = 0;
>      int error;
>  
> -    error = tc_add_del_ingress_qdisc(ifindex, true, block_id);
> +    error = tc_add_del_ingress_qdisc(ifindex, true, block_id, false);
>      if (error) {
>          return;
>      }
> @@ -1470,7 +1470,7 @@ probe_multi_mask_per_prio(int ifindex)
>      memset(&flower.key.dst_mac, 0x11, sizeof flower.key.dst_mac);
>      memset(&flower.mask.dst_mac, 0xff, sizeof flower.mask.dst_mac);
>  
> -    error = tc_replace_flower(ifindex, 1, 1, &flower, block_id);
> +    error = tc_replace_flower(ifindex, 1, 1, &flower, block_id, false);
>      if (error) {
>          goto out;
>      }
> @@ -1478,20 +1478,20 @@ probe_multi_mask_per_prio(int ifindex)
>      memset(&flower.key.src_mac, 0x11, sizeof flower.key.src_mac);
>      memset(&flower.mask.src_mac, 0xff, sizeof flower.mask.src_mac);
>  
> -    error = tc_replace_flower(ifindex, 1, 2, &flower, block_id);
> -    tc_del_filter(ifindex, 1, 1, block_id);
> +    error = tc_replace_flower(ifindex, 1, 2, &flower, block_id, false);
> +    tc_del_filter(ifindex, 1, 1, block_id, false);
>  
>      if (error) {
>          goto out;
>      }
>  
> -    tc_del_filter(ifindex, 1, 2, block_id);
> +    tc_del_filter(ifindex, 1, 2, block_id, false);
>  
>      multi_mask_per_prio = true;
>      VLOG_INFO("probe tc: multiple masks on single tc prio is supported.");
>  
>  out:
> -    tc_add_del_ingress_qdisc(ifindex, false, block_id);
> +    tc_add_del_ingress_qdisc(ifindex, false, block_id, false);
>  }
>  
>  static void
> @@ -1500,12 +1500,12 @@ probe_tc_block_support(int ifindex)
>      uint32_t block_id = 1;
>      int error;
>  
> -    error = tc_add_del_ingress_qdisc(ifindex, true, block_id);
> +    error = tc_add_del_ingress_qdisc(ifindex, true, block_id, false);
>      if (error) {
>          return;
>      }
>  
> -    tc_add_del_ingress_qdisc(ifindex, false, block_id);
> +    tc_add_del_ingress_qdisc(ifindex, false, block_id, false);
>  
>      block_support = true;
>      VLOG_INFO("probe tc: block offload is supported.");
> @@ -1538,7 +1538,7 @@ netdev_tc_init_flow_api(struct netdev *netdev)
>      }
>  
>      block_id = get_block_id_from_netdev(netdev);
> -    error = tc_add_del_ingress_qdisc(ifindex, true, block_id);
> +    error = tc_add_del_ingress_qdisc(ifindex, true, block_id, false);
>  
>      if (error && error != EEXIST) {
>          VLOG_ERR("failed adding ingress qdisc required for offloading: %s",
> diff --git a/lib/tc.c b/lib/tc.c
> index 2d42003..441115a 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -219,7 +219,7 @@ tc_transact(struct ofpbuf *request, struct ofpbuf 
> **replyp)
>   * Returns 0 if successful, otherwise a positive errno value.
>   */
>  int
> -tc_add_del_ingress_qdisc(int ifindex, bool add, uint32_t block_id)
> +tc_add_del_ingress_qdisc(int ifindex, bool add, uint32_t block_id, bool 
> egress)
>  {
>      struct ofpbuf request;
>      struct tcmsg *tcmsg;
> @@ -228,11 +228,19 @@ tc_add_del_ingress_qdisc(int ifindex, bool add, 
> uint32_t block_id)
>      int flags = add ? NLM_F_EXCL | NLM_F_CREATE : 0;
>  
>      tcmsg = tc_make_request(ifindex, type, flags, &request);
> -    tcmsg->tcm_handle = TC_H_MAKE(TC_H_INGRESS, 0);
> -    tcmsg->tcm_parent = TC_H_INGRESS;
> -    nl_msg_put_string(&request, TCA_KIND, "ingress");
> +
> +    if (egress) {
> +        tcmsg->tcm_handle = TC_H_MAKE(TC_H_CLSACT, 0);
> +        tcmsg->tcm_parent = TC_H_CLSACT;
> +        nl_msg_put_string(&request, TCA_KIND, "clsact");
> +    } else {
> +        tcmsg->tcm_handle = TC_H_MAKE(TC_H_INGRESS, 0);
> +        tcmsg->tcm_parent = TC_H_INGRESS;
> +        nl_msg_put_string(&request, TCA_KIND, "ingress");
> +    }
> +
>      nl_msg_put_unspec(&request, TCA_OPTIONS, NULL, 0);
> -    if (block_id) {
> +    if (!egress && block_id) {
>          nl_msg_put_u32(&request, TCA_INGRESS_BLOCK, block_id);
>      }
>  
> @@ -1441,7 +1449,8 @@ parse_netlink_to_tc_flower(struct ofpbuf *reply, struct 
> tc_flower *flower)
>  }
>  
>  int
> -tc_dump_flower_start(int ifindex, struct nl_dump *dump, uint32_t block_id)
> +tc_dump_flower_start(int ifindex, struct nl_dump *dump, uint32_t block_id,
> +                     bool egress)
>  {
>      struct ofpbuf request;
>      struct tcmsg *tcmsg;
> @@ -1449,7 +1458,8 @@ tc_dump_flower_start(int ifindex, struct nl_dump *dump, 
> uint32_t block_id)
>  
>      index = block_id ? TCM_IFINDEX_MAGIC_BLOCK : ifindex;
>      tcmsg = tc_make_request(index, RTM_GETTFILTER, NLM_F_DUMP, &request);
> -    tcmsg->tcm_parent = block_id ? : TC_INGRESS_PARENT;
> +    tcmsg->tcm_parent =
> +        egress ? TC_EGRESS_PARENT : (block_id ? : TC_INGRESS_PARENT);
>      tcmsg->tcm_info = TC_H_UNSPEC;
>      tcmsg->tcm_handle = 0;
>  
> @@ -1460,7 +1470,7 @@ tc_dump_flower_start(int ifindex, struct nl_dump *dump, 
> uint32_t block_id)
>  }
>  
>  int
> -tc_flush(int ifindex, uint32_t block_id)
> +tc_flush(int ifindex, uint32_t block_id, bool egress)
>  {
>      struct ofpbuf request;
>      struct tcmsg *tcmsg;
> @@ -1468,14 +1478,16 @@ tc_flush(int ifindex, uint32_t block_id)
>  
>      index = block_id ? TCM_IFINDEX_MAGIC_BLOCK : ifindex;
>      tcmsg = tc_make_request(index, RTM_DELTFILTER, NLM_F_ACK, &request);
> -    tcmsg->tcm_parent = block_id ? : TC_INGRESS_PARENT;
> +    tcmsg->tcm_parent =
> +        egress ? TC_EGRESS_PARENT : (block_id ? : TC_INGRESS_PARENT);
>      tcmsg->tcm_info = TC_H_UNSPEC;
>  
>      return tc_transact(&request, NULL);
>  }
>  
>  int
> -tc_del_filter(int ifindex, int prio, int handle, uint32_t block_id)
> +tc_del_filter(int ifindex, int prio, int handle, uint32_t block_id,
> +              bool egress)
>  {
>      struct ofpbuf request;
>      struct tcmsg *tcmsg;
> @@ -1485,7 +1497,8 @@ tc_del_filter(int ifindex, int prio, int handle, 
> uint32_t block_id)
>  
>      index = block_id ? TCM_IFINDEX_MAGIC_BLOCK : ifindex;
>      tcmsg = tc_make_request(index, RTM_DELTFILTER, NLM_F_ECHO, &request);
> -    tcmsg->tcm_parent = block_id ? : TC_INGRESS_PARENT;
> +    tcmsg->tcm_parent =
> +        egress ? TC_EGRESS_PARENT : (block_id ? : TC_INGRESS_PARENT);
>      tcmsg->tcm_info = tc_make_handle(prio, 0);
>      tcmsg->tcm_handle = handle;
>  
> @@ -1498,7 +1511,7 @@ tc_del_filter(int ifindex, int prio, int handle, 
> uint32_t block_id)
>  
>  int
>  tc_get_flower(int ifindex, int prio, int handle, struct tc_flower *flower,
> -              uint32_t block_id)
> +              uint32_t block_id, bool egress)
>  {
>      struct ofpbuf request;
>      struct tcmsg *tcmsg;
> @@ -1508,7 +1521,8 @@ tc_get_flower(int ifindex, int prio, int handle, struct 
> tc_flower *flower,
>  
>      index = block_id ? TCM_IFINDEX_MAGIC_BLOCK : ifindex;
>      tcmsg = tc_make_request(index, RTM_GETTFILTER, NLM_F_ECHO, &request);
> -    tcmsg->tcm_parent = block_id ? : TC_INGRESS_PARENT;
> +    tcmsg->tcm_parent =
> +        egress ? TC_EGRESS_PARENT : (block_id ? : TC_INGRESS_PARENT);
>      tcmsg->tcm_info = tc_make_handle(prio, 0);
>      tcmsg->tcm_handle = handle;
>  
> @@ -2228,7 +2242,7 @@ nl_msg_put_flower_options(struct ofpbuf *request, 
> struct tc_flower *flower)
>  
>  int
>  tc_replace_flower(int ifindex, uint16_t prio, uint32_t handle,
> -                  struct tc_flower *flower, uint32_t block_id)
> +                  struct tc_flower *flower, uint32_t block_id, bool egress)
>  {
>      struct ofpbuf request;
>      struct tcmsg *tcmsg;
> @@ -2241,7 +2255,8 @@ tc_replace_flower(int ifindex, uint16_t prio, uint32_t 
> handle,
>      index = block_id ? TCM_IFINDEX_MAGIC_BLOCK : ifindex;
>      tcmsg = tc_make_request(index, RTM_NEWTFILTER, NLM_F_CREATE | NLM_F_ECHO,
>                              &request);
> -    tcmsg->tcm_parent = block_id ? : TC_INGRESS_PARENT;
> +    tcmsg->tcm_parent =
> +        egress ? TC_EGRESS_PARENT : (block_id ? : TC_INGRESS_PARENT);
>      tcmsg->tcm_info = tc_make_handle(prio, eth_type);
>      tcmsg->tcm_handle = handle;
>  
> diff --git a/lib/tc.h b/lib/tc.h
> index 3113206..adc79cf 100644
> --- a/lib/tc.h
> +++ b/lib/tc.h
> @@ -36,8 +36,12 @@
>  #ifndef TC_H_MIN_INGRESS
>  #define TC_H_MIN_INGRESS       0xFFF2U
>  #endif
> +#ifndef TC_H_MIN_EGRESS
> +#define TC_H_MIN_EGRESS       0xFFF3U
> +#endif
>  
>  #define TC_INGRESS_PARENT TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_INGRESS)
> +#define TC_EGRESS_PARENT TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_EGRESS)
>  
>  #define TC_POLICY_DEFAULT "none"
>  
> @@ -65,7 +69,8 @@ tc_get_minor(unsigned int handle)
>  struct tcmsg *tc_make_request(int ifindex, int type,
>                                unsigned int flags, struct ofpbuf *);
>  int tc_transact(struct ofpbuf *request, struct ofpbuf **replyp);
> -int tc_add_del_ingress_qdisc(int ifindex, bool add, uint32_t block_id);
> +int tc_add_del_ingress_qdisc(int ifindex, bool add, uint32_t block_id,
> +                             bool egress);
>  
>  struct tc_cookie {
>      const void *data;
> @@ -215,12 +220,15 @@ BUILD_ASSERT_DECL(offsetof(struct tc_flower, rewrite)
>                    + sizeof(uint32_t) - 2 < sizeof(struct tc_flower));
>  
>  int tc_replace_flower(int ifindex, uint16_t prio, uint32_t handle,
> -                      struct tc_flower *flower, uint32_t block_id);
> -int tc_del_filter(int ifindex, int prio, int handle, uint32_t block_id);
> +                      struct tc_flower *flower, uint32_t block_id,
> +                      bool egress);
> +int tc_del_filter(int ifindex, int prio, int handle, uint32_t block_id,
> +                  bool egress);
>  int tc_get_flower(int ifindex, int prio, int handle,
> -                  struct tc_flower *flower, uint32_t block_id);
> -int tc_flush(int ifindex, uint32_t block_id);
> -int tc_dump_flower_start(int ifindex, struct nl_dump *dump, uint32_t 
> block_id);
> +                  struct tc_flower *flower, uint32_t block_id, bool egress);
> +int tc_flush(int ifindex, uint32_t block_id, bool egress);
> +int tc_dump_flower_start(int ifindex, struct nl_dump *dump, uint32_t 
> block_id,
> +                         bool egress);
>  int parse_netlink_to_tc_flower(struct ofpbuf *reply,
>                                 struct tc_flower *flower);
>  void tc_set_policy(const char *policy);
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to