On Tue, Mar 14, 2017 at 11:45:11AM -0700, Joe Stringer wrote:
> On 16 February 2017 at 14:25, Eric Garver <[email protected]> wrote:
> > Creates VXLAN devices using rtnetlink and tunnel metadata.
> >
> > Co-Authored-by: Thadeu Lima de Souza Cascardo <[email protected]>
> > Signed-off-by: Thadeu Lima de Souza Cascardo <[email protected]>
> > Signed-off-by: Eric Garver <[email protected]>
> > ---
> > 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!
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
