diff mbox

[ovs-dev,ovs,V8,02/26] netdev: Adding a new netdev api to be used for offloading flows

Message ID 1493824097-47495-3-git-send-email-roid@mellanox.com
State Changes Requested
Headers show

Commit Message

Roi Dayan May 3, 2017, 3:07 p.m. UTC
From: Paul Blakey <paulb@mellanox.com>

Signed-off-by: Paul Blakey <paulb@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 lib/automake.mk          |   2 +
 lib/netdev-bsd.c         |   2 +
 lib/netdev-dpdk.c        |   1 +
 lib/netdev-dummy.c       |   2 +
 lib/netdev-linux.c       |  15 +++--
 lib/netdev-linux.h       |   9 +++
 lib/netdev-provider.h    |  71 ++++++++++++++++++++++
 lib/netdev-tc-offloads.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/netdev-tc-offloads.h |  42 +++++++++++++
 lib/netdev-vport.c       |  11 +++-
 lib/netdev.c             |  98 ++++++++++++++++++++++++++++++
 lib/netdev.h             |  23 +++++++
 12 files changed, 425 insertions(+), 5 deletions(-)
 create mode 100644 lib/netdev-tc-offloads.c
 create mode 100644 lib/netdev-tc-offloads.h

Comments

Simon Horman May 8, 2017, 12:28 p.m. UTC | #1
On Wed, May 03, 2017 at 06:07:53PM +0300, Roi Dayan wrote:
> From: Paul Blakey <paulb@mellanox.com>

Please add some text to the changelog.

> 
> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>

...

> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> new file mode 100644
> index 0000000..eb5e79a
> --- /dev/null
> +++ b/lib/netdev-tc-offloads.c
> @@ -0,0 +1,154 @@
> +/*
> + * Copyright (c) 2016 Mellanox Technologies, Ltd.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +
> +#include "netdev-tc-offloads.h"
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <arpa/inet.h>
> +#include <inttypes.h>
> +#include <linux/filter.h>
> +#include <linux/gen_stats.h>
> +#include <linux/if_ether.h>
> +#include <linux/if_tun.h>
> +#include <linux/types.h>
> +#include <linux/ethtool.h>
> +#include <linux/mii.h>
> +#include <linux/pkt_cls.h>
> +#include <linux/pkt_sched.h>
> +#include <linux/rtnetlink.h>
> +#include <linux/sockios.h>
> +#include <sys/types.h>
> +#include <sys/ioctl.h>
> +#include <sys/socket.h>
> +#include <sys/utsname.h>
> +#include <netpacket/packet.h>
> +#include <net/if.h>
> +#include <net/if_arp.h>
> +#include <net/if_packet.h>
> +#include <net/route.h>
> +#include <netinet/in.h>
> +#include <poll.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include "coverage.h"
> +#include "dp-packet.h"
> +#include "dpif-netlink.h"
> +#include "dpif-netdev.h"
> +#include "openvswitch/dynamic-string.h"
> +#include "fatal-signal.h"
> +#include "hash.h"
> +#include "openvswitch/hmap.h"
> +#include "netdev-provider.h"
> +#include "netdev-vport.h"
> +#include "netlink-notifier.h"
> +#include "netlink-socket.h"
> +#include "netlink.h"
> +#include "openvswitch/ofpbuf.h"
> +#include "openflow/openflow.h"
> +#include "ovs-atomic.h"
> +#include "packets.h"
> +#include "poll-loop.h"
> +#include "rtnetlink.h"
> +#include "openvswitch/shash.h"
> +#include "netdev-provider.h"
> +#include "openvswitch/match.h"
> +#include "openvswitch/vlog.h"
> +#include "tc.h"

Are all of the above headers needed for what follows?
There seems to be a lot of #includes above.

> +VLOG_DEFINE_THIS_MODULE(netdev_tc_offloads);
> +
> +int
> +netdev_tc_flow_flush(struct netdev *netdev OVS_UNUSED)
> +{
> +    return EOPNOTSUPP;
> +}
> +
> +int
> +netdev_tc_flow_dump_create(struct netdev *netdev,
> +                           struct netdev_flow_dump **dump_out)
> +{
> +    struct netdev_flow_dump *dump = xzalloc(sizeof *dump);
> +
> +    dump->netdev = netdev_ref(netdev);
> +
> +    *dump_out = dump;
> +
> +    return 0;
> +}
> +
> +int
> +netdev_tc_flow_dump_destroy(struct netdev_flow_dump *dump)
> +{
> +    netdev_close(dump->netdev);
> +    free(dump);
> +
> +    return 0;
> +}
> +
> +bool
> +netdev_tc_flow_dump_next(struct netdev_flow_dump *dump OVS_UNUSED,
> +                         struct match *match OVS_UNUSED,
> +                         struct nlattr **actions OVS_UNUSED,
> +                         struct dpif_flow_stats *stats OVS_UNUSED,
> +                         ovs_u128 *ufid OVS_UNUSED,
> +                         struct ofpbuf *rbuffer OVS_UNUSED,
> +                         struct ofpbuf *wbuffer OVS_UNUSED)
> +{
> +    return false;
> +}
> +
> +int
> +netdev_tc_flow_put(struct netdev *netdev OVS_UNUSED,
> +                   struct match *match OVS_UNUSED,
> +                   struct nlattr *actions OVS_UNUSED,
> +                   size_t actions_len OVS_UNUSED,
> +                   struct dpif_flow_stats *stats OVS_UNUSED,
> +                   const ovs_u128 *ufid OVS_UNUSED,

Here and above ufid follows stats.

> +                   struct offload_info *info OVS_UNUSED)
> +{
> +    return EOPNOTSUPP;
> +}
> +
> +int
> +netdev_tc_flow_get(struct netdev *netdev OVS_UNUSED,
> +                   struct match *match OVS_UNUSED,
> +                   struct nlattr **actions OVS_UNUSED,
> +                   const ovs_u128 *ufid OVS_UNUSED,
> +                   struct dpif_flow_stats *stats OVS_UNUSED,

But here the order is reversed.

I always think that when a reviewer asks for entries to be sorted they
have run out of things to say. But nonetheless it would be slightly
nicer if the order was consistent unless there is a reason for it not to
be.

> +                   struct ofpbuf *buf OVS_UNUSED)
> +{
> +    return EOPNOTSUPP;
> +}
> +
> +int
> +netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
> +                   const ovs_u128 *ufid OVS_UNUSED,
> +                   struct dpif_flow_stats *stats OVS_UNUSED)
> +{
> +    return EOPNOTSUPP;
> +}
> +
> +int
> +netdev_tc_init_flow_api(struct netdev *netdev OVS_UNUSED)
> +{
> +    return 0;
> +}
> +

There is a trailing blank line above.

> diff --git a/lib/netdev-tc-offloads.h b/lib/netdev-tc-offloads.h
> new file mode 100644
> index 0000000..76b7938
> --- /dev/null
> +++ b/lib/netdev-tc-offloads.h
> @@ -0,0 +1,42 @@

> +int netdev_tc_flow_put(struct netdev *, struct match *,
> +                       struct nlattr *actions, size_t actions_len,
> +                       const ovs_u128 *, struct offload_info *,
> +                       struct dpif_flow_stats *);

The declaration of netdev_tc_flow_put() does not match its definition.

...
Flavio Leitner May 9, 2017, 6:12 p.m. UTC | #2
On Wed, May 03, 2017 at 06:07:53PM +0300, Roi Dayan wrote:
> From: Paul Blakey <paulb@mellanox.com>
> 
> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
> ---
>  lib/automake.mk          |   2 +
>  lib/netdev-bsd.c         |   2 +
>  lib/netdev-dpdk.c        |   1 +
>  lib/netdev-dummy.c       |   2 +
>  lib/netdev-linux.c       |  15 +++--
>  lib/netdev-linux.h       |   9 +++
>  lib/netdev-provider.h    |  71 ++++++++++++++++++++++
>  lib/netdev-tc-offloads.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++
>  lib/netdev-tc-offloads.h |  42 +++++++++++++
>  lib/netdev-vport.c       |  11 +++-
>  lib/netdev.c             |  98 ++++++++++++++++++++++++++++++
>  lib/netdev.h             |  23 +++++++
>  12 files changed, 425 insertions(+), 5 deletions(-)
>  create mode 100644 lib/netdev-tc-offloads.c
>  create mode 100644 lib/netdev-tc-offloads.h
> 
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 3d57610..a7c8009 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -354,6 +354,8 @@ lib_libopenvswitch_la_SOURCES += \
>  	lib/dpif-netlink.h \
>  	lib/tc.h \
>  	lib/tc.c \
> +	lib/netdev-tc-offloads.h \
> +	lib/netdev-tc-offloads.c \
>  	lib/if-notifier.c \
>  	lib/if-notifier.h \
>  	lib/netdev-linux.c \
> diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
> index 94c515d..5b54d79 100644
> --- a/lib/netdev-bsd.c
> +++ b/lib/netdev-bsd.c
> @@ -1547,6 +1547,8 @@ netdev_bsd_update_flags(struct netdev *netdev_, enum netdev_flags off,
>      netdev_bsd_rxq_recv,                             \
>      netdev_bsd_rxq_wait,                             \
>      netdev_bsd_rxq_drain,                            \
> +                                                     \
> +    NO_OFFLOAD_API                                   \
>  }
>  
>  const struct netdev_class netdev_bsd_class =
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index ddc651b..9ad96cd 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -3314,6 +3314,7 @@ unlock:
>      RXQ_RECV,                                                 \
>      NULL,                       /* rx_wait */                 \
>      NULL,                       /* rxq_drain */               \
> +    NO_OFFLOAD_API                                            \
>  }
>  
>  static const struct netdev_class dpdk_class =
> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> index 0657434..7e1383f 100644
> --- a/lib/netdev-dummy.c
> +++ b/lib/netdev-dummy.c
> @@ -1409,6 +1409,8 @@ netdev_dummy_update_flags(struct netdev *netdev_,
>      netdev_dummy_rxq_recv,                                      \
>      netdev_dummy_rxq_wait,                                      \
>      netdev_dummy_rxq_drain,                                     \
> +                                                                \
> +    NO_OFFLOAD_API                                              \
>  }
>  
>  static const struct netdev_class dummy_class =
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index a6bb515..f7941fd 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -73,6 +73,7 @@
>  #include "openvswitch/vlog.h"
>  #include "util.h"
>  #include "tc.h"
> +#include "netdev-tc-offloads.h"
>  
>  VLOG_DEFINE_THIS_MODULE(netdev_linux);
>  
> @@ -2783,7 +2784,8 @@ netdev_linux_update_flags(struct netdev *netdev_, enum netdev_flags off,
>  }
>  
>  #define NETDEV_LINUX_CLASS(NAME, CONSTRUCT, GET_STATS,          \
> -                           GET_FEATURES, GET_STATUS)            \
> +                           GET_FEATURES, GET_STATUS,            \
> +                           FLOW_OFFLOAD_API)                    \
>  {                                                               \
>      NAME,                                                       \
>      false,                      /* is_pmd */                    \
> @@ -2852,6 +2854,8 @@ netdev_linux_update_flags(struct netdev *netdev_, enum netdev_flags off,
>      netdev_linux_rxq_recv,                                      \
>      netdev_linux_rxq_wait,                                      \
>      netdev_linux_rxq_drain,                                     \
> +                                                                \
> +    FLOW_OFFLOAD_API                                            \
>  }
>  
>  const struct netdev_class netdev_linux_class =
> @@ -2860,7 +2864,8 @@ const struct netdev_class netdev_linux_class =
>          netdev_linux_construct,
>          netdev_linux_get_stats,
>          netdev_linux_get_features,
> -        netdev_linux_get_status);
> +        netdev_linux_get_status,
> +        LINUX_FLOW_OFFLOAD_API);
>  
>  const struct netdev_class netdev_tap_class =
>      NETDEV_LINUX_CLASS(
> @@ -2868,7 +2873,8 @@ const struct netdev_class netdev_tap_class =
>          netdev_linux_construct_tap,
>          netdev_tap_get_stats,
>          netdev_linux_get_features,
> -        netdev_linux_get_status);
> +        netdev_linux_get_status,
> +        NO_OFFLOAD_API);
>  
>  const struct netdev_class netdev_internal_class =
>      NETDEV_LINUX_CLASS(
> @@ -2876,7 +2882,8 @@ const struct netdev_class netdev_internal_class =
>          netdev_linux_construct,
>          netdev_internal_get_stats,
>          NULL,                  /* get_features */
> -        netdev_internal_get_status);
> +        netdev_internal_get_status,
> +        NO_OFFLOAD_API);
>  
>  
>  #define CODEL_N_QUEUES 0x0000
> diff --git a/lib/netdev-linux.h b/lib/netdev-linux.h
> index 0c61bc9..d944691 100644
> --- a/lib/netdev-linux.h
> +++ b/lib/netdev-linux.h
> @@ -28,4 +28,13 @@ struct netdev;
>  int netdev_linux_ethtool_set_flag(struct netdev *netdev, uint32_t flag,
>                                    const char *flag_name, bool enable);
>  
> +#define LINUX_FLOW_OFFLOAD_API                                  \
> +            netdev_tc_flow_flush,                               \
> +            netdev_tc_flow_dump_create,                         \
> +            netdev_tc_flow_dump_destroy,                        \
> +            netdev_tc_flow_dump_next,                           \
> +            netdev_tc_flow_put,                                 \
> +            netdev_tc_flow_get,                                 \
> +            netdev_tc_flow_del,                                 \
> +            netdev_tc_init_flow_api
>  #endif /* netdev-linux.h */
> diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
> index 8346fc4..e8bfdc8 100644
> --- a/lib/netdev-provider.h
> +++ b/lib/netdev-provider.h
> @@ -115,6 +115,14 @@ struct netdev_rxq {
>  
>  struct netdev *netdev_rxq_get_netdev(const struct netdev_rxq *);
>  
> +
> +struct netdev_flow_dump {
> +    struct netdev *netdev;
> +    odp_port_t port;
> +    struct nl_dump *nl_dump;
> +    bool terse;
> +};
> +
>  /* Network device class structure, to be defined by each implementation of a
>   * network device.
>   *
> @@ -769,6 +777,67 @@ struct netdev_class {
>  
>      /* Discards all packets waiting to be received from 'rx'. */
>      int (*rxq_drain)(struct netdev_rxq *rx);
> +
> +    /* ## -------------------------------- ## */
> +    /* ## netdev flow offloading functions ## */
> +    /* ## -------------------------------- ## */
> +
> +    /* If a particular netdev class does not support offloading flows,
> +     * all these function pointers must be NULL. */
> +
> +    /* Flush all offloaded flows from a netdev.
> +     * Return 0 if successful, otherwise returns a positive errno value. */
> +    int (*flow_flush)(struct netdev *);
> +
> +    /* Flow dumping interface.
> +     *
> +     * This is the back-end for the flow dumping interface described in
> +     * dpif.h.  Please read the comments there first, because this code
> +     * closely follows it.
> +     *
> +     * 'flow_dump_create' is being executed in a dpif thread so there is
> +     * no need for 'flow_dump_thread_create' implementation.
> +     * On success returns allocated netdev_flow_dump data, on failure returns
> +     * positive errno. */
> +    int (*flow_dump_create)(struct netdev *, struct netdev_flow_dump **dump);
> +    int (*flow_dump_destroy)(struct netdev_flow_dump *);
> +
> +    /* Returns true while there are more flows to dump.
> +     * rbuffer is used as a temporary buffer and needs to be pre allocated
> +     * by the caller. while there are more flows the same rbuffer should
> +     * be provided. wbuffer is used to store dumped actions and needs to be
> +     * pre allocated by the caller. */
> +    bool (*flow_dump_next)(struct netdev_flow_dump *, struct match *,
> +                           struct nlattr **actions,
> +                           struct dpif_flow_stats *stats, ovs_u128 *ufid,
> +                           struct ofpbuf *rbuffer, struct ofpbuf *wbuffer);
> +
> +    /* Offload the given flow on netdev.
> +     * To modify a flow, use the same ufid.
> +     * actions are in netlink format, as with struct dpif_flow_put.
> +     * info is extra info needed to offload the flow.
> +     * Read the comments on 'struct dpif_flow_put' in dpif.h about stats.
> +     * Return 0 if successful, otherwise returns a positive errno value. */
> +    int (*flow_put)(struct netdev *, struct match *, struct nlattr *actions,
> +                    size_t actions_len, const ovs_u128 *ufid,
> +                    struct offload_info *info, struct dpif_flow_stats *);
> +
> +    /* Queries a flow specified by ufid on netdev.
> +     * Fills output buffer as wbuffer in flow_dump_next.
> +     * the buffer needs to be pre allocated.
> +     * Return 0 if successful, otherwise returns a positive errno value. */
> +    int (*flow_get)(struct netdev *, struct match *, struct nlattr **actions,
> +                    const ovs_u128 *ufid, struct dpif_flow_stats *,
> +                    struct ofpbuf *wbuffer);
> +
> +    /* Delete a flow specified by ufid from netdev.
> +     * Read the comments on 'struct dpif_flow_del' in dpif.h about stats.
> +     * Return 0 if successful, otherwise returns a positive errno value. */
> +    int (*flow_del)(struct netdev *, const ovs_u128 *ufid,
> +                    struct dpif_flow_stats *);
> +
> +    /* Initializies the netdev flow api. */
> +    int (*init_flow_api)(struct netdev *);
>  };
>  
>  int netdev_register_provider(const struct netdev_class *);
> @@ -788,4 +857,6 @@ extern const struct netdev_class netdev_tap_class;
>  }
>  #endif
>  
> +#define NO_OFFLOAD_API NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL
> +
>  #endif /* netdev.h */
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> new file mode 100644
> index 0000000..eb5e79a
> --- /dev/null
> +++ b/lib/netdev-tc-offloads.c
> @@ -0,0 +1,154 @@
> +/*
> + * Copyright (c) 2016 Mellanox Technologies, Ltd.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +
> +#include "netdev-tc-offloads.h"
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <arpa/inet.h>
> +#include <inttypes.h>
> +#include <linux/filter.h>
> +#include <linux/gen_stats.h>
> +#include <linux/if_ether.h>
> +#include <linux/if_tun.h>
> +#include <linux/types.h>
> +#include <linux/ethtool.h>
> +#include <linux/mii.h>
> +#include <linux/pkt_cls.h>
> +#include <linux/pkt_sched.h>
> +#include <linux/rtnetlink.h>
> +#include <linux/sockios.h>
> +#include <sys/types.h>
> +#include <sys/ioctl.h>
> +#include <sys/socket.h>
> +#include <sys/utsname.h>
> +#include <netpacket/packet.h>
> +#include <net/if.h>
> +#include <net/if_arp.h>
> +#include <net/if_packet.h>
> +#include <net/route.h>
> +#include <netinet/in.h>
> +#include <poll.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include "coverage.h"
> +#include "dp-packet.h"
> +#include "dpif-netlink.h"
> +#include "dpif-netdev.h"
> +#include "openvswitch/dynamic-string.h"
> +#include "fatal-signal.h"
> +#include "hash.h"
> +#include "openvswitch/hmap.h"
> +#include "netdev-provider.h"
> +#include "netdev-vport.h"
> +#include "netlink-notifier.h"
> +#include "netlink-socket.h"
> +#include "netlink.h"
> +#include "openvswitch/ofpbuf.h"
> +#include "openflow/openflow.h"
> +#include "ovs-atomic.h"
> +#include "packets.h"
> +#include "poll-loop.h"
> +#include "rtnetlink.h"
> +#include "openvswitch/shash.h"
> +#include "netdev-provider.h"
> +#include "openvswitch/match.h"
> +#include "openvswitch/vlog.h"
> +#include "tc.h"
> +
> +VLOG_DEFINE_THIS_MODULE(netdev_tc_offloads);
> +
> +int
> +netdev_tc_flow_flush(struct netdev *netdev OVS_UNUSED)
> +{
> +    return EOPNOTSUPP;
> +}
> +
> +int
> +netdev_tc_flow_dump_create(struct netdev *netdev,
> +                           struct netdev_flow_dump **dump_out)
> +{
> +    struct netdev_flow_dump *dump = xzalloc(sizeof *dump);
> +
> +    dump->netdev = netdev_ref(netdev);
> +
> +    *dump_out = dump;
> +
> +    return 0;
> +}
> +
> +int
> +netdev_tc_flow_dump_destroy(struct netdev_flow_dump *dump)
> +{
> +    netdev_close(dump->netdev);
> +    free(dump);
> +
> +    return 0;
> +}
> +
> +bool
> +netdev_tc_flow_dump_next(struct netdev_flow_dump *dump OVS_UNUSED,
> +                         struct match *match OVS_UNUSED,
> +                         struct nlattr **actions OVS_UNUSED,
> +                         struct dpif_flow_stats *stats OVS_UNUSED,
> +                         ovs_u128 *ufid OVS_UNUSED,
> +                         struct ofpbuf *rbuffer OVS_UNUSED,
> +                         struct ofpbuf *wbuffer OVS_UNUSED)
> +{
> +    return false;
> +}
> +
> +int
> +netdev_tc_flow_put(struct netdev *netdev OVS_UNUSED,
> +                   struct match *match OVS_UNUSED,
> +                   struct nlattr *actions OVS_UNUSED,
> +                   size_t actions_len OVS_UNUSED,
> +                   struct dpif_flow_stats *stats OVS_UNUSED,
> +                   const ovs_u128 *ufid OVS_UNUSED,
> +                   struct offload_info *info OVS_UNUSED)
> +{
> +    return EOPNOTSUPP;
> +}
> +
> +int
> +netdev_tc_flow_get(struct netdev *netdev OVS_UNUSED,
> +                   struct match *match OVS_UNUSED,
> +                   struct nlattr **actions OVS_UNUSED,
> +                   const ovs_u128 *ufid OVS_UNUSED,
> +                   struct dpif_flow_stats *stats OVS_UNUSED,
> +                   struct ofpbuf *buf OVS_UNUSED)
> +{
> +    return EOPNOTSUPP;
> +}
> +
> +int
> +netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
> +                   const ovs_u128 *ufid OVS_UNUSED,
> +                   struct dpif_flow_stats *stats OVS_UNUSED)
> +{
> +    return EOPNOTSUPP;
> +}
> +
> +int
> +netdev_tc_init_flow_api(struct netdev *netdev OVS_UNUSED)
> +{
> +    return 0;
> +}
> +
> diff --git a/lib/netdev-tc-offloads.h b/lib/netdev-tc-offloads.h
> new file mode 100644
> index 0000000..76b7938
> --- /dev/null
> +++ b/lib/netdev-tc-offloads.h
> @@ -0,0 +1,42 @@
> +/*
> + * Copyright (c) 2016 Mellanox Technologies, Ltd.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef NETDEV_TC_OFFLOADS_H
> +#define NETDEV_TC_OFFLOADS_H 1
> +
> +#include "netdev.h"
> +
> +int netdev_tc_flow_flush(struct netdev *);
> +int netdev_tc_flow_dump_create(struct netdev *, struct netdev_flow_dump **);
> +int netdev_tc_flow_dump_destroy(struct netdev_flow_dump *);
> +bool netdev_tc_flow_dump_next(struct netdev_flow_dump *, struct match *,
> +                              struct nlattr **actions,
> +                              struct dpif_flow_stats *,
> +                              ovs_u128 *ufid,
> +                              struct ofpbuf *rbuffer,
> +                              struct ofpbuf *wbuffer);
> +int netdev_tc_flow_put(struct netdev *, struct match *,
> +                       struct nlattr *actions, size_t actions_len,
> +                       const ovs_u128 *, struct offload_info *,
> +                       struct dpif_flow_stats *);
> +int netdev_tc_flow_get(struct netdev *, struct match *,
> +                       struct nlattr **actions, const ovs_u128 *,
> +                       struct dpif_flow_stats *, struct ofpbuf *);
> +int netdev_tc_flow_del(struct netdev *, const ovs_u128 *,
> +                        struct dpif_flow_stats *);
> +int netdev_tc_init_flow_api(struct netdev *);
> +
> +#endif /* netdev-tc-offloads.h */
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index 39093e8..2cad5cb 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -847,7 +847,16 @@ get_stats(const struct netdev *netdev, struct netdev_stats *stats)
>      NULL,                   /* rx_dealloc */                \
>      NULL,                   /* rx_recv */                   \
>      NULL,                   /* rx_wait */                   \
> -    NULL,                   /* rx_drain */
> +    NULL,                   /* rx_drain */                  \
> +                                                            \
> +    NULL,                   /* flow_flush */                \
> +    NULL,                   /* flow_dump_create */          \
> +    NULL,                   /* flow_dump_destroy */         \
> +    NULL,                   /* flow_dump_next */            \
> +    NULL,                   /* flow_put */                  \
> +    NULL,                   /* flow_get */                  \
> +    NULL,                   /* flow_del */                  \
> +    NULL,                   /* init_flow_api */
>  
>  
>  #define TUNNEL_CLASS(NAME, DPIF_PORT, BUILD_HEADER, PUSH_HEADER, POP_HEADER)   \
> diff --git a/lib/netdev.c b/lib/netdev.c
> index a8d8eda..1677027 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -1998,3 +1998,101 @@ netdev_reconfigure(struct netdev *netdev)
>              ? class->reconfigure(netdev)
>              : EOPNOTSUPP);
>  }
> +
> +int
> +netdev_flow_flush(struct netdev *netdev)
> +{
> +    const struct netdev_class *class = netdev->netdev_class;
> +
> +    return (class->flow_flush
> +            ? class->flow_flush(netdev)
> +            : EOPNOTSUPP);
> +}
> +
> +int
> +netdev_flow_dump_create(struct netdev *netdev, struct netdev_flow_dump **dump)
> +{
> +    const struct netdev_class *class = netdev->netdev_class;
> +
> +    if (class->flow_dump_create) {
> +        return class->flow_dump_create(netdev, dump);
> +    }
> +
> +    return EOPNOTSUPP;
> +}
> +
> +int
> +netdev_flow_dump_destroy(struct netdev_flow_dump *dump)
> +{
> +    const struct netdev_class *class = dump->netdev->netdev_class;
> +
> +    if (class->flow_dump_destroy) {
> +        return class->flow_dump_destroy(dump);
> +    }
> +
> +    return EOPNOTSUPP;
> +}
> +
> +bool
> +netdev_flow_dump_next(struct netdev_flow_dump *dump,
> +                      struct match *match,
> +                      struct nlattr **actions,
> +                      struct dpif_flow_stats *stats,
> +                      ovs_u128 *ufid,
> +                      struct ofpbuf *rbuffer,
> +                      struct ofpbuf *wbuffer)
> +{
> +    const struct netdev_class *class = dump->netdev->netdev_class;
> +
> +    return (class->flow_dump_next
> +            ? class->flow_dump_next(dump, match, actions, stats, ufid,
> +                                    rbuffer, wbuffer)
> +            : false);

Sorry to nitpick, but some functions are using the format above and
others the format below (perhaps less dense):
[...]
	if () {
	   return class->function()
	}

	return default_ret;

It would be great if they all followed the same style.

Also, some function declarations have one parameter per line when it
could have been at least two per line (less than 79 chars).

fbl


> +}
> +
> +int
> +netdev_flow_put(struct netdev *netdev, struct match *match,
> +                struct nlattr *actions, size_t act_len,
> +                const ovs_u128 *ufid, struct offload_info *info,
> +                struct dpif_flow_stats *stats)
> +{
> +    const struct netdev_class *class = netdev->netdev_class;
> +
> +    return (class->flow_put
> +            ? class->flow_put(netdev, match, actions, act_len, ufid,
> +                              info, stats)
> +            : EOPNOTSUPP);
> +}
> +
> +int
> +netdev_flow_get(struct netdev *netdev, struct match *match,
> +                struct nlattr **actions, const ovs_u128 *ufid,
> +                struct dpif_flow_stats *stats, struct ofpbuf *buf)
> +{
> +    const struct netdev_class *class = netdev->netdev_class;
> +
> +    return (class->flow_get
> +            ? class->flow_get(netdev, match, actions, ufid, stats, buf)
> +            : EOPNOTSUPP);
> +}
> +
> +int
> +netdev_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
> +                struct dpif_flow_stats *stats)
> +{
> +    const struct netdev_class *class = netdev->netdev_class;
> +
> +    return (class->flow_del
> +            ? class->flow_del(netdev, ufid, stats)
> +            : EOPNOTSUPP);
> +}
> +
> +int
> +netdev_init_flow_api(struct netdev *netdev)
> +{
> +    const struct netdev_class *class = netdev->netdev_class;
> +
> +    return (class->init_flow_api
> +            ? class->init_flow_api(netdev)
> +            : EOPNOTSUPP);
> +}
> diff --git a/lib/netdev.h b/lib/netdev.h
> index d6c07c1..17890e6 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -156,6 +156,29 @@ int netdev_send(struct netdev *, int qid, struct dp_packet_batch *,
>                  bool may_steal, bool concurrent_txq);
>  void netdev_send_wait(struct netdev *, int qid);
>  
> +/* Flow offloading. */
> +struct offload_info {
> +    const void *port_hmap_obj; /* To query ports info from netdev port map */
> +    ovs_be16 tp_dst_port; /* Destination port for tunnel in SET action */
> +};
> +struct netdev_flow_dump;
> +int netdev_flow_flush(struct netdev *);
> +int netdev_flow_dump_create(struct netdev *, struct netdev_flow_dump **dump);
> +int netdev_flow_dump_destroy(struct netdev_flow_dump *);
> +bool netdev_flow_dump_next(struct netdev_flow_dump *, struct match *,
> +                          struct nlattr **actions, struct dpif_flow_stats *,
> +                          ovs_u128 *ufid, struct ofpbuf *rbuffer,
> +                          struct ofpbuf *wbuffer);
> +int netdev_flow_put(struct netdev *, struct match *, struct nlattr *actions,
> +                    size_t actions_len, const ovs_u128 *,
> +                    struct offload_info *, struct dpif_flow_stats *);
> +int netdev_flow_get(struct netdev *, struct match *, struct nlattr **actions,
> +                    const ovs_u128 *, struct dpif_flow_stats *,
> +                    struct ofpbuf *wbuffer);
> +int netdev_flow_del(struct netdev *, const ovs_u128 *,
> +                    struct dpif_flow_stats *);
> +int netdev_init_flow_api(struct netdev *);
> +
>  /* native tunnel APIs */
>  /* Structure to pass parameters required to build a tunnel header. */
>  struct netdev_tnl_build_header_params {
> -- 
> 2.7.4
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Roi Dayan May 15, 2017, 6:28 a.m. UTC | #3
On 08/05/2017 15:28, Simon Horman wrote:
> On Wed, May 03, 2017 at 06:07:53PM +0300, Roi Dayan wrote:
>> From: Paul Blakey <paulb@mellanox.com>
>
> Please add some text to the changelog.
>
>>
>> Signed-off-by: Paul Blakey <paulb@mellanox.com>
>> Reviewed-by: Roi Dayan <roid@mellanox.com>
>> Reviewed-by: Simon Horman <simon.horman@netronome.com>
>
> ...
>
>> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
>> new file mode 100644
>> index 0000000..eb5e79a
>> --- /dev/null
>> +++ b/lib/netdev-tc-offloads.c
>> @@ -0,0 +1,154 @@
>> +/*
>> + * Copyright (c) 2016 Mellanox Technologies, Ltd.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at:
>> + *
>> + *     http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#include <config.h>
>> +
>> +#include "netdev-tc-offloads.h"
>> +
>> +#include <errno.h>
>> +#include <fcntl.h>
>> +#include <arpa/inet.h>
>> +#include <inttypes.h>
>> +#include <linux/filter.h>
>> +#include <linux/gen_stats.h>
>> +#include <linux/if_ether.h>
>> +#include <linux/if_tun.h>
>> +#include <linux/types.h>
>> +#include <linux/ethtool.h>
>> +#include <linux/mii.h>
>> +#include <linux/pkt_cls.h>
>> +#include <linux/pkt_sched.h>
>> +#include <linux/rtnetlink.h>
>> +#include <linux/sockios.h>
>> +#include <sys/types.h>
>> +#include <sys/ioctl.h>
>> +#include <sys/socket.h>
>> +#include <sys/utsname.h>
>> +#include <netpacket/packet.h>
>> +#include <net/if.h>
>> +#include <net/if_arp.h>
>> +#include <net/if_packet.h>
>> +#include <net/route.h>
>> +#include <netinet/in.h>
>> +#include <poll.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <unistd.h>
>> +
>> +#include "coverage.h"
>> +#include "dp-packet.h"
>> +#include "dpif-netlink.h"
>> +#include "dpif-netdev.h"
>> +#include "openvswitch/dynamic-string.h"
>> +#include "fatal-signal.h"
>> +#include "hash.h"
>> +#include "openvswitch/hmap.h"
>> +#include "netdev-provider.h"
>> +#include "netdev-vport.h"
>> +#include "netlink-notifier.h"
>> +#include "netlink-socket.h"
>> +#include "netlink.h"
>> +#include "openvswitch/ofpbuf.h"
>> +#include "openflow/openflow.h"
>> +#include "ovs-atomic.h"
>> +#include "packets.h"
>> +#include "poll-loop.h"
>> +#include "rtnetlink.h"
>> +#include "openvswitch/shash.h"
>> +#include "netdev-provider.h"
>> +#include "openvswitch/match.h"
>> +#include "openvswitch/vlog.h"
>> +#include "tc.h"
>
> Are all of the above headers needed for what follows?
> There seems to be a lot of #includes above.

probably not. was started as copy paste to avoid compile errors and we 
forgot to clean it.

>
>> +VLOG_DEFINE_THIS_MODULE(netdev_tc_offloads);
>> +
>> +int
>> +netdev_tc_flow_flush(struct netdev *netdev OVS_UNUSED)
>> +{
>> +    return EOPNOTSUPP;
>> +}
>> +
>> +int
>> +netdev_tc_flow_dump_create(struct netdev *netdev,
>> +                           struct netdev_flow_dump **dump_out)
>> +{
>> +    struct netdev_flow_dump *dump = xzalloc(sizeof *dump);
>> +
>> +    dump->netdev = netdev_ref(netdev);
>> +
>> +    *dump_out = dump;
>> +
>> +    return 0;
>> +}
>> +
>> +int
>> +netdev_tc_flow_dump_destroy(struct netdev_flow_dump *dump)
>> +{
>> +    netdev_close(dump->netdev);
>> +    free(dump);
>> +
>> +    return 0;
>> +}
>> +
>> +bool
>> +netdev_tc_flow_dump_next(struct netdev_flow_dump *dump OVS_UNUSED,
>> +                         struct match *match OVS_UNUSED,
>> +                         struct nlattr **actions OVS_UNUSED,
>> +                         struct dpif_flow_stats *stats OVS_UNUSED,
>> +                         ovs_u128 *ufid OVS_UNUSED,
>> +                         struct ofpbuf *rbuffer OVS_UNUSED,
>> +                         struct ofpbuf *wbuffer OVS_UNUSED)
>> +{
>> +    return false;
>> +}
>> +
>> +int
>> +netdev_tc_flow_put(struct netdev *netdev OVS_UNUSED,
>> +                   struct match *match OVS_UNUSED,
>> +                   struct nlattr *actions OVS_UNUSED,
>> +                   size_t actions_len OVS_UNUSED,
>> +                   struct dpif_flow_stats *stats OVS_UNUSED,
>> +                   const ovs_u128 *ufid OVS_UNUSED,
>
> Here and above ufid follows stats.
>
>> +                   struct offload_info *info OVS_UNUSED)
>> +{
>> +    return EOPNOTSUPP;
>> +}
>> +
>> +int
>> +netdev_tc_flow_get(struct netdev *netdev OVS_UNUSED,
>> +                   struct match *match OVS_UNUSED,
>> +                   struct nlattr **actions OVS_UNUSED,
>> +                   const ovs_u128 *ufid OVS_UNUSED,
>> +                   struct dpif_flow_stats *stats OVS_UNUSED,
>
> But here the order is reversed.
>
> I always think that when a reviewer asks for entries to be sorted they
> have run out of things to say. But nonetheless it would be slightly
> nicer if the order was consistent unless there is a reason for it not to
> be.

I agree. In one of the submissions I fixed the order but probably didn't 
notice this one. thanks.

>
>> +                   struct ofpbuf *buf OVS_UNUSED)
>> +{
>> +    return EOPNOTSUPP;
>> +}
>> +
>> +int
>> +netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
>> +                   const ovs_u128 *ufid OVS_UNUSED,
>> +                   struct dpif_flow_stats *stats OVS_UNUSED)
>> +{
>> +    return EOPNOTSUPP;
>> +}
>> +
>> +int
>> +netdev_tc_init_flow_api(struct netdev *netdev OVS_UNUSED)
>> +{
>> +    return 0;
>> +}
>> +
>
> There is a trailing blank line above.

ok

>
>> diff --git a/lib/netdev-tc-offloads.h b/lib/netdev-tc-offloads.h
>> new file mode 100644
>> index 0000000..76b7938
>> --- /dev/null
>> +++ b/lib/netdev-tc-offloads.h
>> @@ -0,0 +1,42 @@
>
>> +int netdev_tc_flow_put(struct netdev *, struct match *,
>> +                       struct nlattr *actions, size_t actions_len,
>> +                       const ovs_u128 *, struct offload_info *,
>> +                       struct dpif_flow_stats *);
>
> The declaration of netdev_tc_flow_put() does not match its definition.
>
> ...
>

ok. probably happened from all the rebases. i'll fix it.
Roi Dayan May 15, 2017, 6:34 a.m. UTC | #4
On 09/05/2017 21:12, Flavio Leitner wrote:
> On Wed, May 03, 2017 at 06:07:53PM +0300, Roi Dayan wrote:
>> From: Paul Blakey <paulb@mellanox.com>
>>
>> Signed-off-by: Paul Blakey <paulb@mellanox.com>
>> Reviewed-by: Roi Dayan <roid@mellanox.com>
>> Reviewed-by: Simon Horman <simon.horman@netronome.com>
>> ---
>>  lib/automake.mk          |   2 +
>>  lib/netdev-bsd.c         |   2 +
>>  lib/netdev-dpdk.c        |   1 +
>>  lib/netdev-dummy.c       |   2 +
>>  lib/netdev-linux.c       |  15 +++--
>>  lib/netdev-linux.h       |   9 +++
>>  lib/netdev-provider.h    |  71 ++++++++++++++++++++++
>>  lib/netdev-tc-offloads.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++
>>  lib/netdev-tc-offloads.h |  42 +++++++++++++
>>  lib/netdev-vport.c       |  11 +++-
>>  lib/netdev.c             |  98 ++++++++++++++++++++++++++++++
>>  lib/netdev.h             |  23 +++++++
>>  12 files changed, 425 insertions(+), 5 deletions(-)
>>  create mode 100644 lib/netdev-tc-offloads.c
>>  create mode 100644 lib/netdev-tc-offloads.h
>>
>> diff --git a/lib/automake.mk b/lib/automake.mk
>> index 3d57610..a7c8009 100644
>> --- a/lib/automake.mk
>> +++ b/lib/automake.mk
>> @@ -354,6 +354,8 @@ lib_libopenvswitch_la_SOURCES += \
>>  	lib/dpif-netlink.h \
>>  	lib/tc.h \
>>  	lib/tc.c \
>> +	lib/netdev-tc-offloads.h \
>> +	lib/netdev-tc-offloads.c \
>>  	lib/if-notifier.c \
>>  	lib/if-notifier.h \
>>  	lib/netdev-linux.c \
>> diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
>> index 94c515d..5b54d79 100644
>> --- a/lib/netdev-bsd.c
>> +++ b/lib/netdev-bsd.c
>> @@ -1547,6 +1547,8 @@ netdev_bsd_update_flags(struct netdev *netdev_, enum netdev_flags off,
>>      netdev_bsd_rxq_recv,                             \
>>      netdev_bsd_rxq_wait,                             \
>>      netdev_bsd_rxq_drain,                            \
>> +                                                     \
>> +    NO_OFFLOAD_API                                   \
>>  }
>>
>>  const struct netdev_class netdev_bsd_class =
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index ddc651b..9ad96cd 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -3314,6 +3314,7 @@ unlock:
>>      RXQ_RECV,                                                 \
>>      NULL,                       /* rx_wait */                 \
>>      NULL,                       /* rxq_drain */               \
>> +    NO_OFFLOAD_API                                            \
>>  }
>>
>>  static const struct netdev_class dpdk_class =
>> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
>> index 0657434..7e1383f 100644
>> --- a/lib/netdev-dummy.c
>> +++ b/lib/netdev-dummy.c
>> @@ -1409,6 +1409,8 @@ netdev_dummy_update_flags(struct netdev *netdev_,
>>      netdev_dummy_rxq_recv,                                      \
>>      netdev_dummy_rxq_wait,                                      \
>>      netdev_dummy_rxq_drain,                                     \
>> +                                                                \
>> +    NO_OFFLOAD_API                                              \
>>  }
>>
>>  static const struct netdev_class dummy_class =
>> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
>> index a6bb515..f7941fd 100644
>> --- a/lib/netdev-linux.c
>> +++ b/lib/netdev-linux.c
>> @@ -73,6 +73,7 @@
>>  #include "openvswitch/vlog.h"
>>  #include "util.h"
>>  #include "tc.h"
>> +#include "netdev-tc-offloads.h"
>>
>>  VLOG_DEFINE_THIS_MODULE(netdev_linux);
>>
>> @@ -2783,7 +2784,8 @@ netdev_linux_update_flags(struct netdev *netdev_, enum netdev_flags off,
>>  }
>>
>>  #define NETDEV_LINUX_CLASS(NAME, CONSTRUCT, GET_STATS,          \
>> -                           GET_FEATURES, GET_STATUS)            \
>> +                           GET_FEATURES, GET_STATUS,            \
>> +                           FLOW_OFFLOAD_API)                    \
>>  {                                                               \
>>      NAME,                                                       \
>>      false,                      /* is_pmd */                    \
>> @@ -2852,6 +2854,8 @@ netdev_linux_update_flags(struct netdev *netdev_, enum netdev_flags off,
>>      netdev_linux_rxq_recv,                                      \
>>      netdev_linux_rxq_wait,                                      \
>>      netdev_linux_rxq_drain,                                     \
>> +                                                                \
>> +    FLOW_OFFLOAD_API                                            \
>>  }
>>
>>  const struct netdev_class netdev_linux_class =
>> @@ -2860,7 +2864,8 @@ const struct netdev_class netdev_linux_class =
>>          netdev_linux_construct,
>>          netdev_linux_get_stats,
>>          netdev_linux_get_features,
>> -        netdev_linux_get_status);
>> +        netdev_linux_get_status,
>> +        LINUX_FLOW_OFFLOAD_API);
>>
>>  const struct netdev_class netdev_tap_class =
>>      NETDEV_LINUX_CLASS(
>> @@ -2868,7 +2873,8 @@ const struct netdev_class netdev_tap_class =
>>          netdev_linux_construct_tap,
>>          netdev_tap_get_stats,
>>          netdev_linux_get_features,
>> -        netdev_linux_get_status);
>> +        netdev_linux_get_status,
>> +        NO_OFFLOAD_API);
>>
>>  const struct netdev_class netdev_internal_class =
>>      NETDEV_LINUX_CLASS(
>> @@ -2876,7 +2882,8 @@ const struct netdev_class netdev_internal_class =
>>          netdev_linux_construct,
>>          netdev_internal_get_stats,
>>          NULL,                  /* get_features */
>> -        netdev_internal_get_status);
>> +        netdev_internal_get_status,
>> +        NO_OFFLOAD_API);
>>  
>>
>>  #define CODEL_N_QUEUES 0x0000
>> diff --git a/lib/netdev-linux.h b/lib/netdev-linux.h
>> index 0c61bc9..d944691 100644
>> --- a/lib/netdev-linux.h
>> +++ b/lib/netdev-linux.h
>> @@ -28,4 +28,13 @@ struct netdev;
>>  int netdev_linux_ethtool_set_flag(struct netdev *netdev, uint32_t flag,
>>                                    const char *flag_name, bool enable);
>>
>> +#define LINUX_FLOW_OFFLOAD_API                                  \
>> +            netdev_tc_flow_flush,                               \
>> +            netdev_tc_flow_dump_create,                         \
>> +            netdev_tc_flow_dump_destroy,                        \
>> +            netdev_tc_flow_dump_next,                           \
>> +            netdev_tc_flow_put,                                 \
>> +            netdev_tc_flow_get,                                 \
>> +            netdev_tc_flow_del,                                 \
>> +            netdev_tc_init_flow_api
>>  #endif /* netdev-linux.h */
>> diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
>> index 8346fc4..e8bfdc8 100644
>> --- a/lib/netdev-provider.h
>> +++ b/lib/netdev-provider.h
>> @@ -115,6 +115,14 @@ struct netdev_rxq {
>>
>>  struct netdev *netdev_rxq_get_netdev(const struct netdev_rxq *);
>>
>> +
>> +struct netdev_flow_dump {
>> +    struct netdev *netdev;
>> +    odp_port_t port;
>> +    struct nl_dump *nl_dump;
>> +    bool terse;
>> +};
>> +
>>  /* Network device class structure, to be defined by each implementation of a
>>   * network device.
>>   *
>> @@ -769,6 +777,67 @@ struct netdev_class {
>>
>>      /* Discards all packets waiting to be received from 'rx'. */
>>      int (*rxq_drain)(struct netdev_rxq *rx);
>> +
>> +    /* ## -------------------------------- ## */
>> +    /* ## netdev flow offloading functions ## */
>> +    /* ## -------------------------------- ## */
>> +
>> +    /* If a particular netdev class does not support offloading flows,
>> +     * all these function pointers must be NULL. */
>> +
>> +    /* Flush all offloaded flows from a netdev.
>> +     * Return 0 if successful, otherwise returns a positive errno value. */
>> +    int (*flow_flush)(struct netdev *);
>> +
>> +    /* Flow dumping interface.
>> +     *
>> +     * This is the back-end for the flow dumping interface described in
>> +     * dpif.h.  Please read the comments there first, because this code
>> +     * closely follows it.
>> +     *
>> +     * 'flow_dump_create' is being executed in a dpif thread so there is
>> +     * no need for 'flow_dump_thread_create' implementation.
>> +     * On success returns allocated netdev_flow_dump data, on failure returns
>> +     * positive errno. */
>> +    int (*flow_dump_create)(struct netdev *, struct netdev_flow_dump **dump);
>> +    int (*flow_dump_destroy)(struct netdev_flow_dump *);
>> +
>> +    /* Returns true while there are more flows to dump.
>> +     * rbuffer is used as a temporary buffer and needs to be pre allocated
>> +     * by the caller. while there are more flows the same rbuffer should
>> +     * be provided. wbuffer is used to store dumped actions and needs to be
>> +     * pre allocated by the caller. */
>> +    bool (*flow_dump_next)(struct netdev_flow_dump *, struct match *,
>> +                           struct nlattr **actions,
>> +                           struct dpif_flow_stats *stats, ovs_u128 *ufid,
>> +                           struct ofpbuf *rbuffer, struct ofpbuf *wbuffer);
>> +
>> +    /* Offload the given flow on netdev.
>> +     * To modify a flow, use the same ufid.
>> +     * actions are in netlink format, as with struct dpif_flow_put.
>> +     * info is extra info needed to offload the flow.
>> +     * Read the comments on 'struct dpif_flow_put' in dpif.h about stats.
>> +     * Return 0 if successful, otherwise returns a positive errno value. */
>> +    int (*flow_put)(struct netdev *, struct match *, struct nlattr *actions,
>> +                    size_t actions_len, const ovs_u128 *ufid,
>> +                    struct offload_info *info, struct dpif_flow_stats *);
>> +
>> +    /* Queries a flow specified by ufid on netdev.
>> +     * Fills output buffer as wbuffer in flow_dump_next.
>> +     * the buffer needs to be pre allocated.
>> +     * Return 0 if successful, otherwise returns a positive errno value. */
>> +    int (*flow_get)(struct netdev *, struct match *, struct nlattr **actions,
>> +                    const ovs_u128 *ufid, struct dpif_flow_stats *,
>> +                    struct ofpbuf *wbuffer);
>> +
>> +    /* Delete a flow specified by ufid from netdev.
>> +     * Read the comments on 'struct dpif_flow_del' in dpif.h about stats.
>> +     * Return 0 if successful, otherwise returns a positive errno value. */
>> +    int (*flow_del)(struct netdev *, const ovs_u128 *ufid,
>> +                    struct dpif_flow_stats *);
>> +
>> +    /* Initializies the netdev flow api. */
>> +    int (*init_flow_api)(struct netdev *);
>>  };
>>
>>  int netdev_register_provider(const struct netdev_class *);
>> @@ -788,4 +857,6 @@ extern const struct netdev_class netdev_tap_class;
>>  }
>>  #endif
>>
>> +#define NO_OFFLOAD_API NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL
>> +
>>  #endif /* netdev.h */
>> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
>> new file mode 100644
>> index 0000000..eb5e79a
>> --- /dev/null
>> +++ b/lib/netdev-tc-offloads.c
>> @@ -0,0 +1,154 @@
>> +/*
>> + * Copyright (c) 2016 Mellanox Technologies, Ltd.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at:
>> + *
>> + *     http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#include <config.h>
>> +
>> +#include "netdev-tc-offloads.h"
>> +
>> +#include <errno.h>
>> +#include <fcntl.h>
>> +#include <arpa/inet.h>
>> +#include <inttypes.h>
>> +#include <linux/filter.h>
>> +#include <linux/gen_stats.h>
>> +#include <linux/if_ether.h>
>> +#include <linux/if_tun.h>
>> +#include <linux/types.h>
>> +#include <linux/ethtool.h>
>> +#include <linux/mii.h>
>> +#include <linux/pkt_cls.h>
>> +#include <linux/pkt_sched.h>
>> +#include <linux/rtnetlink.h>
>> +#include <linux/sockios.h>
>> +#include <sys/types.h>
>> +#include <sys/ioctl.h>
>> +#include <sys/socket.h>
>> +#include <sys/utsname.h>
>> +#include <netpacket/packet.h>
>> +#include <net/if.h>
>> +#include <net/if_arp.h>
>> +#include <net/if_packet.h>
>> +#include <net/route.h>
>> +#include <netinet/in.h>
>> +#include <poll.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <unistd.h>
>> +
>> +#include "coverage.h"
>> +#include "dp-packet.h"
>> +#include "dpif-netlink.h"
>> +#include "dpif-netdev.h"
>> +#include "openvswitch/dynamic-string.h"
>> +#include "fatal-signal.h"
>> +#include "hash.h"
>> +#include "openvswitch/hmap.h"
>> +#include "netdev-provider.h"
>> +#include "netdev-vport.h"
>> +#include "netlink-notifier.h"
>> +#include "netlink-socket.h"
>> +#include "netlink.h"
>> +#include "openvswitch/ofpbuf.h"
>> +#include "openflow/openflow.h"
>> +#include "ovs-atomic.h"
>> +#include "packets.h"
>> +#include "poll-loop.h"
>> +#include "rtnetlink.h"
>> +#include "openvswitch/shash.h"
>> +#include "netdev-provider.h"
>> +#include "openvswitch/match.h"
>> +#include "openvswitch/vlog.h"
>> +#include "tc.h"
>> +
>> +VLOG_DEFINE_THIS_MODULE(netdev_tc_offloads);
>> +
>> +int
>> +netdev_tc_flow_flush(struct netdev *netdev OVS_UNUSED)
>> +{
>> +    return EOPNOTSUPP;
>> +}
>> +
>> +int
>> +netdev_tc_flow_dump_create(struct netdev *netdev,
>> +                           struct netdev_flow_dump **dump_out)
>> +{
>> +    struct netdev_flow_dump *dump = xzalloc(sizeof *dump);
>> +
>> +    dump->netdev = netdev_ref(netdev);
>> +
>> +    *dump_out = dump;
>> +
>> +    return 0;
>> +}
>> +
>> +int
>> +netdev_tc_flow_dump_destroy(struct netdev_flow_dump *dump)
>> +{
>> +    netdev_close(dump->netdev);
>> +    free(dump);
>> +
>> +    return 0;
>> +}
>> +
>> +bool
>> +netdev_tc_flow_dump_next(struct netdev_flow_dump *dump OVS_UNUSED,
>> +                         struct match *match OVS_UNUSED,
>> +                         struct nlattr **actions OVS_UNUSED,
>> +                         struct dpif_flow_stats *stats OVS_UNUSED,
>> +                         ovs_u128 *ufid OVS_UNUSED,
>> +                         struct ofpbuf *rbuffer OVS_UNUSED,
>> +                         struct ofpbuf *wbuffer OVS_UNUSED)
>> +{
>> +    return false;
>> +}
>> +
>> +int
>> +netdev_tc_flow_put(struct netdev *netdev OVS_UNUSED,
>> +                   struct match *match OVS_UNUSED,
>> +                   struct nlattr *actions OVS_UNUSED,
>> +                   size_t actions_len OVS_UNUSED,
>> +                   struct dpif_flow_stats *stats OVS_UNUSED,
>> +                   const ovs_u128 *ufid OVS_UNUSED,
>> +                   struct offload_info *info OVS_UNUSED)
>> +{
>> +    return EOPNOTSUPP;
>> +}
>> +
>> +int
>> +netdev_tc_flow_get(struct netdev *netdev OVS_UNUSED,
>> +                   struct match *match OVS_UNUSED,
>> +                   struct nlattr **actions OVS_UNUSED,
>> +                   const ovs_u128 *ufid OVS_UNUSED,
>> +                   struct dpif_flow_stats *stats OVS_UNUSED,
>> +                   struct ofpbuf *buf OVS_UNUSED)
>> +{
>> +    return EOPNOTSUPP;
>> +}
>> +
>> +int
>> +netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
>> +                   const ovs_u128 *ufid OVS_UNUSED,
>> +                   struct dpif_flow_stats *stats OVS_UNUSED)
>> +{
>> +    return EOPNOTSUPP;
>> +}
>> +
>> +int
>> +netdev_tc_init_flow_api(struct netdev *netdev OVS_UNUSED)
>> +{
>> +    return 0;
>> +}
>> +
>> diff --git a/lib/netdev-tc-offloads.h b/lib/netdev-tc-offloads.h
>> new file mode 100644
>> index 0000000..76b7938
>> --- /dev/null
>> +++ b/lib/netdev-tc-offloads.h
>> @@ -0,0 +1,42 @@
>> +/*
>> + * Copyright (c) 2016 Mellanox Technologies, Ltd.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at:
>> + *
>> + *     http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#ifndef NETDEV_TC_OFFLOADS_H
>> +#define NETDEV_TC_OFFLOADS_H 1
>> +
>> +#include "netdev.h"
>> +
>> +int netdev_tc_flow_flush(struct netdev *);
>> +int netdev_tc_flow_dump_create(struct netdev *, struct netdev_flow_dump **);
>> +int netdev_tc_flow_dump_destroy(struct netdev_flow_dump *);
>> +bool netdev_tc_flow_dump_next(struct netdev_flow_dump *, struct match *,
>> +                              struct nlattr **actions,
>> +                              struct dpif_flow_stats *,
>> +                              ovs_u128 *ufid,
>> +                              struct ofpbuf *rbuffer,
>> +                              struct ofpbuf *wbuffer);
>> +int netdev_tc_flow_put(struct netdev *, struct match *,
>> +                       struct nlattr *actions, size_t actions_len,
>> +                       const ovs_u128 *, struct offload_info *,
>> +                       struct dpif_flow_stats *);
>> +int netdev_tc_flow_get(struct netdev *, struct match *,
>> +                       struct nlattr **actions, const ovs_u128 *,
>> +                       struct dpif_flow_stats *, struct ofpbuf *);
>> +int netdev_tc_flow_del(struct netdev *, const ovs_u128 *,
>> +                        struct dpif_flow_stats *);
>> +int netdev_tc_init_flow_api(struct netdev *);
>> +
>> +#endif /* netdev-tc-offloads.h */
>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
>> index 39093e8..2cad5cb 100644
>> --- a/lib/netdev-vport.c
>> +++ b/lib/netdev-vport.c
>> @@ -847,7 +847,16 @@ get_stats(const struct netdev *netdev, struct netdev_stats *stats)
>>      NULL,                   /* rx_dealloc */                \
>>      NULL,                   /* rx_recv */                   \
>>      NULL,                   /* rx_wait */                   \
>> -    NULL,                   /* rx_drain */
>> +    NULL,                   /* rx_drain */                  \
>> +                                                            \
>> +    NULL,                   /* flow_flush */                \
>> +    NULL,                   /* flow_dump_create */          \
>> +    NULL,                   /* flow_dump_destroy */         \
>> +    NULL,                   /* flow_dump_next */            \
>> +    NULL,                   /* flow_put */                  \
>> +    NULL,                   /* flow_get */                  \
>> +    NULL,                   /* flow_del */                  \
>> +    NULL,                   /* init_flow_api */
>>
>>
>>  #define TUNNEL_CLASS(NAME, DPIF_PORT, BUILD_HEADER, PUSH_HEADER, POP_HEADER)   \
>> diff --git a/lib/netdev.c b/lib/netdev.c
>> index a8d8eda..1677027 100644
>> --- a/lib/netdev.c
>> +++ b/lib/netdev.c
>> @@ -1998,3 +1998,101 @@ netdev_reconfigure(struct netdev *netdev)
>>              ? class->reconfigure(netdev)
>>              : EOPNOTSUPP);
>>  }
>> +
>> +int
>> +netdev_flow_flush(struct netdev *netdev)
>> +{
>> +    const struct netdev_class *class = netdev->netdev_class;
>> +
>> +    return (class->flow_flush
>> +            ? class->flow_flush(netdev)
>> +            : EOPNOTSUPP);
>> +}
>> +
>> +int
>> +netdev_flow_dump_create(struct netdev *netdev, struct netdev_flow_dump **dump)
>> +{
>> +    const struct netdev_class *class = netdev->netdev_class;
>> +
>> +    if (class->flow_dump_create) {
>> +        return class->flow_dump_create(netdev, dump);
>> +    }
>> +
>> +    return EOPNOTSUPP;
>> +}
>> +
>> +int
>> +netdev_flow_dump_destroy(struct netdev_flow_dump *dump)
>> +{
>> +    const struct netdev_class *class = dump->netdev->netdev_class;
>> +
>> +    if (class->flow_dump_destroy) {
>> +        return class->flow_dump_destroy(dump);
>> +    }
>> +
>> +    return EOPNOTSUPP;
>> +}
>> +
>> +bool
>> +netdev_flow_dump_next(struct netdev_flow_dump *dump,
>> +                      struct match *match,
>> +                      struct nlattr **actions,
>> +                      struct dpif_flow_stats *stats,
>> +                      ovs_u128 *ufid,
>> +                      struct ofpbuf *rbuffer,
>> +                      struct ofpbuf *wbuffer)
>> +{
>> +    const struct netdev_class *class = dump->netdev->netdev_class;
>> +
>> +    return (class->flow_dump_next
>> +            ? class->flow_dump_next(dump, match, actions, stats, ufid,
>> +                                    rbuffer, wbuffer)
>> +            : false);
>
> Sorry to nitpick, but some functions are using the format above and
> others the format below (perhaps less dense):
> [...]
> 	if () {
> 	   return class->function()
> 	}
>
> 	return default_ret;
>
> It would be great if they all followed the same style.

ok

>
> Also, some function declarations have one parameter per line when it
> could have been at least two per line (less than 79 chars).
>
> fbl

usually I think it's more readable to have 1 parameter per line when we 
already have to break to more than a single line.
Should we always squeeze args to a single line when possible?

>
>
>> +}
>> +
>> +int
>> +netdev_flow_put(struct netdev *netdev, struct match *match,
>> +                struct nlattr *actions, size_t act_len,
>> +                const ovs_u128 *ufid, struct offload_info *info,
>> +                struct dpif_flow_stats *stats)
>> +{
>> +    const struct netdev_class *class = netdev->netdev_class;
>> +
>> +    return (class->flow_put
>> +            ? class->flow_put(netdev, match, actions, act_len, ufid,
>> +                              info, stats)
>> +            : EOPNOTSUPP);
>> +}
>> +
>> +int
>> +netdev_flow_get(struct netdev *netdev, struct match *match,
>> +                struct nlattr **actions, const ovs_u128 *ufid,
>> +                struct dpif_flow_stats *stats, struct ofpbuf *buf)
>> +{
>> +    const struct netdev_class *class = netdev->netdev_class;
>> +
>> +    return (class->flow_get
>> +            ? class->flow_get(netdev, match, actions, ufid, stats, buf)
>> +            : EOPNOTSUPP);
>> +}
>> +
>> +int
>> +netdev_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
>> +                struct dpif_flow_stats *stats)
>> +{
>> +    const struct netdev_class *class = netdev->netdev_class;
>> +
>> +    return (class->flow_del
>> +            ? class->flow_del(netdev, ufid, stats)
>> +            : EOPNOTSUPP);
>> +}
>> +
>> +int
>> +netdev_init_flow_api(struct netdev *netdev)
>> +{
>> +    const struct netdev_class *class = netdev->netdev_class;
>> +
>> +    return (class->init_flow_api
>> +            ? class->init_flow_api(netdev)
>> +            : EOPNOTSUPP);
>> +}
>> diff --git a/lib/netdev.h b/lib/netdev.h
>> index d6c07c1..17890e6 100644
>> --- a/lib/netdev.h
>> +++ b/lib/netdev.h
>> @@ -156,6 +156,29 @@ int netdev_send(struct netdev *, int qid, struct dp_packet_batch *,
>>                  bool may_steal, bool concurrent_txq);
>>  void netdev_send_wait(struct netdev *, int qid);
>>
>> +/* Flow offloading. */
>> +struct offload_info {
>> +    const void *port_hmap_obj; /* To query ports info from netdev port map */
>> +    ovs_be16 tp_dst_port; /* Destination port for tunnel in SET action */
>> +};
>> +struct netdev_flow_dump;
>> +int netdev_flow_flush(struct netdev *);
>> +int netdev_flow_dump_create(struct netdev *, struct netdev_flow_dump **dump);
>> +int netdev_flow_dump_destroy(struct netdev_flow_dump *);
>> +bool netdev_flow_dump_next(struct netdev_flow_dump *, struct match *,
>> +                          struct nlattr **actions, struct dpif_flow_stats *,
>> +                          ovs_u128 *ufid, struct ofpbuf *rbuffer,
>> +                          struct ofpbuf *wbuffer);
>> +int netdev_flow_put(struct netdev *, struct match *, struct nlattr *actions,
>> +                    size_t actions_len, const ovs_u128 *,
>> +                    struct offload_info *, struct dpif_flow_stats *);
>> +int netdev_flow_get(struct netdev *, struct match *, struct nlattr **actions,
>> +                    const ovs_u128 *, struct dpif_flow_stats *,
>> +                    struct ofpbuf *wbuffer);
>> +int netdev_flow_del(struct netdev *, const ovs_u128 *,
>> +                    struct dpif_flow_stats *);
>> +int netdev_init_flow_api(struct netdev *);
>> +
>>  /* native tunnel APIs */
>>  /* Structure to pass parameters required to build a tunnel header. */
>>  struct netdev_tnl_build_header_params {
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Flavio Leitner May 16, 2017, 12:43 p.m. UTC | #5
On Mon, May 15, 2017 at 09:34:35AM +0300, Roi Dayan wrote:
> 
> 
> On 09/05/2017 21:12, Flavio Leitner wrote:
> > On Wed, May 03, 2017 at 06:07:53PM +0300, Roi Dayan wrote:
> > > From: Paul Blakey <paulb@mellanox.com>
> > > 
> > > Signed-off-by: Paul Blakey <paulb@mellanox.com>
> > > Reviewed-by: Roi Dayan <roid@mellanox.com>
> > > Reviewed-by: Simon Horman <simon.horman@netronome.com>
> > > ---
> > >  lib/automake.mk          |   2 +
> > >  lib/netdev-bsd.c         |   2 +
> > >  lib/netdev-dpdk.c        |   1 +
> > >  lib/netdev-dummy.c       |   2 +
> > >  lib/netdev-linux.c       |  15 +++--
> > >  lib/netdev-linux.h       |   9 +++
> > >  lib/netdev-provider.h    |  71 ++++++++++++++++++++++
> > >  lib/netdev-tc-offloads.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++
> > >  lib/netdev-tc-offloads.h |  42 +++++++++++++
> > >  lib/netdev-vport.c       |  11 +++-
> > >  lib/netdev.c             |  98 ++++++++++++++++++++++++++++++
> > >  lib/netdev.h             |  23 +++++++
> > >  12 files changed, 425 insertions(+), 5 deletions(-)
> > >  create mode 100644 lib/netdev-tc-offloads.c
> > >  create mode 100644 lib/netdev-tc-offloads.h
> > > 
> > > diff --git a/lib/automake.mk b/lib/automake.mk
> > > index 3d57610..a7c8009 100644
> > > --- a/lib/automake.mk
> > > +++ b/lib/automake.mk
> > > @@ -354,6 +354,8 @@ lib_libopenvswitch_la_SOURCES += \
> > >  	lib/dpif-netlink.h \
> > >  	lib/tc.h \
> > >  	lib/tc.c \
> > > +	lib/netdev-tc-offloads.h \
> > > +	lib/netdev-tc-offloads.c \
> > >  	lib/if-notifier.c \
> > >  	lib/if-notifier.h \
> > >  	lib/netdev-linux.c \
> > > diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
> > > index 94c515d..5b54d79 100644
> > > --- a/lib/netdev-bsd.c
> > > +++ b/lib/netdev-bsd.c
> > > @@ -1547,6 +1547,8 @@ netdev_bsd_update_flags(struct netdev *netdev_, enum netdev_flags off,
> > >      netdev_bsd_rxq_recv,                             \
> > >      netdev_bsd_rxq_wait,                             \
> > >      netdev_bsd_rxq_drain,                            \
> > > +                                                     \
> > > +    NO_OFFLOAD_API                                   \
> > >  }
> > > 
> > >  const struct netdev_class netdev_bsd_class =
> > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > > index ddc651b..9ad96cd 100644
> > > --- a/lib/netdev-dpdk.c
> > > +++ b/lib/netdev-dpdk.c
> > > @@ -3314,6 +3314,7 @@ unlock:
> > >      RXQ_RECV,                                                 \
> > >      NULL,                       /* rx_wait */                 \
> > >      NULL,                       /* rxq_drain */               \
> > > +    NO_OFFLOAD_API                                            \
> > >  }
> > > 
> > >  static const struct netdev_class dpdk_class =
> > > diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> > > index 0657434..7e1383f 100644
> > > --- a/lib/netdev-dummy.c
> > > +++ b/lib/netdev-dummy.c
> > > @@ -1409,6 +1409,8 @@ netdev_dummy_update_flags(struct netdev *netdev_,
> > >      netdev_dummy_rxq_recv,                                      \
> > >      netdev_dummy_rxq_wait,                                      \
> > >      netdev_dummy_rxq_drain,                                     \
> > > +                                                                \
> > > +    NO_OFFLOAD_API                                              \
> > >  }
> > > 
> > >  static const struct netdev_class dummy_class =
> > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> > > index a6bb515..f7941fd 100644
> > > --- a/lib/netdev-linux.c
> > > +++ b/lib/netdev-linux.c
> > > @@ -73,6 +73,7 @@
> > >  #include "openvswitch/vlog.h"
> > >  #include "util.h"
> > >  #include "tc.h"
> > > +#include "netdev-tc-offloads.h"
> > > 
> > >  VLOG_DEFINE_THIS_MODULE(netdev_linux);
> > > 
> > > @@ -2783,7 +2784,8 @@ netdev_linux_update_flags(struct netdev *netdev_, enum netdev_flags off,
> > >  }
> > > 
> > >  #define NETDEV_LINUX_CLASS(NAME, CONSTRUCT, GET_STATS,          \
> > > -                           GET_FEATURES, GET_STATUS)            \
> > > +                           GET_FEATURES, GET_STATUS,            \
> > > +                           FLOW_OFFLOAD_API)                    \
> > >  {                                                               \
> > >      NAME,                                                       \
> > >      false,                      /* is_pmd */                    \
> > > @@ -2852,6 +2854,8 @@ netdev_linux_update_flags(struct netdev *netdev_, enum netdev_flags off,
> > >      netdev_linux_rxq_recv,                                      \
> > >      netdev_linux_rxq_wait,                                      \
> > >      netdev_linux_rxq_drain,                                     \
> > > +                                                                \
> > > +    FLOW_OFFLOAD_API                                            \
> > >  }
> > > 
> > >  const struct netdev_class netdev_linux_class =
> > > @@ -2860,7 +2864,8 @@ const struct netdev_class netdev_linux_class =
> > >          netdev_linux_construct,
> > >          netdev_linux_get_stats,
> > >          netdev_linux_get_features,
> > > -        netdev_linux_get_status);
> > > +        netdev_linux_get_status,
> > > +        LINUX_FLOW_OFFLOAD_API);
> > > 
> > >  const struct netdev_class netdev_tap_class =
> > >      NETDEV_LINUX_CLASS(
> > > @@ -2868,7 +2873,8 @@ const struct netdev_class netdev_tap_class =
> > >          netdev_linux_construct_tap,
> > >          netdev_tap_get_stats,
> > >          netdev_linux_get_features,
> > > -        netdev_linux_get_status);
> > > +        netdev_linux_get_status,
> > > +        NO_OFFLOAD_API);
> > > 
> > >  const struct netdev_class netdev_internal_class =
> > >      NETDEV_LINUX_CLASS(
> > > @@ -2876,7 +2882,8 @@ const struct netdev_class netdev_internal_class =
> > >          netdev_linux_construct,
> > >          netdev_internal_get_stats,
> > >          NULL,                  /* get_features */
> > > -        netdev_internal_get_status);
> > > +        netdev_internal_get_status,
> > > +        NO_OFFLOAD_API);
> > >  
> > > 
> > >  #define CODEL_N_QUEUES 0x0000
> > > diff --git a/lib/netdev-linux.h b/lib/netdev-linux.h
> > > index 0c61bc9..d944691 100644
> > > --- a/lib/netdev-linux.h
> > > +++ b/lib/netdev-linux.h
> > > @@ -28,4 +28,13 @@ struct netdev;
> > >  int netdev_linux_ethtool_set_flag(struct netdev *netdev, uint32_t flag,
> > >                                    const char *flag_name, bool enable);
> > > 
> > > +#define LINUX_FLOW_OFFLOAD_API                                  \
> > > +            netdev_tc_flow_flush,                               \
> > > +            netdev_tc_flow_dump_create,                         \
> > > +            netdev_tc_flow_dump_destroy,                        \
> > > +            netdev_tc_flow_dump_next,                           \
> > > +            netdev_tc_flow_put,                                 \
> > > +            netdev_tc_flow_get,                                 \
> > > +            netdev_tc_flow_del,                                 \
> > > +            netdev_tc_init_flow_api
> > >  #endif /* netdev-linux.h */
> > > diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
> > > index 8346fc4..e8bfdc8 100644
> > > --- a/lib/netdev-provider.h
> > > +++ b/lib/netdev-provider.h
> > > @@ -115,6 +115,14 @@ struct netdev_rxq {
> > > 
> > >  struct netdev *netdev_rxq_get_netdev(const struct netdev_rxq *);
> > > 
> > > +
> > > +struct netdev_flow_dump {
> > > +    struct netdev *netdev;
> > > +    odp_port_t port;
> > > +    struct nl_dump *nl_dump;
> > > +    bool terse;
> > > +};
> > > +
> > >  /* Network device class structure, to be defined by each implementation of a
> > >   * network device.
> > >   *
> > > @@ -769,6 +777,67 @@ struct netdev_class {
> > > 
> > >      /* Discards all packets waiting to be received from 'rx'. */
> > >      int (*rxq_drain)(struct netdev_rxq *rx);
> > > +
> > > +    /* ## -------------------------------- ## */
> > > +    /* ## netdev flow offloading functions ## */
> > > +    /* ## -------------------------------- ## */
> > > +
> > > +    /* If a particular netdev class does not support offloading flows,
> > > +     * all these function pointers must be NULL. */
> > > +
> > > +    /* Flush all offloaded flows from a netdev.
> > > +     * Return 0 if successful, otherwise returns a positive errno value. */
> > > +    int (*flow_flush)(struct netdev *);
> > > +
> > > +    /* Flow dumping interface.
> > > +     *
> > > +     * This is the back-end for the flow dumping interface described in
> > > +     * dpif.h.  Please read the comments there first, because this code
> > > +     * closely follows it.
> > > +     *
> > > +     * 'flow_dump_create' is being executed in a dpif thread so there is
> > > +     * no need for 'flow_dump_thread_create' implementation.
> > > +     * On success returns allocated netdev_flow_dump data, on failure returns
> > > +     * positive errno. */
> > > +    int (*flow_dump_create)(struct netdev *, struct netdev_flow_dump **dump);
> > > +    int (*flow_dump_destroy)(struct netdev_flow_dump *);
> > > +
> > > +    /* Returns true while there are more flows to dump.
> > > +     * rbuffer is used as a temporary buffer and needs to be pre allocated
> > > +     * by the caller. while there are more flows the same rbuffer should
> > > +     * be provided. wbuffer is used to store dumped actions and needs to be
> > > +     * pre allocated by the caller. */
> > > +    bool (*flow_dump_next)(struct netdev_flow_dump *, struct match *,
> > > +                           struct nlattr **actions,
> > > +                           struct dpif_flow_stats *stats, ovs_u128 *ufid,
> > > +                           struct ofpbuf *rbuffer, struct ofpbuf *wbuffer);
> > > +
> > > +    /* Offload the given flow on netdev.
> > > +     * To modify a flow, use the same ufid.
> > > +     * actions are in netlink format, as with struct dpif_flow_put.
> > > +     * info is extra info needed to offload the flow.
> > > +     * Read the comments on 'struct dpif_flow_put' in dpif.h about stats.
> > > +     * Return 0 if successful, otherwise returns a positive errno value. */
> > > +    int (*flow_put)(struct netdev *, struct match *, struct nlattr *actions,
> > > +                    size_t actions_len, const ovs_u128 *ufid,
> > > +                    struct offload_info *info, struct dpif_flow_stats *);
> > > +
> > > +    /* Queries a flow specified by ufid on netdev.
> > > +     * Fills output buffer as wbuffer in flow_dump_next.
> > > +     * the buffer needs to be pre allocated.
> > > +     * Return 0 if successful, otherwise returns a positive errno value. */
> > > +    int (*flow_get)(struct netdev *, struct match *, struct nlattr **actions,
> > > +                    const ovs_u128 *ufid, struct dpif_flow_stats *,
> > > +                    struct ofpbuf *wbuffer);
> > > +
> > > +    /* Delete a flow specified by ufid from netdev.
> > > +     * Read the comments on 'struct dpif_flow_del' in dpif.h about stats.
> > > +     * Return 0 if successful, otherwise returns a positive errno value. */
> > > +    int (*flow_del)(struct netdev *, const ovs_u128 *ufid,
> > > +                    struct dpif_flow_stats *);
> > > +
> > > +    /* Initializies the netdev flow api. */
> > > +    int (*init_flow_api)(struct netdev *);
> > >  };
> > > 
> > >  int netdev_register_provider(const struct netdev_class *);
> > > @@ -788,4 +857,6 @@ extern const struct netdev_class netdev_tap_class;
> > >  }
> > >  #endif
> > > 
> > > +#define NO_OFFLOAD_API NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL
> > > +
> > >  #endif /* netdev.h */
> > > diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> > > new file mode 100644
> > > index 0000000..eb5e79a
> > > --- /dev/null
> > > +++ b/lib/netdev-tc-offloads.c
> > > @@ -0,0 +1,154 @@
> > > +/*
> > > + * Copyright (c) 2016 Mellanox Technologies, Ltd.
> > > + *
> > > + * Licensed under the Apache License, Version 2.0 (the "License");
> > > + * you may not use this file except in compliance with the License.
> > > + * You may obtain a copy of the License at:
> > > + *
> > > + *     http://www.apache.org/licenses/LICENSE-2.0
> > > + *
> > > + * Unless required by applicable law or agreed to in writing, software
> > > + * distributed under the License is distributed on an "AS IS" BASIS,
> > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > > + * See the License for the specific language governing permissions and
> > > + * limitations under the License.
> > > + */
> > > +
> > > +#include <config.h>
> > > +
> > > +#include "netdev-tc-offloads.h"
> > > +
> > > +#include <errno.h>
> > > +#include <fcntl.h>
> > > +#include <arpa/inet.h>
> > > +#include <inttypes.h>
> > > +#include <linux/filter.h>
> > > +#include <linux/gen_stats.h>
> > > +#include <linux/if_ether.h>
> > > +#include <linux/if_tun.h>
> > > +#include <linux/types.h>
> > > +#include <linux/ethtool.h>
> > > +#include <linux/mii.h>
> > > +#include <linux/pkt_cls.h>
> > > +#include <linux/pkt_sched.h>
> > > +#include <linux/rtnetlink.h>
> > > +#include <linux/sockios.h>
> > > +#include <sys/types.h>
> > > +#include <sys/ioctl.h>
> > > +#include <sys/socket.h>
> > > +#include <sys/utsname.h>
> > > +#include <netpacket/packet.h>
> > > +#include <net/if.h>
> > > +#include <net/if_arp.h>
> > > +#include <net/if_packet.h>
> > > +#include <net/route.h>
> > > +#include <netinet/in.h>
> > > +#include <poll.h>
> > > +#include <stdlib.h>
> > > +#include <string.h>
> > > +#include <unistd.h>
> > > +
> > > +#include "coverage.h"
> > > +#include "dp-packet.h"
> > > +#include "dpif-netlink.h"
> > > +#include "dpif-netdev.h"
> > > +#include "openvswitch/dynamic-string.h"
> > > +#include "fatal-signal.h"
> > > +#include "hash.h"
> > > +#include "openvswitch/hmap.h"
> > > +#include "netdev-provider.h"
> > > +#include "netdev-vport.h"
> > > +#include "netlink-notifier.h"
> > > +#include "netlink-socket.h"
> > > +#include "netlink.h"
> > > +#include "openvswitch/ofpbuf.h"
> > > +#include "openflow/openflow.h"
> > > +#include "ovs-atomic.h"
> > > +#include "packets.h"
> > > +#include "poll-loop.h"
> > > +#include "rtnetlink.h"
> > > +#include "openvswitch/shash.h"
> > > +#include "netdev-provider.h"
> > > +#include "openvswitch/match.h"
> > > +#include "openvswitch/vlog.h"
> > > +#include "tc.h"
> > > +
> > > +VLOG_DEFINE_THIS_MODULE(netdev_tc_offloads);
> > > +
> > > +int
> > > +netdev_tc_flow_flush(struct netdev *netdev OVS_UNUSED)
> > > +{
> > > +    return EOPNOTSUPP;
> > > +}
> > > +
> > > +int
> > > +netdev_tc_flow_dump_create(struct netdev *netdev,
> > > +                           struct netdev_flow_dump **dump_out)
> > > +{
> > > +    struct netdev_flow_dump *dump = xzalloc(sizeof *dump);
> > > +
> > > +    dump->netdev = netdev_ref(netdev);
> > > +
> > > +    *dump_out = dump;
> > > +
> > > +    return 0;
> > > +}
> > > +
> > > +int
> > > +netdev_tc_flow_dump_destroy(struct netdev_flow_dump *dump)
> > > +{
> > > +    netdev_close(dump->netdev);
> > > +    free(dump);
> > > +
> > > +    return 0;
> > > +}
> > > +
> > > +bool
> > > +netdev_tc_flow_dump_next(struct netdev_flow_dump *dump OVS_UNUSED,
> > > +                         struct match *match OVS_UNUSED,
> > > +                         struct nlattr **actions OVS_UNUSED,
> > > +                         struct dpif_flow_stats *stats OVS_UNUSED,
> > > +                         ovs_u128 *ufid OVS_UNUSED,
> > > +                         struct ofpbuf *rbuffer OVS_UNUSED,
> > > +                         struct ofpbuf *wbuffer OVS_UNUSED)
> > > +{
> > > +    return false;
> > > +}
> > > +
> > > +int
> > > +netdev_tc_flow_put(struct netdev *netdev OVS_UNUSED,
> > > +                   struct match *match OVS_UNUSED,
> > > +                   struct nlattr *actions OVS_UNUSED,
> > > +                   size_t actions_len OVS_UNUSED,
> > > +                   struct dpif_flow_stats *stats OVS_UNUSED,
> > > +                   const ovs_u128 *ufid OVS_UNUSED,
> > > +                   struct offload_info *info OVS_UNUSED)
> > > +{
> > > +    return EOPNOTSUPP;
> > > +}
> > > +
> > > +int
> > > +netdev_tc_flow_get(struct netdev *netdev OVS_UNUSED,
> > > +                   struct match *match OVS_UNUSED,
> > > +                   struct nlattr **actions OVS_UNUSED,
> > > +                   const ovs_u128 *ufid OVS_UNUSED,
> > > +                   struct dpif_flow_stats *stats OVS_UNUSED,
> > > +                   struct ofpbuf *buf OVS_UNUSED)
> > > +{
> > > +    return EOPNOTSUPP;
> > > +}
> > > +
> > > +int
> > > +netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
> > > +                   const ovs_u128 *ufid OVS_UNUSED,
> > > +                   struct dpif_flow_stats *stats OVS_UNUSED)
> > > +{
> > > +    return EOPNOTSUPP;
> > > +}
> > > +
> > > +int
> > > +netdev_tc_init_flow_api(struct netdev *netdev OVS_UNUSED)
> > > +{
> > > +    return 0;
> > > +}
> > > +
> > > diff --git a/lib/netdev-tc-offloads.h b/lib/netdev-tc-offloads.h
> > > new file mode 100644
> > > index 0000000..76b7938
> > > --- /dev/null
> > > +++ b/lib/netdev-tc-offloads.h
> > > @@ -0,0 +1,42 @@
> > > +/*
> > > + * Copyright (c) 2016 Mellanox Technologies, Ltd.
> > > + *
> > > + * Licensed under the Apache License, Version 2.0 (the "License");
> > > + * you may not use this file except in compliance with the License.
> > > + * You may obtain a copy of the License at:
> > > + *
> > > + *     http://www.apache.org/licenses/LICENSE-2.0
> > > + *
> > > + * Unless required by applicable law or agreed to in writing, software
> > > + * distributed under the License is distributed on an "AS IS" BASIS,
> > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > > + * See the License for the specific language governing permissions and
> > > + * limitations under the License.
> > > + */
> > > +
> > > +#ifndef NETDEV_TC_OFFLOADS_H
> > > +#define NETDEV_TC_OFFLOADS_H 1
> > > +
> > > +#include "netdev.h"
> > > +
> > > +int netdev_tc_flow_flush(struct netdev *);
> > > +int netdev_tc_flow_dump_create(struct netdev *, struct netdev_flow_dump **);
> > > +int netdev_tc_flow_dump_destroy(struct netdev_flow_dump *);
> > > +bool netdev_tc_flow_dump_next(struct netdev_flow_dump *, struct match *,
> > > +                              struct nlattr **actions,
> > > +                              struct dpif_flow_stats *,
> > > +                              ovs_u128 *ufid,
> > > +                              struct ofpbuf *rbuffer,
> > > +                              struct ofpbuf *wbuffer);
> > > +int netdev_tc_flow_put(struct netdev *, struct match *,
> > > +                       struct nlattr *actions, size_t actions_len,
> > > +                       const ovs_u128 *, struct offload_info *,
> > > +                       struct dpif_flow_stats *);
> > > +int netdev_tc_flow_get(struct netdev *, struct match *,
> > > +                       struct nlattr **actions, const ovs_u128 *,
> > > +                       struct dpif_flow_stats *, struct ofpbuf *);
> > > +int netdev_tc_flow_del(struct netdev *, const ovs_u128 *,
> > > +                        struct dpif_flow_stats *);
> > > +int netdev_tc_init_flow_api(struct netdev *);
> > > +
> > > +#endif /* netdev-tc-offloads.h */
> > > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> > > index 39093e8..2cad5cb 100644
> > > --- a/lib/netdev-vport.c
> > > +++ b/lib/netdev-vport.c
> > > @@ -847,7 +847,16 @@ get_stats(const struct netdev *netdev, struct netdev_stats *stats)
> > >      NULL,                   /* rx_dealloc */                \
> > >      NULL,                   /* rx_recv */                   \
> > >      NULL,                   /* rx_wait */                   \
> > > -    NULL,                   /* rx_drain */
> > > +    NULL,                   /* rx_drain */                  \
> > > +                                                            \
> > > +    NULL,                   /* flow_flush */                \
> > > +    NULL,                   /* flow_dump_create */          \
> > > +    NULL,                   /* flow_dump_destroy */         \
> > > +    NULL,                   /* flow_dump_next */            \
> > > +    NULL,                   /* flow_put */                  \
> > > +    NULL,                   /* flow_get */                  \
> > > +    NULL,                   /* flow_del */                  \
> > > +    NULL,                   /* init_flow_api */
> > > 
> > > 
> > >  #define TUNNEL_CLASS(NAME, DPIF_PORT, BUILD_HEADER, PUSH_HEADER, POP_HEADER)   \
> > > diff --git a/lib/netdev.c b/lib/netdev.c
> > > index a8d8eda..1677027 100644
> > > --- a/lib/netdev.c
> > > +++ b/lib/netdev.c
> > > @@ -1998,3 +1998,101 @@ netdev_reconfigure(struct netdev *netdev)
> > >              ? class->reconfigure(netdev)
> > >              : EOPNOTSUPP);
> > >  }
> > > +
> > > +int
> > > +netdev_flow_flush(struct netdev *netdev)
> > > +{
> > > +    const struct netdev_class *class = netdev->netdev_class;
> > > +
> > > +    return (class->flow_flush
> > > +            ? class->flow_flush(netdev)
> > > +            : EOPNOTSUPP);
> > > +}
> > > +
> > > +int
> > > +netdev_flow_dump_create(struct netdev *netdev, struct netdev_flow_dump **dump)
> > > +{
> > > +    const struct netdev_class *class = netdev->netdev_class;
> > > +
> > > +    if (class->flow_dump_create) {
> > > +        return class->flow_dump_create(netdev, dump);
> > > +    }
> > > +
> > > +    return EOPNOTSUPP;
> > > +}
> > > +
> > > +int
> > > +netdev_flow_dump_destroy(struct netdev_flow_dump *dump)
> > > +{
> > > +    const struct netdev_class *class = dump->netdev->netdev_class;
> > > +
> > > +    if (class->flow_dump_destroy) {
> > > +        return class->flow_dump_destroy(dump);
> > > +    }
> > > +
> > > +    return EOPNOTSUPP;
> > > +}
> > > +
> > > +bool
> > > +netdev_flow_dump_next(struct netdev_flow_dump *dump,
> > > +                      struct match *match,
> > > +                      struct nlattr **actions,
> > > +                      struct dpif_flow_stats *stats,
> > > +                      ovs_u128 *ufid,
> > > +                      struct ofpbuf *rbuffer,
> > > +                      struct ofpbuf *wbuffer)
> > > +{
> > > +    const struct netdev_class *class = dump->netdev->netdev_class;
> > > +
> > > +    return (class->flow_dump_next
> > > +            ? class->flow_dump_next(dump, match, actions, stats, ufid,
> > > +                                    rbuffer, wbuffer)
> > > +            : false);
> > 
> > Sorry to nitpick, but some functions are using the format above and
> > others the format below (perhaps less dense):
> > [...]
> > 	if () {
> > 	   return class->function()
> > 	}
> > 
> > 	return default_ret;
> > 
> > It would be great if they all followed the same style.
> 
> ok
> 
> > 
> > Also, some function declarations have one parameter per line when it
> > could have been at least two per line (less than 79 chars).
> > 
> > fbl
> 
> usually I think it's more readable to have 1 parameter per line when we
> already have to break to more than a single line.
> Should we always squeeze args to a single line when possible?

It helps when the code follows a pattern and although I haven't done
any stats, it seems OVS uses more than one parameter per line.
diff mbox

Patch

diff --git a/lib/automake.mk b/lib/automake.mk
index 3d57610..a7c8009 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -354,6 +354,8 @@  lib_libopenvswitch_la_SOURCES += \
 	lib/dpif-netlink.h \
 	lib/tc.h \
 	lib/tc.c \
+	lib/netdev-tc-offloads.h \
+	lib/netdev-tc-offloads.c \
 	lib/if-notifier.c \
 	lib/if-notifier.h \
 	lib/netdev-linux.c \
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 94c515d..5b54d79 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -1547,6 +1547,8 @@  netdev_bsd_update_flags(struct netdev *netdev_, enum netdev_flags off,
     netdev_bsd_rxq_recv,                             \
     netdev_bsd_rxq_wait,                             \
     netdev_bsd_rxq_drain,                            \
+                                                     \
+    NO_OFFLOAD_API                                   \
 }
 
 const struct netdev_class netdev_bsd_class =
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ddc651b..9ad96cd 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -3314,6 +3314,7 @@  unlock:
     RXQ_RECV,                                                 \
     NULL,                       /* rx_wait */                 \
     NULL,                       /* rxq_drain */               \
+    NO_OFFLOAD_API                                            \
 }
 
 static const struct netdev_class dpdk_class =
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 0657434..7e1383f 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -1409,6 +1409,8 @@  netdev_dummy_update_flags(struct netdev *netdev_,
     netdev_dummy_rxq_recv,                                      \
     netdev_dummy_rxq_wait,                                      \
     netdev_dummy_rxq_drain,                                     \
+                                                                \
+    NO_OFFLOAD_API                                              \
 }
 
 static const struct netdev_class dummy_class =
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index a6bb515..f7941fd 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -73,6 +73,7 @@ 
 #include "openvswitch/vlog.h"
 #include "util.h"
 #include "tc.h"
+#include "netdev-tc-offloads.h"
 
 VLOG_DEFINE_THIS_MODULE(netdev_linux);
 
@@ -2783,7 +2784,8 @@  netdev_linux_update_flags(struct netdev *netdev_, enum netdev_flags off,
 }
 
 #define NETDEV_LINUX_CLASS(NAME, CONSTRUCT, GET_STATS,          \
-                           GET_FEATURES, GET_STATUS)            \
+                           GET_FEATURES, GET_STATUS,            \
+                           FLOW_OFFLOAD_API)                    \
 {                                                               \
     NAME,                                                       \
     false,                      /* is_pmd */                    \
@@ -2852,6 +2854,8 @@  netdev_linux_update_flags(struct netdev *netdev_, enum netdev_flags off,
     netdev_linux_rxq_recv,                                      \
     netdev_linux_rxq_wait,                                      \
     netdev_linux_rxq_drain,                                     \
+                                                                \
+    FLOW_OFFLOAD_API                                            \
 }
 
 const struct netdev_class netdev_linux_class =
@@ -2860,7 +2864,8 @@  const struct netdev_class netdev_linux_class =
         netdev_linux_construct,
         netdev_linux_get_stats,
         netdev_linux_get_features,
-        netdev_linux_get_status);
+        netdev_linux_get_status,
+        LINUX_FLOW_OFFLOAD_API);
 
 const struct netdev_class netdev_tap_class =
     NETDEV_LINUX_CLASS(
@@ -2868,7 +2873,8 @@  const struct netdev_class netdev_tap_class =
         netdev_linux_construct_tap,
         netdev_tap_get_stats,
         netdev_linux_get_features,
-        netdev_linux_get_status);
+        netdev_linux_get_status,
+        NO_OFFLOAD_API);
 
 const struct netdev_class netdev_internal_class =
     NETDEV_LINUX_CLASS(
@@ -2876,7 +2882,8 @@  const struct netdev_class netdev_internal_class =
         netdev_linux_construct,
         netdev_internal_get_stats,
         NULL,                  /* get_features */
-        netdev_internal_get_status);
+        netdev_internal_get_status,
+        NO_OFFLOAD_API);
 
 
 #define CODEL_N_QUEUES 0x0000
diff --git a/lib/netdev-linux.h b/lib/netdev-linux.h
index 0c61bc9..d944691 100644
--- a/lib/netdev-linux.h
+++ b/lib/netdev-linux.h
@@ -28,4 +28,13 @@  struct netdev;
 int netdev_linux_ethtool_set_flag(struct netdev *netdev, uint32_t flag,
                                   const char *flag_name, bool enable);
 
+#define LINUX_FLOW_OFFLOAD_API                                  \
+            netdev_tc_flow_flush,                               \
+            netdev_tc_flow_dump_create,                         \
+            netdev_tc_flow_dump_destroy,                        \
+            netdev_tc_flow_dump_next,                           \
+            netdev_tc_flow_put,                                 \
+            netdev_tc_flow_get,                                 \
+            netdev_tc_flow_del,                                 \
+            netdev_tc_init_flow_api
 #endif /* netdev-linux.h */
diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
index 8346fc4..e8bfdc8 100644
--- a/lib/netdev-provider.h
+++ b/lib/netdev-provider.h
@@ -115,6 +115,14 @@  struct netdev_rxq {
 
 struct netdev *netdev_rxq_get_netdev(const struct netdev_rxq *);
 
+
+struct netdev_flow_dump {
+    struct netdev *netdev;
+    odp_port_t port;
+    struct nl_dump *nl_dump;
+    bool terse;
+};
+
 /* Network device class structure, to be defined by each implementation of a
  * network device.
  *
@@ -769,6 +777,67 @@  struct netdev_class {
 
     /* Discards all packets waiting to be received from 'rx'. */
     int (*rxq_drain)(struct netdev_rxq *rx);
+
+    /* ## -------------------------------- ## */
+    /* ## netdev flow offloading functions ## */
+    /* ## -------------------------------- ## */
+
+    /* If a particular netdev class does not support offloading flows,
+     * all these function pointers must be NULL. */
+
+    /* Flush all offloaded flows from a netdev.
+     * Return 0 if successful, otherwise returns a positive errno value. */
+    int (*flow_flush)(struct netdev *);
+
+    /* Flow dumping interface.
+     *
+     * This is the back-end for the flow dumping interface described in
+     * dpif.h.  Please read the comments there first, because this code
+     * closely follows it.
+     *
+     * 'flow_dump_create' is being executed in a dpif thread so there is
+     * no need for 'flow_dump_thread_create' implementation.
+     * On success returns allocated netdev_flow_dump data, on failure returns
+     * positive errno. */
+    int (*flow_dump_create)(struct netdev *, struct netdev_flow_dump **dump);
+    int (*flow_dump_destroy)(struct netdev_flow_dump *);
+
+    /* Returns true while there are more flows to dump.
+     * rbuffer is used as a temporary buffer and needs to be pre allocated
+     * by the caller. while there are more flows the same rbuffer should
+     * be provided. wbuffer is used to store dumped actions and needs to be
+     * pre allocated by the caller. */
+    bool (*flow_dump_next)(struct netdev_flow_dump *, struct match *,
+                           struct nlattr **actions,
+                           struct dpif_flow_stats *stats, ovs_u128 *ufid,
+                           struct ofpbuf *rbuffer, struct ofpbuf *wbuffer);
+
+    /* Offload the given flow on netdev.
+     * To modify a flow, use the same ufid.
+     * actions are in netlink format, as with struct dpif_flow_put.
+     * info is extra info needed to offload the flow.
+     * Read the comments on 'struct dpif_flow_put' in dpif.h about stats.
+     * Return 0 if successful, otherwise returns a positive errno value. */
+    int (*flow_put)(struct netdev *, struct match *, struct nlattr *actions,
+                    size_t actions_len, const ovs_u128 *ufid,
+                    struct offload_info *info, struct dpif_flow_stats *);
+
+    /* Queries a flow specified by ufid on netdev.
+     * Fills output buffer as wbuffer in flow_dump_next.
+     * the buffer needs to be pre allocated.
+     * Return 0 if successful, otherwise returns a positive errno value. */
+    int (*flow_get)(struct netdev *, struct match *, struct nlattr **actions,
+                    const ovs_u128 *ufid, struct dpif_flow_stats *,
+                    struct ofpbuf *wbuffer);
+
+    /* Delete a flow specified by ufid from netdev.
+     * Read the comments on 'struct dpif_flow_del' in dpif.h about stats.
+     * Return 0 if successful, otherwise returns a positive errno value. */
+    int (*flow_del)(struct netdev *, const ovs_u128 *ufid,
+                    struct dpif_flow_stats *);
+
+    /* Initializies the netdev flow api. */
+    int (*init_flow_api)(struct netdev *);
 };
 
 int netdev_register_provider(const struct netdev_class *);
@@ -788,4 +857,6 @@  extern const struct netdev_class netdev_tap_class;
 }
 #endif
 
+#define NO_OFFLOAD_API NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL
+
 #endif /* netdev.h */
diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
new file mode 100644
index 0000000..eb5e79a
--- /dev/null
+++ b/lib/netdev-tc-offloads.c
@@ -0,0 +1,154 @@ 
+/*
+ * Copyright (c) 2016 Mellanox Technologies, Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <config.h>
+
+#include "netdev-tc-offloads.h"
+
+#include <errno.h>
+#include <fcntl.h>
+#include <arpa/inet.h>
+#include <inttypes.h>
+#include <linux/filter.h>
+#include <linux/gen_stats.h>
+#include <linux/if_ether.h>
+#include <linux/if_tun.h>
+#include <linux/types.h>
+#include <linux/ethtool.h>
+#include <linux/mii.h>
+#include <linux/pkt_cls.h>
+#include <linux/pkt_sched.h>
+#include <linux/rtnetlink.h>
+#include <linux/sockios.h>
+#include <sys/types.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <sys/utsname.h>
+#include <netpacket/packet.h>
+#include <net/if.h>
+#include <net/if_arp.h>
+#include <net/if_packet.h>
+#include <net/route.h>
+#include <netinet/in.h>
+#include <poll.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "coverage.h"
+#include "dp-packet.h"
+#include "dpif-netlink.h"
+#include "dpif-netdev.h"
+#include "openvswitch/dynamic-string.h"
+#include "fatal-signal.h"
+#include "hash.h"
+#include "openvswitch/hmap.h"
+#include "netdev-provider.h"
+#include "netdev-vport.h"
+#include "netlink-notifier.h"
+#include "netlink-socket.h"
+#include "netlink.h"
+#include "openvswitch/ofpbuf.h"
+#include "openflow/openflow.h"
+#include "ovs-atomic.h"
+#include "packets.h"
+#include "poll-loop.h"
+#include "rtnetlink.h"
+#include "openvswitch/shash.h"
+#include "netdev-provider.h"
+#include "openvswitch/match.h"
+#include "openvswitch/vlog.h"
+#include "tc.h"
+
+VLOG_DEFINE_THIS_MODULE(netdev_tc_offloads);
+
+int
+netdev_tc_flow_flush(struct netdev *netdev OVS_UNUSED)
+{
+    return EOPNOTSUPP;
+}
+
+int
+netdev_tc_flow_dump_create(struct netdev *netdev,
+                           struct netdev_flow_dump **dump_out)
+{
+    struct netdev_flow_dump *dump = xzalloc(sizeof *dump);
+
+    dump->netdev = netdev_ref(netdev);
+
+    *dump_out = dump;
+
+    return 0;
+}
+
+int
+netdev_tc_flow_dump_destroy(struct netdev_flow_dump *dump)
+{
+    netdev_close(dump->netdev);
+    free(dump);
+
+    return 0;
+}
+
+bool
+netdev_tc_flow_dump_next(struct netdev_flow_dump *dump OVS_UNUSED,
+                         struct match *match OVS_UNUSED,
+                         struct nlattr **actions OVS_UNUSED,
+                         struct dpif_flow_stats *stats OVS_UNUSED,
+                         ovs_u128 *ufid OVS_UNUSED,
+                         struct ofpbuf *rbuffer OVS_UNUSED,
+                         struct ofpbuf *wbuffer OVS_UNUSED)
+{
+    return false;
+}
+
+int
+netdev_tc_flow_put(struct netdev *netdev OVS_UNUSED,
+                   struct match *match OVS_UNUSED,
+                   struct nlattr *actions OVS_UNUSED,
+                   size_t actions_len OVS_UNUSED,
+                   struct dpif_flow_stats *stats OVS_UNUSED,
+                   const ovs_u128 *ufid OVS_UNUSED,
+                   struct offload_info *info OVS_UNUSED)
+{
+    return EOPNOTSUPP;
+}
+
+int
+netdev_tc_flow_get(struct netdev *netdev OVS_UNUSED,
+                   struct match *match OVS_UNUSED,
+                   struct nlattr **actions OVS_UNUSED,
+                   const ovs_u128 *ufid OVS_UNUSED,
+                   struct dpif_flow_stats *stats OVS_UNUSED,
+                   struct ofpbuf *buf OVS_UNUSED)
+{
+    return EOPNOTSUPP;
+}
+
+int
+netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
+                   const ovs_u128 *ufid OVS_UNUSED,
+                   struct dpif_flow_stats *stats OVS_UNUSED)
+{
+    return EOPNOTSUPP;
+}
+
+int
+netdev_tc_init_flow_api(struct netdev *netdev OVS_UNUSED)
+{
+    return 0;
+}
+
diff --git a/lib/netdev-tc-offloads.h b/lib/netdev-tc-offloads.h
new file mode 100644
index 0000000..76b7938
--- /dev/null
+++ b/lib/netdev-tc-offloads.h
@@ -0,0 +1,42 @@ 
+/*
+ * Copyright (c) 2016 Mellanox Technologies, Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef NETDEV_TC_OFFLOADS_H
+#define NETDEV_TC_OFFLOADS_H 1
+
+#include "netdev.h"
+
+int netdev_tc_flow_flush(struct netdev *);
+int netdev_tc_flow_dump_create(struct netdev *, struct netdev_flow_dump **);
+int netdev_tc_flow_dump_destroy(struct netdev_flow_dump *);
+bool netdev_tc_flow_dump_next(struct netdev_flow_dump *, struct match *,
+                              struct nlattr **actions,
+                              struct dpif_flow_stats *,
+                              ovs_u128 *ufid,
+                              struct ofpbuf *rbuffer,
+                              struct ofpbuf *wbuffer);
+int netdev_tc_flow_put(struct netdev *, struct match *,
+                       struct nlattr *actions, size_t actions_len,
+                       const ovs_u128 *, struct offload_info *,
+                       struct dpif_flow_stats *);
+int netdev_tc_flow_get(struct netdev *, struct match *,
+                       struct nlattr **actions, const ovs_u128 *,
+                       struct dpif_flow_stats *, struct ofpbuf *);
+int netdev_tc_flow_del(struct netdev *, const ovs_u128 *,
+                        struct dpif_flow_stats *);
+int netdev_tc_init_flow_api(struct netdev *);
+
+#endif /* netdev-tc-offloads.h */
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 39093e8..2cad5cb 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -847,7 +847,16 @@  get_stats(const struct netdev *netdev, struct netdev_stats *stats)
     NULL,                   /* rx_dealloc */                \
     NULL,                   /* rx_recv */                   \
     NULL,                   /* rx_wait */                   \
-    NULL,                   /* rx_drain */
+    NULL,                   /* rx_drain */                  \
+                                                            \
+    NULL,                   /* flow_flush */                \
+    NULL,                   /* flow_dump_create */          \
+    NULL,                   /* flow_dump_destroy */         \
+    NULL,                   /* flow_dump_next */            \
+    NULL,                   /* flow_put */                  \
+    NULL,                   /* flow_get */                  \
+    NULL,                   /* flow_del */                  \
+    NULL,                   /* init_flow_api */
 
 
 #define TUNNEL_CLASS(NAME, DPIF_PORT, BUILD_HEADER, PUSH_HEADER, POP_HEADER)   \
diff --git a/lib/netdev.c b/lib/netdev.c
index a8d8eda..1677027 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -1998,3 +1998,101 @@  netdev_reconfigure(struct netdev *netdev)
             ? class->reconfigure(netdev)
             : EOPNOTSUPP);
 }
+
+int
+netdev_flow_flush(struct netdev *netdev)
+{
+    const struct netdev_class *class = netdev->netdev_class;
+
+    return (class->flow_flush
+            ? class->flow_flush(netdev)
+            : EOPNOTSUPP);
+}
+
+int
+netdev_flow_dump_create(struct netdev *netdev, struct netdev_flow_dump **dump)
+{
+    const struct netdev_class *class = netdev->netdev_class;
+
+    if (class->flow_dump_create) {
+        return class->flow_dump_create(netdev, dump);
+    }
+
+    return EOPNOTSUPP;
+}
+
+int
+netdev_flow_dump_destroy(struct netdev_flow_dump *dump)
+{
+    const struct netdev_class *class = dump->netdev->netdev_class;
+
+    if (class->flow_dump_destroy) {
+        return class->flow_dump_destroy(dump);
+    }
+
+    return EOPNOTSUPP;
+}
+
+bool
+netdev_flow_dump_next(struct netdev_flow_dump *dump,
+                      struct match *match,
+                      struct nlattr **actions,
+                      struct dpif_flow_stats *stats,
+                      ovs_u128 *ufid,
+                      struct ofpbuf *rbuffer,
+                      struct ofpbuf *wbuffer)
+{
+    const struct netdev_class *class = dump->netdev->netdev_class;
+
+    return (class->flow_dump_next
+            ? class->flow_dump_next(dump, match, actions, stats, ufid,
+                                    rbuffer, wbuffer)
+            : false);
+}
+
+int
+netdev_flow_put(struct netdev *netdev, struct match *match,
+                struct nlattr *actions, size_t act_len,
+                const ovs_u128 *ufid, struct offload_info *info,
+                struct dpif_flow_stats *stats)
+{
+    const struct netdev_class *class = netdev->netdev_class;
+
+    return (class->flow_put
+            ? class->flow_put(netdev, match, actions, act_len, ufid,
+                              info, stats)
+            : EOPNOTSUPP);
+}
+
+int
+netdev_flow_get(struct netdev *netdev, struct match *match,
+                struct nlattr **actions, const ovs_u128 *ufid,
+                struct dpif_flow_stats *stats, struct ofpbuf *buf)
+{
+    const struct netdev_class *class = netdev->netdev_class;
+
+    return (class->flow_get
+            ? class->flow_get(netdev, match, actions, ufid, stats, buf)
+            : EOPNOTSUPP);
+}
+
+int
+netdev_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
+                struct dpif_flow_stats *stats)
+{
+    const struct netdev_class *class = netdev->netdev_class;
+
+    return (class->flow_del
+            ? class->flow_del(netdev, ufid, stats)
+            : EOPNOTSUPP);
+}
+
+int
+netdev_init_flow_api(struct netdev *netdev)
+{
+    const struct netdev_class *class = netdev->netdev_class;
+
+    return (class->init_flow_api
+            ? class->init_flow_api(netdev)
+            : EOPNOTSUPP);
+}
diff --git a/lib/netdev.h b/lib/netdev.h
index d6c07c1..17890e6 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -156,6 +156,29 @@  int netdev_send(struct netdev *, int qid, struct dp_packet_batch *,
                 bool may_steal, bool concurrent_txq);
 void netdev_send_wait(struct netdev *, int qid);
 
+/* Flow offloading. */
+struct offload_info {
+    const void *port_hmap_obj; /* To query ports info from netdev port map */
+    ovs_be16 tp_dst_port; /* Destination port for tunnel in SET action */
+};
+struct netdev_flow_dump;
+int netdev_flow_flush(struct netdev *);
+int netdev_flow_dump_create(struct netdev *, struct netdev_flow_dump **dump);
+int netdev_flow_dump_destroy(struct netdev_flow_dump *);
+bool netdev_flow_dump_next(struct netdev_flow_dump *, struct match *,
+                          struct nlattr **actions, struct dpif_flow_stats *,
+                          ovs_u128 *ufid, struct ofpbuf *rbuffer,
+                          struct ofpbuf *wbuffer);
+int netdev_flow_put(struct netdev *, struct match *, struct nlattr *actions,
+                    size_t actions_len, const ovs_u128 *,
+                    struct offload_info *, struct dpif_flow_stats *);
+int netdev_flow_get(struct netdev *, struct match *, struct nlattr **actions,
+                    const ovs_u128 *, struct dpif_flow_stats *,
+                    struct ofpbuf *wbuffer);
+int netdev_flow_del(struct netdev *, const ovs_u128 *,
+                    struct dpif_flow_stats *);
+int netdev_init_flow_api(struct netdev *);
+
 /* native tunnel APIs */
 /* Structure to pass parameters required to build a tunnel header. */
 struct netdev_tnl_build_header_params {