[<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