All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH net-next] ipmr: ip6mr: APIs to support adding more than MAXVIFS/MAXMIFS
@ 2021-09-17 22:41 Stephen Suryaputra
  2021-09-18  3:10 ` kernel test robot
  2021-09-19  1:07 ` Andrew Lunn
  0 siblings, 2 replies; 5+ messages in thread
From: Stephen Suryaputra @ 2021-09-17 22:41 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Suryaputra

MAXVIFS and MAXMIFS are too small (32) for certain applications. But
they are defined in user header files  So, use a different definition
CONFIG_IP_MROUTE_EXT_MAXVIFS that is configurable and different ioctl
requests (MRT_xyz_EXT and MRT6_xyz_EXT) as well as a different structure
for adding MFC (mfcctl_ext).

CONFIG_IP_MROUTE_EXT_MAXVIFS is bounded by the IF_SETSIZE (256) in
mroute6.h.

This patch is extending the following RFC:
http://patchwork.ozlabs.org/project/netdev/patch/m1eiis8uc6.fsf@fess.ebiederm.org/

Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
---
 include/linux/mroute_base.h  |  11 +---
 include/uapi/linux/mroute.h  |  14 ++++-
 include/uapi/linux/mroute6.h |   5 +-
 net/ipv4/Kconfig             |  11 ++++
 net/ipv4/ipmr.c              | 115 +++++++++++++++++++++++++++++------
 net/ipv4/ipmr_base.c         |   2 +-
 net/ipv6/ip6mr.c             |  52 +++++++++++-----
 7 files changed, 163 insertions(+), 47 deletions(-)

diff --git a/include/linux/mroute_base.h b/include/linux/mroute_base.h
index 8071148f29a6..85f3a8864044 100644
--- a/include/linux/mroute_base.h
+++ b/include/linux/mroute_base.h
@@ -89,13 +89,6 @@ static inline int mr_call_vif_notifiers(struct net *net,
 	return call_fib_notifiers(net, event_type, &info.info);
 }
 
-#ifndef MAXVIFS
-/* This one is nasty; value is defined in uapi using different symbols for
- * mroute and morute6 but both map into same 32.
- */
-#define MAXVIFS	32
-#endif
-
 #define VIF_EXISTS(_mrt, _idx) (!!((_mrt)->vif_table[_idx].dev))
 
 /* mfc_flags:
@@ -145,7 +138,7 @@ struct mr_mfc {
 			unsigned long pkt;
 			unsigned long wrong_if;
 			unsigned long lastuse;
-			unsigned char ttls[MAXVIFS];
+			unsigned char ttls[CONFIG_IP_MROUTE_EXT_MAXVIFS];
 			refcount_t refcount;
 		} res;
 	} mfc_un;
@@ -246,7 +239,7 @@ struct mr_table {
 	struct sock __rcu	*mroute_sk;
 	struct timer_list	ipmr_expire_timer;
 	struct list_head	mfc_unres_queue;
-	struct vif_device	vif_table[MAXVIFS];
+	struct vif_device	vif_table[CONFIG_IP_MROUTE_EXT_MAXVIFS];
 	struct rhltable		mfc_hash;
 	struct list_head	mfc_cache_list;
 	int			maxvif;
diff --git a/include/uapi/linux/mroute.h b/include/uapi/linux/mroute.h
index 1a42f5f9b31b..b28f46565a43 100644
--- a/include/uapi/linux/mroute.h
+++ b/include/uapi/linux/mroute.h
@@ -29,7 +29,10 @@
 #define MRT_ADD_MFC_PROXY	(MRT_BASE+10)	/* Add a (*,*|G) mfc entry	*/
 #define MRT_DEL_MFC_PROXY	(MRT_BASE+11)	/* Del a (*,*|G) mfc entry	*/
 #define MRT_FLUSH	(MRT_BASE+12)	/* Flush all mfc entries and/or vifs	*/
-#define MRT_MAX		(MRT_BASE+12)
+#define MRT_ADD_VIF_EXT	(MRT_BASE+13)	/* Add a virtual interface		*/
+#define MRT_ADD_MFC_EXT	(MRT_BASE+14)	/* Add a multicast forwarding entry	*/
+#define MRT_ADD_MFC_PROXY_EXT	(MRT_BASE+15)	/* Add a (*,*|G) mfc entry	*/
+#define MRT_MAX		(MRT_BASE+15)
 
 #define SIOCGETVIFCNT	SIOCPROTOPRIVATE	/* IP protocol privates */
 #define SIOCGETSGCNT	(SIOCPROTOPRIVATE+1)
@@ -88,6 +91,15 @@ struct mfcctl {
 	int	     mfcc_expire;
 };
 
+struct mfcctl_ext {
+	/* Need to be the same as mfcctl */
+	struct in_addr mfcc_origin;		/* Origin of mcast	*/
+	struct in_addr mfcc_mcastgrp;		/* Group in question	*/
+	vifi_t	mfcc_parent;			/* Where it arrived	*/
+	unsigned char mfcc_ttls[];		/* Where it is going	*/
+	/* Don't put anything here as mfcc_ttls should grow into here */
+};
+
 /*  Group count retrieval for mrouted */
 struct sioc_sg_req {
 	struct in_addr src;
diff --git a/include/uapi/linux/mroute6.h b/include/uapi/linux/mroute6.h
index a1fd6173e2db..1d15af3e0011 100644
--- a/include/uapi/linux/mroute6.h
+++ b/include/uapi/linux/mroute6.h
@@ -32,7 +32,10 @@
 #define MRT6_ADD_MFC_PROXY	(MRT6_BASE+10)	/* Add a (*,*|G) mfc entry	*/
 #define MRT6_DEL_MFC_PROXY	(MRT6_BASE+11)	/* Del a (*,*|G) mfc entry	*/
 #define MRT6_FLUSH	(MRT6_BASE+12)	/* Flush all mfc entries and/or vifs	*/
-#define MRT6_MAX	(MRT6_BASE+12)
+#define MRT6_ADD_MIF_EXT	(MRT6_BASE+13)	/* Add a virtual interface		*/
+#define MRT6_ADD_MFC_EXT	(MRT6_BASE+14)	/* Add a multicast forwarding entry	*/
+#define MRT6_ADD_MFC_PROXY_EXT	(MRT6_BASE+15)	/* Add a (*,*|G) mfc entry	*/
+#define MRT6_MAX	(MRT6_BASE+15)
 
 #define SIOCGETMIFCNT_IN6	SIOCPROTOPRIVATE	/* IP protocol privates */
 #define SIOCGETSGCNT_IN6	(SIOCPROTOPRIVATE+1)
diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index 87983e70f03f..34ff677a4dca 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -243,6 +243,17 @@ config IP_MROUTE_MULTIPLE_TABLES
 
 	  If unsure, say N.
 
+config IP_MROUTE_EXT_MAXVIFS
+	int "IP: extended maximum multicast routing vif"
+	depends on IP_MROUTE || IPV6_MROUTE
+	range 32 256
+	default 32
+	help
+	  Maximum multicast routing vif is set at 32 and it is defined in the
+	  user API header file, so it is fixed. There are cases where more
+	  vifs are needed. This config is used to get more vifs. The default
+	  is set to be the same as MAXVIFS (32).
+
 config IP_PIMSM_V1
 	bool "IP: PIM-SM version 1 support"
 	depends on IP_MROUTE
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 2dda856ca260..50c81c4c347a 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -788,9 +788,9 @@ static void ipmr_update_thresholds(struct mr_table *mrt, struct mr_mfc *cache,
 {
 	int vifi;
 
-	cache->mfc_un.res.minvif = MAXVIFS;
+	cache->mfc_un.res.minvif = CONFIG_IP_MROUTE_EXT_MAXVIFS;
 	cache->mfc_un.res.maxvif = 0;
-	memset(cache->mfc_un.res.ttls, 255, MAXVIFS);
+	memset(cache->mfc_un.res.ttls, 255, CONFIG_IP_MROUTE_EXT_MAXVIFS);
 
 	for (vifi = 0; vifi < mrt->maxvif; vifi++) {
 		if (VIF_EXISTS(mrt, vifi) &&
@@ -952,7 +952,7 @@ static struct mfc_cache *ipmr_cache_alloc(void)
 
 	if (c) {
 		c->_c.mfc_un.res.last_assert = jiffies - MFC_ASSERT_THRESH - 1;
-		c->_c.mfc_un.res.minvif = MAXVIFS;
+		c->_c.mfc_un.res.minvif = CONFIG_IP_MROUTE_EXT_MAXVIFS;
 		c->_c.free = ipmr_cache_free_rcu;
 		refcount_set(&c->_c.mfc_un.res.refcount, 1);
 	}
@@ -1185,15 +1185,15 @@ static int ipmr_mfc_delete(struct mr_table *mrt, struct mfcctl *mfc, int parent)
 	return 0;
 }
 
-static int ipmr_mfc_add(struct net *net, struct mr_table *mrt,
-			struct mfcctl *mfc, int mrtsock, int parent)
+static int ipmr_mfc_add_ext(struct net *net, struct mr_table *mrt,
+			    struct mfcctl_ext *mfc, int mrtsock, int parent)
 {
 	struct mfc_cache *uc, *c;
 	struct mr_mfc *_uc;
 	bool found;
 	int ret;
 
-	if (mfc->mfcc_parent >= MAXVIFS)
+	if (mfc->mfcc_parent >= CONFIG_IP_MROUTE_EXT_MAXVIFS)
 		return -ENFILE;
 
 	/* The entries are added/deleted only under RTNL */
@@ -1265,6 +1265,36 @@ static int ipmr_mfc_add(struct net *net, struct mr_table *mrt,
 	return 0;
 }
 
+static int ipmr_mfc_add(struct net *net, struct mr_table *mrt,
+			struct mfcctl *mfc, int mrtsock, int parent)
+{
+	unsigned int ext_maxvifs = CONFIG_IP_MROUTE_EXT_MAXVIFS;
+	struct mfcctl_ext *mfc_ext;
+	int i, ret;
+
+	/* Maintain the behavior for MRT_ADD_MFC. */
+	if (mfc->mfcc_parent >= MAXVIFS)
+		return -ENFILE;
+
+	mfc_ext = kzalloc(sizeof(*mfc_ext) +
+			   sizeof(mfc_ext->mfcc_ttls[0]) * ext_maxvifs,
+			   GFP_KERNEL);
+	if (!mfc_ext)
+		return -ENOMEM;
+	memcpy(mfc_ext, mfc, sizeof(*mfc_ext));
+	for (i = 0; i < MAXVIFS; i++)
+		mfc_ext->mfcc_ttls[i] = mfc->mfcc_ttls[i];
+	/* Prevent processing ttls for vifs over MAXVIFS. Also vif above MAXVIFS
+	 * shouldn't exist. So, ipmr_update_thresholds() will not process.
+	 */
+	for (; i < ext_maxvifs; i++)
+		mfc_ext->mfcc_ttls[i] = 0;
+
+	ret = ipmr_mfc_add_ext(net, mrt, mfc_ext, mrtsock, parent);
+	kfree(mfc_ext);
+	return ret;
+}
+
 /* Close the multicast socket, and clear the vif tables etc */
 static void mroute_clean_tables(struct mr_table *mrt, int flags)
 {
@@ -1348,8 +1378,11 @@ static void mrtsock_destruct(struct sock *sk)
 int ip_mroute_setsockopt(struct sock *sk, int optname, sockptr_t optval,
 			 unsigned int optlen)
 {
+	unsigned int ext_maxvifs = CONFIG_IP_MROUTE_EXT_MAXVIFS;
 	struct net *net = sock_net(sk);
 	int val, ret = 0, parent = 0;
+	struct mfcctl_ext *mfc_ext;
+	unsigned int mfc_ext_size;
 	struct mr_table *mrt;
 	struct vifctl vif;
 	struct mfcctl mfc;
@@ -1413,6 +1446,7 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, sockptr_t optval,
 		break;
 	case MRT_ADD_VIF:
 	case MRT_DEL_VIF:
+	case MRT_ADD_VIF_EXT:
 		if (optlen != sizeof(vif)) {
 			ret = -EINVAL;
 			break;
@@ -1421,11 +1455,18 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, sockptr_t optval,
 			ret = -EFAULT;
 			break;
 		}
-		if (vif.vifc_vifi >= MAXVIFS) {
-			ret = -ENFILE;
-			break;
-		}
 		if (optname == MRT_ADD_VIF) {
+			if (vif.vifc_vifi >= MAXVIFS) {
+				ret = -ENFILE;
+				break;
+			}
+		} else {
+			if (vif.vifc_vifi >= CONFIG_IP_MROUTE_EXT_MAXVIFS) {
+				ret = -ENFILE;
+				break;
+			}
+		}
+		if (optname == MRT_ADD_VIF || optname == MRT_ADD_VIF_EXT) {
 			ret = vif_add(net, mrt, &vif,
 				      sk == rtnl_dereference(mrt->mroute_sk));
 		} else {
@@ -1458,6 +1499,38 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, sockptr_t optval,
 					   sk == rtnl_dereference(mrt->mroute_sk),
 					   parent);
 		break;
+	case MRT_ADD_MFC_EXT:
+		parent = -1;
+		fallthrough;
+	case MRT_ADD_MFC_PROXY_EXT:
+		if (optlen < sizeof(*mfc_ext)) {
+			ret = -EINVAL;
+			break;
+		}
+		/* If userspace passes less than CONFIG_IP_MROUTE_EXT_MAXVIFS
+		 * ttls, make sure to allocate to the max and initialize to
+		 * zeros. If userspace passes more, the rest of the code will
+		 * process up to CONFIG_IP_MROUTE_EXT_MAXVIFS. MRT_ADD_VIF_EXT
+		 * only accepts up to CONFIG_IP_MROUTE_EXT_MAXVIFS
+		 */
+		mfc_ext_size = (unsigned int)(sizeof(*mfc_ext) +
+			       sizeof(mfc_ext->mfcc_ttls[0]) * ext_maxvifs);
+		mfc_ext = kzalloc(max(optlen, mfc_ext_size), GFP_KERNEL);
+		if (!mfc_ext) {
+			ret = -ENOMEM;
+			break;
+		}
+		if (copy_from_sockptr(mfc_ext, optval, optlen)) {
+			ret = -EFAULT;
+			break;
+		}
+		if (parent == 0)
+			parent = mfc_ext->mfcc_parent;
+		ret = ipmr_mfc_add_ext(net, mrt, mfc_ext,
+				       sk == rtnl_dereference(mrt->mroute_sk),
+				       parent);
+		kfree(mfc_ext);
+		break;
 	case MRT_FLUSH:
 		if (optlen != sizeof(val)) {
 			ret = -EINVAL;
@@ -2369,13 +2442,15 @@ static size_t mroute_msgsize(bool unresolved, int maxvif)
 static void mroute_netlink_event(struct mr_table *mrt, struct mfc_cache *mfc,
 				 int cmd)
 {
+	bool unresolved = mfc->_c.mfc_parent >= CONFIG_IP_MROUTE_EXT_MAXVIFS;
 	struct net *net = read_pnet(&mrt->net);
 	struct sk_buff *skb;
 	int err = -ENOBUFS;
 
-	skb = nlmsg_new(mroute_msgsize(mfc->_c.mfc_parent >= MAXVIFS,
-				       mrt->maxvif),
-			GFP_ATOMIC);
+	/* Unresolved cache has the mfc_parent being set to -1, so checking
+	 * with CONFIG_IP_MROUTE_EXT_MAXVIFS should be ok.
+	 */
+	skb = nlmsg_new(mroute_msgsize(unresolved, mrt->maxvif), GFP_ATOMIC);
 	if (!skb)
 		goto errout;
 
@@ -2619,14 +2694,14 @@ static bool ipmr_rtm_validate_proto(unsigned char rtm_protocol)
 	return false;
 }
 
-static int ipmr_nla_get_ttls(const struct nlattr *nla, struct mfcctl *mfcc)
+static int ipmr_nla_get_ttls(const struct nlattr *nla, struct mfcctl_ext *mfcc)
 {
 	struct rtnexthop *rtnh = nla_data(nla);
 	int remaining = nla_len(nla), vifi = 0;
 
 	while (rtnh_ok(rtnh, remaining)) {
 		mfcc->mfcc_ttls[vifi] = rtnh->rtnh_hops;
-		if (++vifi == MAXVIFS)
+		if (++vifi == CONFIG_IP_MROUTE_EXT_MAXVIFS)
 			break;
 		rtnh = rtnh_next(rtnh, &remaining);
 	}
@@ -2636,7 +2711,7 @@ static int ipmr_nla_get_ttls(const struct nlattr *nla, struct mfcctl *mfcc)
 
 /* returns < 0 on error, 0 for ADD_MFC and 1 for ADD_MFC_PROXY */
 static int rtm_to_ipmr_mfcc(struct net *net, struct nlmsghdr *nlh,
-			    struct mfcctl *mfcc, int *mrtsock,
+			    struct mfcctl_ext *mfcc, int *mrtsock,
 			    struct mr_table **mrtret,
 			    struct netlink_ext_ack *extack)
 {
@@ -2713,7 +2788,7 @@ static int ipmr_rtm_route(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct net *net = sock_net(skb->sk);
 	int ret, mrtsock, parent;
 	struct mr_table *tbl;
-	struct mfcctl mfcc;
+	struct mfcctl_ext mfcc;
 
 	mrtsock = 0;
 	tbl = NULL;
@@ -2723,9 +2798,9 @@ static int ipmr_rtm_route(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	parent = ret ? mfcc.mfcc_parent : -1;
 	if (nlh->nlmsg_type == RTM_NEWROUTE)
-		return ipmr_mfc_add(net, tbl, &mfcc, mrtsock, parent);
+		return ipmr_mfc_add_ext(net, tbl, &mfcc, mrtsock, parent);
 	else
-		return ipmr_mfc_delete(tbl, &mfcc, parent);
+		return ipmr_mfc_delete(tbl, (struct mfcctl *)&mfcc, parent);
 }
 
 static bool ipmr_fill_table(struct mr_table *mrt, struct sk_buff *skb)
@@ -3106,6 +3181,8 @@ int __init ip_mr_init(void)
 {
 	int err;
 
+	BUILD_BUG_ON(CONFIG_IP_MROUTE_EXT_MAXVIFS < MAXVIFS);
+
 	mrt_cachep = kmem_cache_create("ip_mrt_cache",
 				       sizeof(struct mfc_cache),
 				       0, SLAB_HWCACHE_ALIGN | SLAB_PANIC,
diff --git a/net/ipv4/ipmr_base.c b/net/ipv4/ipmr_base.c
index aa8738a91210..ce109643ce7b 100644
--- a/net/ipv4/ipmr_base.c
+++ b/net/ipv4/ipmr_base.c
@@ -215,7 +215,7 @@ int mr_fill_mroute(struct mr_table *mrt, struct sk_buff *skb,
 	int ct;
 
 	/* If cache is unresolved, don't try to parse IIF and OIF */
-	if (c->mfc_parent >= MAXVIFS) {
+	if (c->mfc_parent >= CONFIG_IP_MROUTE_EXT_MAXVIFS) {
 		rtm->rtm_flags |= RTNH_F_UNRESOLVED;
 		return -ENOENT;
 	}
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 36ed9efb8825..1310a896191e 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -50,6 +50,8 @@
 
 #include <linux/nospec.h>
 
+#define EXT_MAXMIFS CONFIG_IP_MROUTE_EXT_MAXVIFS
+
 struct ip6mr_rule {
 	struct fib_rule		common;
 };
@@ -839,9 +841,9 @@ static void ip6mr_update_thresholds(struct mr_table *mrt,
 {
 	int vifi;
 
-	cache->mfc_un.res.minvif = MAXMIFS;
+	cache->mfc_un.res.minvif = EXT_MAXMIFS;
 	cache->mfc_un.res.maxvif = 0;
-	memset(cache->mfc_un.res.ttls, 255, MAXMIFS);
+	memset(cache->mfc_un.res.ttls, 255, EXT_MAXMIFS);
 
 	for (vifi = 0; vifi < mrt->maxvif; vifi++) {
 		if (VIF_EXISTS(mrt, vifi) &&
@@ -980,7 +982,7 @@ static struct mfc6_cache *ip6mr_cache_alloc(void)
 	if (!c)
 		return NULL;
 	c->_c.mfc_un.res.last_assert = jiffies - MFC_ASSERT_THRESH - 1;
-	c->_c.mfc_un.res.minvif = MAXMIFS;
+	c->_c.mfc_un.res.minvif = EXT_MAXMIFS;
 	c->_c.free = ip6mr_cache_free_rcu;
 	refcount_set(&c->_c.mfc_un.res.refcount, 1);
 	return c;
@@ -1303,6 +1305,9 @@ static int __net_init ip6mr_net_init(struct net *net)
 {
 	int err;
 
+	BUILD_BUG_ON(EXT_MAXMIFS < MAXMIFS);
+	BUILD_BUG_ON(EXT_MAXMIFS > IF_SETSIZE);
+
 	err = ip6mr_notifier_init(net);
 	if (err)
 		return err;
@@ -1405,17 +1410,18 @@ void ip6_mr_cleanup(void)
 static int ip6mr_mfc_add(struct net *net, struct mr_table *mrt,
 			 struct mf6cctl *mfc, int mrtsock, int parent)
 {
-	unsigned char ttls[MAXMIFS];
+	unsigned char ttls[EXT_MAXMIFS];
 	struct mfc6_cache *uc, *c;
 	struct mr_mfc *_uc;
 	bool found;
 	int i, err;
 
-	if (mfc->mf6cc_parent >= MAXMIFS)
+	/* Check for the case of >= MAXMIFS is done outside of this func */
+	if (mfc->mf6cc_parent >= EXT_MAXMIFS)
 		return -ENFILE;
 
-	memset(ttls, 255, MAXMIFS);
-	for (i = 0; i < MAXMIFS; i++) {
+	memset(ttls, 255, EXT_MAXMIFS);
+	for (i = 0; i < EXT_MAXMIFS; i++) {
 		if (IF_ISSET(i, &mfc->mf6cc_ifset))
 			ttls[i] = 1;
 	}
@@ -1663,12 +1669,18 @@ int ip6_mroute_setsockopt(struct sock *sk, int optname, sockptr_t optval,
 		return ip6mr_sk_done(sk);
 
 	case MRT6_ADD_MIF:
+	case MRT6_ADD_MIF_EXT:
 		if (optlen < sizeof(vif))
 			return -EINVAL;
 		if (copy_from_sockptr(&vif, optval, sizeof(vif)))
 			return -EFAULT;
-		if (vif.mif6c_mifi >= MAXMIFS)
-			return -ENFILE;
+		if (optname == MRT6_ADD_MIF) {
+			if (vif.mif6c_mifi >= MAXMIFS)
+				return -ENFILE;
+		} else {
+			if (vif.mif6c_mifi >= EXT_MAXMIFS)
+				return -ENFILE;
+		}
 		rtnl_lock();
 		ret = mif6_add(net, mrt, &vif,
 			       sk == rtnl_dereference(mrt->mroute_sk));
@@ -1690,10 +1702,12 @@ int ip6_mroute_setsockopt(struct sock *sk, int optname, sockptr_t optval,
 	 *	in a sort of kernel/user symbiosis.
 	 */
 	case MRT6_ADD_MFC:
+	case MRT6_ADD_MFC_EXT:
 	case MRT6_DEL_MFC:
 		parent = -1;
 		fallthrough;
 	case MRT6_ADD_MFC_PROXY:
+	case MRT6_ADD_MFC_PROXY_EXT:
 	case MRT6_DEL_MFC_PROXY:
 		if (optlen < sizeof(mfc))
 			return -EINVAL;
@@ -1702,13 +1716,19 @@ int ip6_mroute_setsockopt(struct sock *sk, int optname, sockptr_t optval,
 		if (parent == 0)
 			parent = mfc.mf6cc_parent;
 		rtnl_lock();
-		if (optname == MRT6_DEL_MFC || optname == MRT6_DEL_MFC_PROXY)
+		if (optname == MRT6_DEL_MFC || optname == MRT6_DEL_MFC_PROXY) {
 			ret = ip6mr_mfc_delete(mrt, &mfc, parent);
-		else
-			ret = ip6mr_mfc_add(net, mrt, &mfc,
-					    sk ==
-					    rtnl_dereference(mrt->mroute_sk),
-					    parent);
+		} else {
+			if ((optname == MRT6_ADD_MFC ||
+			     optname == MRT6_ADD_MFC_PROXY) &&
+			    mfc.mf6cc_parent >= MAXMIFS)
+				ret = -ENFILE;
+			else
+				ret = ip6mr_mfc_add(net, mrt, &mfc,
+						    sk ==
+						    rtnl_dereference(mrt->mroute_sk),
+						    parent);
+		}
 		rtnl_unlock();
 		return ret;
 
@@ -2402,7 +2422,7 @@ static void mr6_netlink_event(struct mr_table *mrt, struct mfc6_cache *mfc,
 	struct sk_buff *skb;
 	int err = -ENOBUFS;
 
-	skb = nlmsg_new(mr6_msgsize(mfc->_c.mfc_parent >= MAXMIFS, mrt->maxvif),
+	skb = nlmsg_new(mr6_msgsize(mfc->_c.mfc_parent >= EXT_MAXMIFS, mrt->maxvif),
 			GFP_ATOMIC);
 	if (!skb)
 		goto errout;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH net-next] ipmr: ip6mr: APIs to support adding more than MAXVIFS/MAXMIFS
  2021-09-17 22:41 [RFC PATCH net-next] ipmr: ip6mr: APIs to support adding more than MAXVIFS/MAXMIFS Stephen Suryaputra
@ 2021-09-18  3:10 ` kernel test robot
  2021-09-19  1:07 ` Andrew Lunn
  1 sibling, 0 replies; 5+ messages in thread
From: kernel test robot @ 2021-09-18  3:10 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3935 bytes --]

Hi Stephen,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Stephen-Suryaputra/ipmr-ip6mr-APIs-to-support-adding-more-than-MAXVIFS-MAXMIFS/20210918-064706
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git af54faab84f754ebd42ecdda871f8d71940ae40b
config: um-x86_64_defconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/a569861a106aab4e715ba5e5ed82db0c20c2ca92
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Stephen-Suryaputra/ipmr-ip6mr-APIs-to-support-adding-more-than-MAXVIFS-MAXMIFS/20210918-064706
        git checkout a569861a106aab4e715ba5e5ed82db0c20c2ca92
        # save the attached .config to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=um SUBARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/mroute.h:10,
                    from net/ipv4/raw.c:51:
>> include/linux/mroute_base.h:141:23: error: 'CONFIG_IP_MROUTE_EXT_MAXVIFS' undeclared here (not in a function)
     141 |    unsigned char ttls[CONFIG_IP_MROUTE_EXT_MAXVIFS];
         |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
   In file included from include/linux/mroute.h:10,
                    from net/ipv4/route.c:83:
>> include/linux/mroute_base.h:141:23: error: 'CONFIG_IP_MROUTE_EXT_MAXVIFS' undeclared here (not in a function)
     141 |    unsigned char ttls[CONFIG_IP_MROUTE_EXT_MAXVIFS];
         |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/ipv4/route.c: In function 'ip_rt_send_redirect':
   net/ipv4/route.c:877:6: warning: variable 'log_martians' set but not used [-Wunused-but-set-variable]
     877 |  int log_martians;
         |      ^~~~~~~~~~~~


vim +/CONFIG_IP_MROUTE_EXT_MAXVIFS +141 include/linux/mroute_base.h

   102	
   103	/**
   104	 * struct mr_mfc - common multicast routing entries
   105	 * @mnode: rhashtable list
   106	 * @mfc_parent: source interface (iif)
   107	 * @mfc_flags: entry flags
   108	 * @expires: unresolved entry expire time
   109	 * @unresolved: unresolved cached skbs
   110	 * @last_assert: time of last assert
   111	 * @minvif: minimum VIF id
   112	 * @maxvif: maximum VIF id
   113	 * @bytes: bytes that have passed for this entry
   114	 * @pkt: packets that have passed for this entry
   115	 * @wrong_if: number of wrong source interface hits
   116	 * @lastuse: time of last use of the group (traffic or update)
   117	 * @ttls: OIF TTL threshold array
   118	 * @refcount: reference count for this entry
   119	 * @list: global entry list
   120	 * @rcu: used for entry destruction
   121	 * @free: Operation used for freeing an entry under RCU
   122	 */
   123	struct mr_mfc {
   124		struct rhlist_head mnode;
   125		unsigned short mfc_parent;
   126		int mfc_flags;
   127	
   128		union {
   129			struct {
   130				unsigned long expires;
   131				struct sk_buff_head unresolved;
   132			} unres;
   133			struct {
   134				unsigned long last_assert;
   135				int minvif;
   136				int maxvif;
   137				unsigned long bytes;
   138				unsigned long pkt;
   139				unsigned long wrong_if;
   140				unsigned long lastuse;
 > 141				unsigned char ttls[CONFIG_IP_MROUTE_EXT_MAXVIFS];
   142				refcount_t refcount;
   143			} res;
   144		} mfc_un;
   145		struct list_head list;
   146		struct rcu_head	rcu;
   147		void (*free)(struct rcu_head *head);
   148	};
   149	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 9645 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH net-next] ipmr: ip6mr: APIs to support adding more than MAXVIFS/MAXMIFS
  2021-09-17 22:41 [RFC PATCH net-next] ipmr: ip6mr: APIs to support adding more than MAXVIFS/MAXMIFS Stephen Suryaputra
  2021-09-18  3:10 ` kernel test robot
@ 2021-09-19  1:07 ` Andrew Lunn
  2021-09-20 18:24   ` Stephen Suryaputra
  1 sibling, 1 reply; 5+ messages in thread
From: Andrew Lunn @ 2021-09-19  1:07 UTC (permalink / raw)
  To: Stephen Suryaputra; +Cc: netdev

On Fri, Sep 17, 2021 at 06:41:23PM -0400, Stephen Suryaputra wrote:
> MAXVIFS and MAXMIFS are too small (32) for certain applications. But
> they are defined in user header files  So, use a different definition
> CONFIG_IP_MROUTE_EXT_MAXVIFS that is configurable and different ioctl
> requests (MRT_xyz_EXT and MRT6_xyz_EXT) as well as a different structure
> for adding MFC (mfcctl_ext).
> 
> CONFIG_IP_MROUTE_EXT_MAXVIFS is bounded by the IF_SETSIZE (256) in
> mroute6.h.
> 
> This patch is extending the following RFC:
> http://patchwork.ozlabs.org/project/netdev/patch/m1eiis8uc6.fsf@fess.ebiederm.org/

Quoting the above URL:

> My goal is an API that works with just a recompile of existing
> applications, and an ABI that continues to work for old applications.

Does this really work? Does the distribution version of mrouted use
the kernel UAPI headers of the running kernel? Can i upgrade to a
newer kernel, with newer headers, and it automagically pulls in a new
mrouted built using the new kernel headers? I think not. ethtool has
its own copy of the kernel headers. mrouted uses
/usr/include/linux/mroute.h which is provided by
linux-libc-dev:amd64. That is not tied to the running kernel. What
about quagga?

So in effect, you have to ask the running kernel, what value is it
using for MAXVIFS? Which means it is much more than just a recompile.
So i doubt think you can achieve this goal.

Given that, i really think you should spend the time to do a proper
solution. Add a netlink based API, which does not have the 32 limit.
Make the kernel implementation be based on a linked list. Have the
ioctl interface simply return the first 32 entries and ignore anything
above that.

      Andrew

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH net-next] ipmr: ip6mr: APIs to support adding more than MAXVIFS/MAXMIFS
  2021-09-19  1:07 ` Andrew Lunn
@ 2021-09-20 18:24   ` Stephen Suryaputra
  2021-09-20 19:05     ` Andrew Lunn
  0 siblings, 1 reply; 5+ messages in thread
From: Stephen Suryaputra @ 2021-09-20 18:24 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev

On Sun, Sep 19, 2021 at 03:07:34AM +0200, Andrew Lunn wrote:
> On Fri, Sep 17, 2021 at 06:41:23PM -0400, Stephen Suryaputra wrote:
> > MAXVIFS and MAXMIFS are too small (32) for certain applications. But
> > they are defined in user header files  So, use a different definition
> > CONFIG_IP_MROUTE_EXT_MAXVIFS that is configurable and different ioctl
> > requests (MRT_xyz_EXT and MRT6_xyz_EXT) as well as a different structure
> > for adding MFC (mfcctl_ext).
> > 
> > CONFIG_IP_MROUTE_EXT_MAXVIFS is bounded by the IF_SETSIZE (256) in
> > mroute6.h.
> > 
> > This patch is extending the following RFC:
> > http://patchwork.ozlabs.org/project/netdev/patch/m1eiis8uc6.fsf@fess.ebiederm.org/
> 
> Quoting the above URL:
> 
> > My goal is an API that works with just a recompile of existing
> > applications, and an ABI that continues to work for old applications.
> 
> Does this really work? Does the distribution version of mrouted use
> the kernel UAPI headers of the running kernel? Can i upgrade to a
> newer kernel, with newer headers, and it automagically pulls in a new
> mrouted built using the new kernel headers? I think not. ethtool has
> its own copy of the kernel headers. mrouted uses
> /usr/include/linux/mroute.h which is provided by
> linux-libc-dev:amd64. That is not tied to the running kernel. What
> about quagga?

That particular goal by Eric isn't exactly my goal. I extended his
approach to be more inline with the latest feedback he got. My
application is written for an embedded router and for it
/usr/include/linux/mroute.h is coming from the
include/uapi/linux/mroute.h. So, the new structure mfcctl_ext can be
used by the application.
> 
> So in effect, you have to ask the running kernel, what value is it
> using for MAXVIFS? Which means it is much more than just a recompile.
> So i doubt think you can achieve this goal.
> 
> Given that, i really think you should spend the time to do a proper
> solution. Add a netlink based API, which does not have the 32 limit.
> Make the kernel implementation be based on a linked list. Have the
> ioctl interface simply return the first 32 entries and ignore anything
> above that.

This proposal doesn't change any existing ones such as MRT_ADD_MFC,
MRT_ADD_VIF, MRT6_ADD_MFC and MRT6_ADD_MIF as they are still using the
unchanged MAXVIFS. So, if the applications such as quagga still use the
existing mroute.h it should still be working with the 32 vifs
limitation.

To use more than 32 vifs, then MRT_ADD_MFC_EXT, etc can be used. But for
that the applications need to be modified and be using the updated
mroute.h and mroute6.h.

Regards,
Stephen.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH net-next] ipmr: ip6mr: APIs to support adding more than MAXVIFS/MAXMIFS
  2021-09-20 18:24   ` Stephen Suryaputra
@ 2021-09-20 19:05     ` Andrew Lunn
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Lunn @ 2021-09-20 19:05 UTC (permalink / raw)
  To: Stephen Suryaputra; +Cc: netdev

> That particular goal by Eric isn't exactly my goal. I extended his
> approach to be more inline with the latest feedback he got. My
> application is written for an embedded router and for it
> /usr/include/linux/mroute.h is coming from the
> include/uapi/linux/mroute.h. So, the new structure mfcctl_ext can be
> used by the application.

Hi Stephan

That however is not the general case. Any new API you add needs to
support the general case, not just work for you. This needs to work
for Debian, Redhat, OpenWRT, Yocto etc, where often a copy of the
kernel headers are used.

> This proposal doesn't change any existing ones such as MRT_ADD_MFC,
> MRT_ADD_VIF, MRT6_ADD_MFC and MRT6_ADD_MIF as they are still using the
> unchanged MAXVIFS. So, if the applications such as quagga still use the
> existing mroute.h it should still be working with the 32 vifs
> limitation.

Agreed, you have not broken the existing code. But you have also not
added something which is a good way forward for quagga, mrouted etc,
to support arbitrary number of VIFS. I doubt the community will allow
this sort of band aid, which works for you, but not many others. They
will want a proper generic solution.

     Andrew

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-09-21  1:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-17 22:41 [RFC PATCH net-next] ipmr: ip6mr: APIs to support adding more than MAXVIFS/MAXMIFS Stephen Suryaputra
2021-09-18  3:10 ` kernel test robot
2021-09-19  1:07 ` Andrew Lunn
2021-09-20 18:24   ` Stephen Suryaputra
2021-09-20 19:05     ` Andrew Lunn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.