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 <pa...@mellanox.com> > > > > > > Signed-off-by: Paul Blakey <pa...@mellanox.com> > > > Reviewed-by: Roi Dayan <r...@mellanox.com> > > > Reviewed-by: Simon Horman <simon.hor...@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. -- Flavio _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev