| Message ID | 20170216222533.11027-5-e@erig.me |
|---|---|
| State | Superseded |
| Headers | show |
On 16 February 2017 at 14:25, Eric Garver <e@erig.me> wrote: > Creates VXLAN devices using rtnetlink and tunnel metadata. > > Co-Authored-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> > Signed-off-by: Eric Garver <e@erig.me> > --- > lib/dpif-rtnetlink.c | 181 ++++++++++++++++++++++++++++++++++++++++++++++++++- > lib/dpif-rtnetlink.h | 2 +- > 2 files changed, 181 insertions(+), 2 deletions(-) > > diff --git a/lib/dpif-rtnetlink.c b/lib/dpif-rtnetlink.c > index b309c88d187a..8b6574d1a145 100644 > --- a/lib/dpif-rtnetlink.c > +++ b/lib/dpif-rtnetlink.c > @@ -18,14 +18,192 @@ > > #include "dpif-rtnetlink.h" > > +#include <net/if.h> > +#include <linux/ip.h> > +#include <linux/rtnetlink.h> > + > #include "dpif-netlink.h" > +#include "netdev-vport.h" > +#include "netlink-socket.h" > + > +/* > + * On some older systems, these enums are not defined. > + */ > +#ifndef IFLA_VXLAN_MAX > +#define IFLA_VXLAN_MAX 0 > +#define IFLA_VXLAN_PORT 15 > +#endif > +#if IFLA_VXLAN_MAX < 20 > +#define IFLA_VXLAN_UDP_ZERO_CSUM6_RX 20 > +#define IFLA_VXLAN_GBP 23 > +#define IFLA_VXLAN_COLLECT_METADATA 25 > +#endif > + > +static const struct nl_policy rtlink_policy[] = { > + [IFLA_LINKINFO] = { .type = NL_A_NESTED }, > +}; > +static const struct nl_policy linkinfo_policy[] = { > + [IFLA_INFO_KIND] = { .type = NL_A_STRING }, > + [IFLA_INFO_DATA] = { .type = NL_A_NESTED }, > +}; > + > + > +static int > +dpif_rtnetlink_destroy(const char *name) > +{ > + int err; > + struct ofpbuf request, *reply; > + > + ofpbuf_init(&request, 0); > + nl_msg_put_nlmsghdr(&request, 0, RTM_DELLINK, > + NLM_F_REQUEST | NLM_F_ACK); > + ofpbuf_put_zeros(&request, sizeof(struct ifinfomsg)); > + nl_msg_put_string(&request, IFLA_IFNAME, name); > + > + err = nl_transact(NETLINK_ROUTE, &request, &reply); > + > + if (!err) { > + ofpbuf_uninit(reply); The comment above nl_transact() states that 'reply' should be freed using ofpbuf_delete(). Please check each of the calls to ensure we aren't leaking memory this way. > + } > + > + ofpbuf_uninit(&request); > + return err; > +} > + > +static int > +dpif_rtnetlink_vxlan_destroy(const char *name) > +{ > + return dpif_rtnetlink_destroy(name); > +} > + > +static int > +dpif_rtnetlink_vxlan_verify(struct netdev *netdev, const char *name, > + const char *kind) > +{ > + int err; > + struct ofpbuf request, *reply; > + struct ifinfomsg *ifmsg; > + const struct netdev_tunnel_config *tnl_cfg; > + > + static const struct nl_policy vxlan_policy[] = { > + [IFLA_VXLAN_COLLECT_METADATA] = { .type = NL_A_U8 }, > + [IFLA_VXLAN_LEARNING] = { .type = NL_A_U8 }, > + [IFLA_VXLAN_UDP_ZERO_CSUM6_RX] = { .type = NL_A_U8 }, > + [IFLA_VXLAN_PORT] = { .type = NL_A_U16 }, > + }; > > + tnl_cfg = netdev_get_tunnel_config(netdev); > + if (!tnl_cfg) { > + return EINVAL; > + } > + > + ofpbuf_init(&request, 0); > + nl_msg_put_nlmsghdr(&request, 0, RTM_GETLINK, > + NLM_F_REQUEST); > + ofpbuf_put_zeros(&request, sizeof(struct ifinfomsg)); > + nl_msg_put_string(&request, IFLA_IFNAME, name); > + > + err = nl_transact(NETLINK_ROUTE, &request, &reply); > + if (!err) { > + struct nlattr *rtlink[ARRAY_SIZE(rtlink_policy)]; > + struct nlattr *linkinfo[ARRAY_SIZE(linkinfo_policy)]; > + struct nlattr *vxlan[ARRAY_SIZE(vxlan_policy)]; > + > + ifmsg = ofpbuf_at(reply, NLMSG_HDRLEN, sizeof *ifmsg); > + if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof *ifmsg, > + rtlink_policy, rtlink, > + ARRAY_SIZE(rtlink_policy)) || OVS userspace code style requests that long lines with operators are split such that the operator begins the new line, indented logically to the matching prior expression. For instance: if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof *ifmsg, rtlink_policy, rtlink, ARRAY_SIZE(rtlink_policy)) || !nl_parse_nested(rtlink[IFLA_LINKINFO], linkinfo_policy, linkinfo, ARRAY_SIZE(linkinfo_policy)) || strcmp(nl_attr_get_string(linkinfo[IFLA_INFO_KIND]), kind) || !nl_parse_nested(linkinfo[IFLA_INFO_DATA], vxlan_policy, vxlan, ARRAY_SIZE(vxlan_policy))) { I won't point out all of the points in this series for this.
On Tue, Mar 14, 2017 at 11:45:11AM -0700, Joe Stringer wrote: > On 16 February 2017 at 14:25, Eric Garver <e@erig.me> wrote: > > Creates VXLAN devices using rtnetlink and tunnel metadata. > > > > Co-Authored-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> > > Signed-off-by: Eric Garver <e@erig.me> > > --- > > lib/dpif-rtnetlink.c | 181 ++++++++++++++++++++++++++++++++++++++++++++++++++- > > lib/dpif-rtnetlink.h | 2 +- > > 2 files changed, 181 insertions(+), 2 deletions(-) > > > > diff --git a/lib/dpif-rtnetlink.c b/lib/dpif-rtnetlink.c > > index b309c88d187a..8b6574d1a145 100644 > > --- a/lib/dpif-rtnetlink.c > > +++ b/lib/dpif-rtnetlink.c > > @@ -18,14 +18,192 @@ > > > > #include "dpif-rtnetlink.h" > > > > +#include <net/if.h> > > +#include <linux/ip.h> > > +#include <linux/rtnetlink.h> > > + > > #include "dpif-netlink.h" > > +#include "netdev-vport.h" > > +#include "netlink-socket.h" > > + > > +/* > > + * On some older systems, these enums are not defined. > > + */ > > +#ifndef IFLA_VXLAN_MAX > > +#define IFLA_VXLAN_MAX 0 > > +#define IFLA_VXLAN_PORT 15 > > +#endif > > +#if IFLA_VXLAN_MAX < 20 > > +#define IFLA_VXLAN_UDP_ZERO_CSUM6_RX 20 > > +#define IFLA_VXLAN_GBP 23 > > +#define IFLA_VXLAN_COLLECT_METADATA 25 > > +#endif > > + > > +static const struct nl_policy rtlink_policy[] = { > > + [IFLA_LINKINFO] = { .type = NL_A_NESTED }, > > +}; > > +static const struct nl_policy linkinfo_policy[] = { > > + [IFLA_INFO_KIND] = { .type = NL_A_STRING }, > > + [IFLA_INFO_DATA] = { .type = NL_A_NESTED }, > > +}; > > + > > + > > +static int > > +dpif_rtnetlink_destroy(const char *name) > > +{ > > + int err; > > + struct ofpbuf request, *reply; > > + > > + ofpbuf_init(&request, 0); > > + nl_msg_put_nlmsghdr(&request, 0, RTM_DELLINK, > > + NLM_F_REQUEST | NLM_F_ACK); > > + ofpbuf_put_zeros(&request, sizeof(struct ifinfomsg)); > > + nl_msg_put_string(&request, IFLA_IFNAME, name); > > + > > + err = nl_transact(NETLINK_ROUTE, &request, &reply); > > + > > + if (!err) { > > + ofpbuf_uninit(reply); > > The comment above nl_transact() states that 'reply' should be freed > using ofpbuf_delete(). Please check each of the calls to ensure we > aren't leaking memory this way. Good catch. In this instance the reply is not used, so I'll remove it. I'll verify the other locations. > > + } > > + > > + ofpbuf_uninit(&request); > > + return err; > > +} > > + > > +static int > > +dpif_rtnetlink_vxlan_destroy(const char *name) > > +{ > > + return dpif_rtnetlink_destroy(name); > > +} > > + > > +static int > > +dpif_rtnetlink_vxlan_verify(struct netdev *netdev, const char *name, > > + const char *kind) > > +{ > > + int err; > > + struct ofpbuf request, *reply; > > + struct ifinfomsg *ifmsg; > > + const struct netdev_tunnel_config *tnl_cfg; > > + > > + static const struct nl_policy vxlan_policy[] = { > > + [IFLA_VXLAN_COLLECT_METADATA] = { .type = NL_A_U8 }, > > + [IFLA_VXLAN_LEARNING] = { .type = NL_A_U8 }, > > + [IFLA_VXLAN_UDP_ZERO_CSUM6_RX] = { .type = NL_A_U8 }, > > + [IFLA_VXLAN_PORT] = { .type = NL_A_U16 }, > > + }; > > > > + tnl_cfg = netdev_get_tunnel_config(netdev); > > + if (!tnl_cfg) { > > + return EINVAL; > > + } > > + > > + ofpbuf_init(&request, 0); > > + nl_msg_put_nlmsghdr(&request, 0, RTM_GETLINK, > > + NLM_F_REQUEST); > > + ofpbuf_put_zeros(&request, sizeof(struct ifinfomsg)); > > + nl_msg_put_string(&request, IFLA_IFNAME, name); > > + > > + err = nl_transact(NETLINK_ROUTE, &request, &reply); > > + if (!err) { > > + struct nlattr *rtlink[ARRAY_SIZE(rtlink_policy)]; > > + struct nlattr *linkinfo[ARRAY_SIZE(linkinfo_policy)]; > > + struct nlattr *vxlan[ARRAY_SIZE(vxlan_policy)]; > > + > > + ifmsg = ofpbuf_at(reply, NLMSG_HDRLEN, sizeof *ifmsg); > > + if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof *ifmsg, > > + rtlink_policy, rtlink, > > + ARRAY_SIZE(rtlink_policy)) || > > OVS userspace code style requests that long lines with operators are > split such that the operator begins the new line, indented logically > to the matching prior expression. For instance: > > if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof *ifmsg, > rtlink_policy, rtlink, > ARRAY_SIZE(rtlink_policy)) > || !nl_parse_nested(rtlink[IFLA_LINKINFO], linkinfo_policy, > linkinfo, ARRAY_SIZE(linkinfo_policy)) > || strcmp(nl_attr_get_string(linkinfo[IFLA_INFO_KIND]), kind) > || !nl_parse_nested(linkinfo[IFLA_INFO_DATA], vxlan_policy, > vxlan, > ARRAY_SIZE(vxlan_policy))) { > > I won't point out all of the points in this series for this. Okay. I'll clean this up as well. Thanks Joe!
diff --git a/lib/dpif-rtnetlink.c b/lib/dpif-rtnetlink.c index b309c88d187a..8b6574d1a145 100644 --- a/lib/dpif-rtnetlink.c +++ b/lib/dpif-rtnetlink.c @@ -18,14 +18,192 @@ #include "dpif-rtnetlink.h" +#include <net/if.h> +#include <linux/ip.h> +#include <linux/rtnetlink.h> + #include "dpif-netlink.h" +#include "netdev-vport.h" +#include "netlink-socket.h" + +/* + * On some older systems, these enums are not defined. + */ +#ifndef IFLA_VXLAN_MAX +#define IFLA_VXLAN_MAX 0 +#define IFLA_VXLAN_PORT 15 +#endif +#if IFLA_VXLAN_MAX < 20 +#define IFLA_VXLAN_UDP_ZERO_CSUM6_RX 20 +#define IFLA_VXLAN_GBP 23 +#define IFLA_VXLAN_COLLECT_METADATA 25 +#endif + +static const struct nl_policy rtlink_policy[] = { + [IFLA_LINKINFO] = { .type = NL_A_NESTED }, +}; +static const struct nl_policy linkinfo_policy[] = { + [IFLA_INFO_KIND] = { .type = NL_A_STRING }, + [IFLA_INFO_DATA] = { .type = NL_A_NESTED }, +}; + + +static int +dpif_rtnetlink_destroy(const char *name) +{ + int err; + struct ofpbuf request, *reply; + + ofpbuf_init(&request, 0); + nl_msg_put_nlmsghdr(&request, 0, RTM_DELLINK, + NLM_F_REQUEST | NLM_F_ACK); + ofpbuf_put_zeros(&request, sizeof(struct ifinfomsg)); + nl_msg_put_string(&request, IFLA_IFNAME, name); + + err = nl_transact(NETLINK_ROUTE, &request, &reply); + + if (!err) { + ofpbuf_uninit(reply); + } + + ofpbuf_uninit(&request); + return err; +} + +static int +dpif_rtnetlink_vxlan_destroy(const char *name) +{ + return dpif_rtnetlink_destroy(name); +} + +static int +dpif_rtnetlink_vxlan_verify(struct netdev *netdev, const char *name, + const char *kind) +{ + int err; + struct ofpbuf request, *reply; + struct ifinfomsg *ifmsg; + const struct netdev_tunnel_config *tnl_cfg; + + static const struct nl_policy vxlan_policy[] = { + [IFLA_VXLAN_COLLECT_METADATA] = { .type = NL_A_U8 }, + [IFLA_VXLAN_LEARNING] = { .type = NL_A_U8 }, + [IFLA_VXLAN_UDP_ZERO_CSUM6_RX] = { .type = NL_A_U8 }, + [IFLA_VXLAN_PORT] = { .type = NL_A_U16 }, + }; + tnl_cfg = netdev_get_tunnel_config(netdev); + if (!tnl_cfg) { + return EINVAL; + } + + ofpbuf_init(&request, 0); + nl_msg_put_nlmsghdr(&request, 0, RTM_GETLINK, + NLM_F_REQUEST); + ofpbuf_put_zeros(&request, sizeof(struct ifinfomsg)); + nl_msg_put_string(&request, IFLA_IFNAME, name); + + err = nl_transact(NETLINK_ROUTE, &request, &reply); + if (!err) { + struct nlattr *rtlink[ARRAY_SIZE(rtlink_policy)]; + struct nlattr *linkinfo[ARRAY_SIZE(linkinfo_policy)]; + struct nlattr *vxlan[ARRAY_SIZE(vxlan_policy)]; + + ifmsg = ofpbuf_at(reply, NLMSG_HDRLEN, sizeof *ifmsg); + if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof *ifmsg, + rtlink_policy, rtlink, + ARRAY_SIZE(rtlink_policy)) || + !nl_parse_nested(rtlink[IFLA_LINKINFO], linkinfo_policy, + linkinfo, ARRAY_SIZE(linkinfo_policy)) || + strcmp(nl_attr_get_string(linkinfo[IFLA_INFO_KIND]), kind) || + !nl_parse_nested(linkinfo[IFLA_INFO_DATA], vxlan_policy, vxlan, + ARRAY_SIZE(vxlan_policy))) { + err = EINVAL; + } + if (!err) { + if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING]) || + 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_COLLECT_METADATA]) || + 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) || + tnl_cfg->dst_port != + nl_attr_get_be16(vxlan[IFLA_VXLAN_PORT])) { + err = EINVAL; + } + } + if (!err) { + if ((tnl_cfg->exts & (1 << OVS_VXLAN_EXT_GBP)) && + !(vxlan[IFLA_VXLAN_GBP] && + nl_attr_get_flag(vxlan[IFLA_VXLAN_GBP]))) { + err = EINVAL; + } + } + ofpbuf_uninit(reply); + } + ofpbuf_uninit(&request); + return err; +} + +static int +dpif_rtnetlink_vxlan_create_kind(struct netdev *netdev, const char *kind) +{ + int err; + struct ofpbuf request, *reply; + size_t linkinfo_off, infodata_off; + char namebuf[NETDEV_VPORT_NAME_BUFSIZE]; + const char *name = netdev_vport_get_dpif_port(netdev, + namebuf, sizeof namebuf); + struct ifinfomsg *ifinfo; + const struct netdev_tunnel_config *tnl_cfg; + tnl_cfg = netdev_get_tunnel_config(netdev); + if (!tnl_cfg) { + return EINVAL; + } + + ofpbuf_init(&request, 0); + nl_msg_put_nlmsghdr(&request, 0, RTM_NEWLINK, + NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE); + ifinfo = ofpbuf_put_zeros(&request, sizeof(struct ifinfomsg)); + ifinfo->ifi_change = ifinfo->ifi_flags = IFF_UP; + nl_msg_put_string(&request, IFLA_IFNAME, name); + nl_msg_put_u32(&request, IFLA_MTU, UINT16_MAX); + linkinfo_off = nl_msg_start_nested(&request, IFLA_LINKINFO); + nl_msg_put_string(&request, IFLA_INFO_KIND, kind); + infodata_off = nl_msg_start_nested(&request, IFLA_INFO_DATA); + nl_msg_put_u8(&request, IFLA_VXLAN_LEARNING, 0); + nl_msg_put_u8(&request, IFLA_VXLAN_COLLECT_METADATA, 1); + nl_msg_put_u8(&request, IFLA_VXLAN_UDP_ZERO_CSUM6_RX, 1); + if (tnl_cfg->exts & (1 << OVS_VXLAN_EXT_GBP)) { + nl_msg_put_flag(&request, IFLA_VXLAN_GBP); + } + nl_msg_put_be16(&request, IFLA_VXLAN_PORT, tnl_cfg->dst_port); + nl_msg_end_nested(&request, infodata_off); + nl_msg_end_nested(&request, linkinfo_off); + + err = nl_transact(NETLINK_ROUTE, &request, &reply); + + if (!err) { + ofpbuf_uninit(reply); + } + + if (!err && (err = dpif_rtnetlink_vxlan_verify(netdev, name, kind))) { + dpif_rtnetlink_vxlan_destroy(name); + } + + ofpbuf_uninit(&request); + return err; +} + +static int +dpif_rtnetlink_vxlan_create(struct netdev *netdev) +{ + return dpif_rtnetlink_vxlan_create_kind(netdev, "vxlan"); +} int dpif_rtnetlink_port_create(struct netdev *netdev) { switch (netdev_to_ovs_vport_type(netdev_get_type(netdev))) { case OVS_VPORT_TYPE_VXLAN: + return dpif_rtnetlink_vxlan_create(netdev); case OVS_VPORT_TYPE_GRE: case OVS_VPORT_TYPE_GENEVE: case OVS_VPORT_TYPE_NETDEV: @@ -41,10 +219,11 @@ dpif_rtnetlink_port_create(struct netdev *netdev) } int -dpif_rtnetlink_port_destroy(const char *name OVS_UNUSED, const char *type) +dpif_rtnetlink_port_destroy(const char *name, const char *type) { switch (netdev_to_ovs_vport_type(type)) { case OVS_VPORT_TYPE_VXLAN: + return dpif_rtnetlink_vxlan_destroy(name); case OVS_VPORT_TYPE_GRE: case OVS_VPORT_TYPE_GENEVE: case OVS_VPORT_TYPE_NETDEV: diff --git a/lib/dpif-rtnetlink.h b/lib/dpif-rtnetlink.h index 56ab63532829..515820f02e66 100644 --- a/lib/dpif-rtnetlink.h +++ b/lib/dpif-rtnetlink.h @@ -22,7 +22,7 @@ #include "netdev.h" int dpif_rtnetlink_port_create(struct netdev *netdev); -int dpif_rtnetlink_port_destroy(const char *name OVS_UNUSED, const char *type); +int dpif_rtnetlink_port_destroy(const char *name, const char *type); #ifndef __linux__ /* Dummy implementations for non Linux builds.