lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJieiUhNHDQ3-Bn8gnQzTM2Nkc-OuLmd=OuTys5b4M0exzpjZQ@mail.gmail.com>
Date:   Tue, 19 Dec 2017 08:28:17 -0800
From:   Roopa Prabhu <roopa@...ulusnetworks.com>
To:     Tom Herbert <tom@...ntonium.net>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        rohit@...ntonium.net
Subject: Re: [PATCH v4 net-next 6/6] ila: Route notify

On Fri, Dec 15, 2017 at 10:28 AM, Tom Herbert <tom@...ntonium.net> wrote:
> Implement RTM notifications for ILA routers. This adds support to
> ILA LWT to send a netlink RTM message when a router is uses.
>
> THe ILA notify mechanism can be used in two contexts:
>
> - On an ILA forwarding cache a route prefix can be configured to
>   do an ILA notification. This method is used when address
>   resolution needs to be done on an address.
> - One an ILA router an ILA host route entry may include a
>   noitification. The purpose of this is to get a notification
>   to a userspace daemon to send and ILA redirect
>
> Signed-off-by: Tom Herbert <tom@...ntonium.net>
> ---
>  include/uapi/linux/ila.h       |   2 +
>  include/uapi/linux/rtnetlink.h |   8 +-
>  net/ipv6/ila/ila_lwt.c         | 268 ++++++++++++++++++++++++++++-------------
>  3 files changed, 193 insertions(+), 85 deletions(-)
>
> diff --git a/include/uapi/linux/ila.h b/include/uapi/linux/ila.h
> index db45d3e49a12..5675f3e71fac 100644
> --- a/include/uapi/linux/ila.h
> +++ b/include/uapi/linux/ila.h
> @@ -19,6 +19,8 @@ enum {
>         ILA_ATTR_CSUM_MODE,                     /* u8 */
>         ILA_ATTR_IDENT_TYPE,                    /* u8 */
>         ILA_ATTR_HOOK_TYPE,                     /* u8 */
> +       ILA_ATTR_NOTIFY_DST,                    /* flag */
> +       ILA_ATTR_NOTIFY_SRC,                    /* flag */
>
>         __ILA_ATTR_MAX,
>  };
> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> index d8b5f80c2ea6..8d358a300d8a 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -13,7 +13,8 @@
>   */
>  #define RTNL_FAMILY_IPMR               128
>  #define RTNL_FAMILY_IP6MR              129
> -#define RTNL_FAMILY_MAX                        129
> +#define RTNL_FAMILY_ILA                        130

I don't see this being used, unless I missed it.


> +#define RTNL_FAMILY_MAX                        130
>
>  /****
>   *             Routing/neighbour discovery messages.
> @@ -150,6 +151,9 @@ enum {
>         RTM_NEWCACHEREPORT = 96,
>  #define RTM_NEWCACHEREPORT RTM_NEWCACHEREPORT
>
> +       RTM_ADDR_RESOLVE = 98,
> +#define RTM_ADDR_RESOLVE RTM_ADDR_RESOLVE


Its nice that you are trying to add a generic resolver notify
format..., looks good...a few minor comments below for consistency:


your patch adds address resolve notifications with:
- generic notification msg type RTM_ADDR_RESOLVE with route msg format
- On the ILA specific RTNLGRP_ILA_NOTIFY multicast group

Other way to possibly format this for "generic" rtnl netlink resolver is:
- notification msg type RTM_NEWROUTE with route msg format (This is
for consistency: route msg format is always used with RTM_*ROUTE msg
types)
- route entry msg type RTNL_FAMILY_ILA (ie struct rtmsg-> rtm_family
to RTNL_FAMILY_ILA)
- generic address resolution netlink multicast group RTNLGRP_ADDR_RESOLVE_NOTIFY


OR   keep everything "specific" to ILA using the ILA genl channel msg
format and family


Besides that, since you are using the route msg format, you could
potentially s/ADDR_RESOLVE/ROUTE_RESOLVE/g everywhere above.



> +
>         __RTM_MAX,
>  #define RTM_MAX                (((__RTM_MAX + 3) & ~3) - 1)
>  };
> @@ -676,6 +680,8 @@ enum rtnetlink_groups {
>  #define RTNLGRP_IPV4_MROUTE_R  RTNLGRP_IPV4_MROUTE_R
>         RTNLGRP_IPV6_MROUTE_R,
>  #define RTNLGRP_IPV6_MROUTE_R  RTNLGRP_IPV6_MROUTE_R
> +       RTNLGRP_ILA_NOTIFY,
> +#define RTNLGRP_ILA_NOTIFY     RTNLGRP_ILA_NOTIFY
>         __RTNLGRP_MAX
>  };
>  #define RTNLGRP_MAX    (__RTNLGRP_MAX - 1)
> diff --git a/net/ipv6/ila/ila_lwt.c b/net/ipv6/ila/ila_lwt.c
> index 9f1e46a1468e..303c91e3bf76 100644
> --- a/net/ipv6/ila/ila_lwt.c
> +++ b/net/ipv6/ila/ila_lwt.c
> @@ -19,10 +19,15 @@
>  struct ila_lwt {
>         struct ila_params p;
>         struct dst_cache dst_cache;
> +       u8 hook_type;
>         u32 connected : 1;
> -       u32 lwt_output : 1;
> +       u32 xlat : 1;
> +       u32 notify : 2;
>  };
>
> +#define ILA_NOTIFY_DST 1
> +#define ILA_NOTIFY_SRC 2
> +
>  static inline struct ila_lwt *ila_lwt_lwtunnel(
>         struct lwtunnel_state *lwt)
>  {
> @@ -35,6 +40,67 @@ static inline struct ila_params *ila_params_lwtunnel(
>         return &ila_lwt_lwtunnel(lwt)->p;
>  }
>
> +static size_t ila_rslv_msgsize(void)
> +{
> +       size_t len =
> +               NLMSG_ALIGN(sizeof(struct rtmsg))
> +               + nla_total_size(16)     /* RTA_DST */
> +               + nla_total_size(16)     /* RTA_SRC */
> +               ;
> +
> +       return len;
> +}
> +
> +void ila_notify(struct net *net, struct sk_buff *skb, struct ila_lwt *lwt)
> +{
> +       struct ipv6hdr *ip6h = ipv6_hdr(skb);
> +       int flags = NLM_F_MULTI;
> +       struct sk_buff *nlskb;
> +       struct nlmsghdr *nlh;
> +       struct rtmsg *rtm;
> +       int err = 0;
> +
> +       /* Send ILA notification to user */
> +       nlskb = nlmsg_new(ila_rslv_msgsize(), GFP_KERNEL);
> +       if (!nlskb)
> +               return;
> +
> +       nlh = nlmsg_put(nlskb, 0, 0, RTM_ADDR_RESOLVE, sizeof(*rtm), flags);
> +       if (!nlh) {
> +               err = -EMSGSIZE;
> +               goto errout;
> +       }
> +
> +       rtm = nlmsg_data(nlh);
> +       rtm->rtm_family   = AF_INET6;
> +       rtm->rtm_dst_len  = 128;
> +       rtm->rtm_src_len  = 0;
> +       rtm->rtm_tos      = 0;
> +       rtm->rtm_table    = RT6_TABLE_UNSPEC;
> +       rtm->rtm_type     = RTN_UNICAST;
> +       rtm->rtm_scope    = RT_SCOPE_UNIVERSE;
> +
> +       if (((lwt->notify & ILA_NOTIFY_DST) &&
> +            nla_put_in6_addr(nlskb, RTA_DST, &ip6h->daddr)) ||
> +           ((lwt->notify & ILA_NOTIFY_SRC) &&
> +            nla_put_in6_addr(nlskb, RTA_SRC, &ip6h->saddr))) {
> +               nlmsg_cancel(nlskb, nlh);
> +               err = -EMSGSIZE;
> +               goto errout;
> +       }
> +
> +       nlmsg_end(nlskb, nlh);
> +
> +       rtnl_notify(nlskb, net, 0, RTNLGRP_ILA_NOTIFY, NULL, GFP_ATOMIC);
> +
> +       return;
> +
> +errout:
> +       kfree_skb(nlskb);
> +       WARN_ON(err == -EMSGSIZE);
> +       rtnl_set_sk_err(net, RTNLGRP_ILA_NOTIFY, err);
> +}
> +
>  static int ila_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>  {
>         struct dst_entry *orig_dst = skb_dst(skb);
> @@ -46,11 +112,14 @@ static int ila_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>         if (skb->protocol != htons(ETH_P_IPV6))
>                 goto drop;
>
> -       if (ilwt->lwt_output)
> +       if (ilwt->xlat)
>                 ila_update_ipv6_locator(skb,
>                                         ila_params_lwtunnel(orig_dst->lwtstate),
>                                         true);
>
> +       if (ilwt->notify)
> +               ila_notify(net, skb, ilwt);
> +
>         if (rt->rt6i_flags & (RTF_GATEWAY | RTF_CACHE)) {
>                 /* Already have a next hop address in route, no need for
>                  * dest cache route.
> @@ -106,11 +175,14 @@ static int ila_input(struct sk_buff *skb)
>         if (skb->protocol != htons(ETH_P_IPV6))
>                 goto drop;
>
> -       if (!ilwt->lwt_output)
> +       if (ilwt->xlat)
>                 ila_update_ipv6_locator(skb,
>                                         ila_params_lwtunnel(dst->lwtstate),
>                                         false);
>
> +       if (ilwt->notify)
> +               ila_notify(dev_net(dst->dev), skb, ilwt);
> +
>         return dst->lwtstate->orig_input(skb);
>
>  drop:
> @@ -123,6 +195,8 @@ static const struct nla_policy ila_nl_policy[ILA_ATTR_MAX + 1] = {
>         [ILA_ATTR_CSUM_MODE] = { .type = NLA_U8, },
>         [ILA_ATTR_IDENT_TYPE] = { .type = NLA_U8, },
>         [ILA_ATTR_HOOK_TYPE] = { .type = NLA_U8, },
> +       [ILA_ATTR_NOTIFY_DST] = { .type = NLA_FLAG },
> +       [ILA_ATTR_NOTIFY_SRC] = { .type = NLA_FLAG },
>  };
>
>  static int ila_build_state(struct net *net, struct nlattr *nla,
> @@ -130,64 +204,73 @@ static int ila_build_state(struct net *net, struct nlattr *nla,
>                            struct lwtunnel_state **ts,
>                            struct netlink_ext_ack *extack)
>  {
> -       struct ila_lwt *ilwt;
> -       struct ila_params *p;
> -       struct nlattr *tb[ILA_ATTR_MAX + 1];
> -       struct lwtunnel_state *newts;
>         const struct fib6_config *cfg6 = cfg;
> -       struct ila_addr *iaddr;
> +       struct ila_addr *iaddr = (struct ila_addr *)&cfg6->fc_dst;
>         u8 ident_type = ILA_ATYPE_USE_FORMAT;
>         u8 hook_type = ILA_HOOK_ROUTE_OUTPUT;
> +       struct nlattr *tb[ILA_ATTR_MAX + 1];
>         u8 csum_mode = ILA_CSUM_NO_ACTION;
> -       bool lwt_output = true;
> +       struct lwtunnel_state *newts;
> +       struct ila_lwt *ilwt;
> +       struct ila_params *p;
>         u8 eff_ident_type;
> -       int ret;
> +       int err;
>
>         if (family != AF_INET6)
>                 return -EINVAL;
>
> -       ret = nla_parse_nested(tb, ILA_ATTR_MAX, nla, ila_nl_policy, extack);
> -       if (ret < 0)
> -               return ret;
> +       err = nla_parse_nested(tb, ILA_ATTR_MAX, nla, ila_nl_policy, extack);
> +       if (err < 0)
> +               return err;
>
> -       if (!tb[ILA_ATTR_LOCATOR])
> -               return -EINVAL;
> +       if (tb[ILA_ATTR_LOCATOR]) {
> +               /* Doing ILA translation */
>
> -       iaddr = (struct ila_addr *)&cfg6->fc_dst;
> +               if (tb[ILA_ATTR_IDENT_TYPE])
> +                       ident_type = nla_get_u8(tb[ILA_ATTR_IDENT_TYPE]);
>
> -       if (tb[ILA_ATTR_IDENT_TYPE])
> -               ident_type = nla_get_u8(tb[ILA_ATTR_IDENT_TYPE]);
> +               if (ident_type == ILA_ATYPE_USE_FORMAT) {
> +                       /* Infer identifier type from type field in formatted
> +                        * identifier.
> +                        */
>
> -       if (ident_type == ILA_ATYPE_USE_FORMAT) {
> -               /* Infer identifier type from type field in formatted
> -                * identifier.
> -                */
> +                       if (cfg6->fc_dst_len < 8 *
> +                           sizeof(struct ila_locator) + 3) {
> +                               /* Need to have full locator and at least type
> +                                * field included in destination
> +                                */
> +                               return -EINVAL;
> +                       }
> +
> +                       eff_ident_type = iaddr->ident.type;
> +               } else {
> +                       eff_ident_type = ident_type;
> +               }
>
> -               if (cfg6->fc_dst_len < 8 * sizeof(struct ila_locator) + 3) {
> -                       /* Need to have full locator and at least type field
> -                        * included in destination
> -                        */
> +               switch (eff_ident_type) {
> +               case ILA_ATYPE_IID:
> +                       /* Don't allow ILA for IID type */
> +                       return -EINVAL;
> +               case ILA_ATYPE_LUID:
> +                       break;
> +               case ILA_ATYPE_VIRT_V4:
> +               case ILA_ATYPE_VIRT_UNI_V6:
> +               case ILA_ATYPE_VIRT_MULTI_V6:
> +               case ILA_ATYPE_NONLOCAL_ADDR:
> +                       /* These ILA formats are not supported yet. */
> +               default:
>                         return -EINVAL;
>                 }
>
> -               eff_ident_type = iaddr->ident.type;
> -       } else {
> -               eff_ident_type = ident_type;
> -       }
> +               csum_mode = nla_get_u8(tb[ILA_ATTR_CSUM_MODE]);
>
> -       switch (eff_ident_type) {
> -       case ILA_ATYPE_IID:
> -               /* Don't allow ILA for IID type */
> -               return -EINVAL;
> -       case ILA_ATYPE_LUID:
> -               break;
> -       case ILA_ATYPE_VIRT_V4:
> -       case ILA_ATYPE_VIRT_UNI_V6:
> -       case ILA_ATYPE_VIRT_MULTI_V6:
> -       case ILA_ATYPE_NONLOCAL_ADDR:
> -               /* These ILA formats are not supported yet. */
> -       default:
> -               return -EINVAL;
> +               if (csum_mode == ILA_CSUM_NEUTRAL_MAP &&
> +                   ila_csum_neutral_set(iaddr->ident)) {
> +                       /* Don't allow translation if checksum neutral bit is
> +                        * configured and it's set in the SIR address.
> +                        */
> +                       return -EINVAL;
> +               }
>         }
>
>         if (tb[ILA_ATTR_HOOK_TYPE])
> @@ -195,58 +278,62 @@ static int ila_build_state(struct net *net, struct nlattr *nla,
>
>         switch (hook_type) {
>         case ILA_HOOK_ROUTE_OUTPUT:
> -               lwt_output = true;
> -               break;
>         case ILA_HOOK_ROUTE_INPUT:
> -               lwt_output = false;
>                 break;
>         default:
>                 return -EINVAL;
>         }
>
> -       if (tb[ILA_ATTR_CSUM_MODE])
> -               csum_mode = nla_get_u8(tb[ILA_ATTR_CSUM_MODE]);
> -
> -       if (csum_mode == ILA_CSUM_NEUTRAL_MAP &&
> -           ila_csum_neutral_set(iaddr->ident)) {
> -               /* Don't allow translation if checksum neutral bit is
> -                * configured and it's set in the SIR address.
> -                */
> -               return -EINVAL;
> -       }
> -
>         newts = lwtunnel_state_alloc(sizeof(*ilwt));
>         if (!newts)
>                 return -ENOMEM;
>
>         ilwt = ila_lwt_lwtunnel(newts);
> -       ret = dst_cache_init(&ilwt->dst_cache, GFP_ATOMIC);
> -       if (ret) {
> +
> +       err = dst_cache_init(&ilwt->dst_cache, GFP_ATOMIC);
> +       if (err) {
>                 kfree(newts);
> -               return ret;
> +               return err;
>         }
>
> -       ilwt->lwt_output = !!lwt_output;
> +       newts->type = LWTUNNEL_ENCAP_ILA;
>
> -       p = ila_params_lwtunnel(newts);
> +       switch (hook_type) {
> +       case ILA_HOOK_ROUTE_OUTPUT:
> +               newts->flags |= LWTUNNEL_STATE_OUTPUT_REDIRECT;
> +               break;
> +       case ILA_HOOK_ROUTE_INPUT:
> +               newts->flags |= LWTUNNEL_STATE_INPUT_REDIRECT;
> +               break;
> +       }
>
> -       p->csum_mode = csum_mode;
> -       p->ident_type = ident_type;
> -       p->locator.v64 = (__force __be64)nla_get_u64(tb[ILA_ATTR_LOCATOR]);
> +       ilwt->hook_type = hook_type;
>
> -       /* Precompute checksum difference for translation since we
> -        * know both the old locator and the new one.
> -        */
> -       p->locator_match = iaddr->loc;
> +       if (tb[ILA_ATTR_NOTIFY_DST])
> +               ilwt->notify |= ILA_NOTIFY_DST;
>
> -       ila_init_saved_csum(p);
> +       if (tb[ILA_ATTR_NOTIFY_SRC])
> +               ilwt->notify |= ILA_NOTIFY_SRC;
>
> -       newts->type = LWTUNNEL_ENCAP_ILA;
> -       newts->flags |= LWTUNNEL_STATE_OUTPUT_REDIRECT |
> -                       LWTUNNEL_STATE_INPUT_REDIRECT;
> +       p = ila_params_lwtunnel(newts);
>
> -       if (cfg6->fc_dst_len == 8 * sizeof(struct in6_addr))
> -               ilwt->connected = 1;
> +       if (tb[ILA_ATTR_LOCATOR]) {
> +               ilwt->xlat = true;
> +               p->csum_mode = csum_mode;
> +               p->ident_type = ident_type;
> +               p->locator.v64 = (__force __be64)nla_get_u64(
> +                                                       tb[ILA_ATTR_LOCATOR]);
> +
> +               /* Precompute checksum difference for translation since we
> +                * know both the old locator and the new one.
> +                */
> +               p->locator_match = iaddr->loc;
> +
> +               ila_init_saved_csum(p);
> +
> +               if (cfg6->fc_dst_len == 8 * sizeof(struct in6_addr))
> +                       ilwt->connected = 1;
> +       }
>
>         *ts = newts;
>
> @@ -264,21 +351,32 @@ static int ila_fill_encap_info(struct sk_buff *skb,
>         struct ila_params *p = ila_params_lwtunnel(lwtstate);
>         struct ila_lwt *ilwt = ila_lwt_lwtunnel(lwtstate);
>
> -       if (nla_put_u64_64bit(skb, ILA_ATTR_LOCATOR, (__force u64)p->locator.v64,
> -                             ILA_ATTR_PAD))
> +       if (ilwt->xlat) {
> +               if (nla_put_u64_64bit(skb, ILA_ATTR_LOCATOR,
> +                                     (__force u64)p->locator.v64,
> +                                     ILA_ATTR_PAD))
>                 goto nla_put_failure;
>
> -       if (nla_put_u8(skb, ILA_ATTR_CSUM_MODE, (__force u8)p->csum_mode))
> -               goto nla_put_failure;
> +               if (nla_put_u8(skb, ILA_ATTR_CSUM_MODE,
> +                              (__force u8)p->csum_mode))
> +                       goto nla_put_failure;
>
> -       if (nla_put_u8(skb, ILA_ATTR_IDENT_TYPE, (__force u8)p->ident_type))
> -               goto nla_put_failure;
> +               if (nla_put_u8(skb, ILA_ATTR_IDENT_TYPE,
> +                              (__force u8)p->ident_type))
> +                       goto nla_put_failure;
> +       }
>
> -       if (nla_put_u8(skb, ILA_ATTR_HOOK_TYPE,
> -                      ilwt->lwt_output ? ILA_HOOK_ROUTE_OUTPUT :
> -                                         ILA_HOOK_ROUTE_INPUT))
> +       if (nla_put_u8(skb, ILA_ATTR_HOOK_TYPE, ilwt->hook_type))
>                 goto nla_put_failure;
>
> +       if (ilwt->notify & ILA_NOTIFY_DST)
> +               if (nla_put_flag(skb, ILA_ATTR_NOTIFY_DST))
> +                       goto nla_put_failure;
> +
> +       if (ilwt->notify & ILA_NOTIFY_SRC)
> +               if (nla_put_flag(skb, ILA_ATTR_NOTIFY_SRC))
> +                       goto nla_put_failure;
> +
>         return 0;
>
>  nla_put_failure:
> @@ -291,6 +389,8 @@ static int ila_encap_nlsize(struct lwtunnel_state *lwtstate)
>                nla_total_size(sizeof(u8)) +        /* ILA_ATTR_CSUM_MODE */
>                nla_total_size(sizeof(u8)) +        /* ILA_ATTR_IDENT_TYPE */
>                nla_total_size(sizeof(u8)) +        /* ILA_ATTR_HOOK_TYPE */
> +              nla_total_size(0) +                 /* ILA_ATTR_NOTIFY_DST */
> +              nla_total_size(0) +                 /* ILA_ATTR_NOTIFY_SRC */
>                0;
>  }
>
> --
> 2.11.0
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ