* [PATCH net-next 1/9] net: ipmr: move the tbl id check in ipmr_new_table
2015-11-21 13:01 [PATCH net-next 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
@ 2015-11-21 13:01 ` Nikolay Aleksandrov
2015-11-21 13:01 ` [PATCH net-next 2/9] net: ipmr: always define mroute_reg_vif_num Nikolay Aleksandrov
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 13:01 UTC (permalink / raw)
To: netdev; +Cc: davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Move the table id check in ipmr_new_table and make it return error
pointer. We need this change for the upcoming netlink table manipulation
support in order to avoid code duplication and a race condition.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/ipv4/ipmr.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 92dd4b74d513..5271e2eee110 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -252,8 +252,8 @@ static int __net_init ipmr_rules_init(struct net *net)
INIT_LIST_HEAD(&net->ipv4.mr_tables);
mrt = ipmr_new_table(net, RT_TABLE_DEFAULT);
- if (!mrt) {
- err = -ENOMEM;
+ if (IS_ERR(mrt)) {
+ err = PTR_ERR(mrt);
goto err1;
}
@@ -301,8 +301,13 @@ static int ipmr_fib_lookup(struct net *net, struct flowi4 *flp4,
static int __net_init ipmr_rules_init(struct net *net)
{
- net->ipv4.mrt = ipmr_new_table(net, RT_TABLE_DEFAULT);
- return net->ipv4.mrt ? 0 : -ENOMEM;
+ struct mr_table *mrt;
+
+ mrt = ipmr_new_table(net, RT_TABLE_DEFAULT);
+ if (IS_ERR(mrt))
+ return PTR_ERR(mrt);
+ net->ipv4.mrt = mrt;
+ return 0;
}
static void __net_exit ipmr_rules_exit(struct net *net)
@@ -319,13 +324,17 @@ static struct mr_table *ipmr_new_table(struct net *net, u32 id)
struct mr_table *mrt;
unsigned int i;
+ /* "pimreg%u" should not exceed 16 bytes (IFNAMSIZ) */
+ if (id != RT_TABLE_DEFAULT && id >= 1000000000)
+ return ERR_PTR(-EINVAL);
+
mrt = ipmr_get_table(net, id);
if (mrt)
return mrt;
mrt = kzalloc(sizeof(*mrt), GFP_KERNEL);
if (!mrt)
- return NULL;
+ return ERR_PTR(-ENOMEM);
write_pnet(&mrt->net, net);
mrt->id = id;
@@ -1407,17 +1416,14 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
if (get_user(v, (u32 __user *)optval))
return -EFAULT;
- /* "pimreg%u" should not exceed 16 bytes (IFNAMSIZ) */
- if (v != RT_TABLE_DEFAULT && v >= 1000000000)
- return -EINVAL;
-
rtnl_lock();
ret = 0;
if (sk == rtnl_dereference(mrt->mroute_sk)) {
ret = -EBUSY;
} else {
- if (!ipmr_new_table(net, v))
- ret = -ENOMEM;
+ mrt = ipmr_new_table(net, v);
+ if (IS_ERR(mrt))
+ ret = PTR_ERR(mrt);
else
raw_sk(sk)->ipmr_table = v;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 2/9] net: ipmr: always define mroute_reg_vif_num
2015-11-21 13:01 [PATCH net-next 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
2015-11-21 13:01 ` [PATCH net-next 1/9] net: ipmr: move the tbl id check in ipmr_new_table Nikolay Aleksandrov
@ 2015-11-21 13:01 ` Nikolay Aleksandrov
2015-11-21 13:01 ` [PATCH net-next 3/9] net: ipmr: remove some pimsm ifdefs and simplify Nikolay Aleksandrov
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 13:01 UTC (permalink / raw)
To: netdev; +Cc: davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Before mroute_reg_vif_num was defined only if any of the CONFIG_PIMSM_
options were set, but that's not really necessary as the size of the
struct is the same in both cases (checked with pahole, both cases size
is 3256 bytes) and we can remove some unnecessary ifdefs to simplify the
code.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/ipv4/ipmr.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 5271e2eee110..dd2462f70d34 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -84,9 +84,7 @@ struct mr_table {
atomic_t cache_resolve_queue_len;
bool mroute_do_assert;
bool mroute_do_pim;
-#if defined(CONFIG_IP_PIMSM_V1) || defined(CONFIG_IP_PIMSM_V2)
int mroute_reg_vif_num;
-#endif
};
struct ipmr_rule {
@@ -347,9 +345,7 @@ static struct mr_table *ipmr_new_table(struct net *net, u32 id)
setup_timer(&mrt->ipmr_expire_timer, ipmr_expire_process,
(unsigned long)mrt);
-#ifdef CONFIG_IP_PIMSM
mrt->mroute_reg_vif_num = -1;
-#endif
#ifdef CONFIG_IP_MROUTE_MULTIPLE_TABLES
list_add_tail_rcu(&mrt->list, &net->ipv4.mr_tables);
#endif
@@ -584,10 +580,8 @@ static int vif_delete(struct mr_table *mrt, int vifi, int notify,
return -EADDRNOTAVAIL;
}
-#ifdef CONFIG_IP_PIMSM
if (vifi == mrt->mroute_reg_vif_num)
mrt->mroute_reg_vif_num = -1;
-#endif
if (vifi + 1 == mrt->maxvif) {
int tmp;
@@ -824,10 +818,8 @@ static int vif_add(struct net *net, struct mr_table *mrt,
/* And finish update writing critical data */
write_lock_bh(&mrt_lock);
v->dev = dev;
-#ifdef CONFIG_IP_PIMSM
if (v->flags & VIFF_REGISTER)
mrt->mroute_reg_vif_num = vifi;
-#endif
if (vifi+1 > mrt->maxvif)
mrt->maxvif = vifi+1;
write_unlock_bh(&mrt_lock);
--
2.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 3/9] net: ipmr: remove some pimsm ifdefs and simplify
2015-11-21 13:01 [PATCH net-next 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
2015-11-21 13:01 ` [PATCH net-next 1/9] net: ipmr: move the tbl id check in ipmr_new_table Nikolay Aleksandrov
2015-11-21 13:01 ` [PATCH net-next 2/9] net: ipmr: always define mroute_reg_vif_num Nikolay Aleksandrov
@ 2015-11-21 13:01 ` Nikolay Aleksandrov
2015-11-21 13:01 ` [PATCH net-next 4/9] net: ipmr: fix code and comment style Nikolay Aleksandrov
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 13:01 UTC (permalink / raw)
To: netdev; +Cc: davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Add the helper pimsm_enabled() which replaces the old CONFIG_IP_PIMSM
define and is used to check if any version of PIM-SM has been enabled.
Use a single if defined(CONFIG_IP_PIMSM_V1) || defined(CONFIG_IP_PIMSM_V2)
for the pim-sm shared code. This is okay w.r.t IGMPMSG_WHOLEPKT because
only a VIFF_REGISTER device can send such packet, and it can't be
created if pimsm_enabled() is false.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/ipv4/ipmr.c | 180 ++++++++++++++++++++++++++------------------------------
1 file changed, 84 insertions(+), 96 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index dd2462f70d34..e153ab7b17a1 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -67,10 +67,6 @@
#include <net/fib_rules.h>
#include <linux/netconf.h>
-#if defined(CONFIG_IP_PIMSM_V1) || defined(CONFIG_IP_PIMSM_V2)
-#define CONFIG_IP_PIMSM 1
-#endif
-
struct mr_table {
struct list_head list;
possible_net_t net;
@@ -95,6 +91,11 @@ struct ipmr_result {
struct mr_table *mrt;
};
+static inline bool pimsm_enabled(void)
+{
+ return IS_BUILTIN(CONFIG_IP_PIMSM_V1) || IS_BUILTIN(CONFIG_IP_PIMSM_V2);
+}
+
/* Big lock, protecting vif table, mrt cache and mroute socket state.
* Note that the changes are semaphored via rtnl_lock.
*/
@@ -454,8 +455,7 @@ failure:
return NULL;
}
-#ifdef CONFIG_IP_PIMSM
-
+#if defined(CONFIG_IP_PIMSM_V1) || defined(CONFIG_IP_PIMSM_V2)
static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct net *net = dev_net(dev);
@@ -552,6 +552,51 @@ failure:
unregister_netdevice(dev);
return NULL;
}
+
+/* called with rcu_read_lock() */
+static int __pim_rcv(struct mr_table *mrt, struct sk_buff *skb,
+ unsigned int pimlen)
+{
+ struct net_device *reg_dev = NULL;
+ struct iphdr *encap;
+
+ encap = (struct iphdr *)(skb_transport_header(skb) + pimlen);
+ /*
+ * Check that:
+ * a. packet is really sent to a multicast group
+ * b. packet is not a NULL-REGISTER
+ * c. packet is not truncated
+ */
+ if (!ipv4_is_multicast(encap->daddr) ||
+ encap->tot_len == 0 ||
+ ntohs(encap->tot_len) + pimlen > skb->len)
+ return 1;
+
+ read_lock(&mrt_lock);
+ if (mrt->mroute_reg_vif_num >= 0)
+ reg_dev = mrt->vif_table[mrt->mroute_reg_vif_num].dev;
+ read_unlock(&mrt_lock);
+
+ if (!reg_dev)
+ return 1;
+
+ skb->mac_header = skb->network_header;
+ skb_pull(skb, (u8 *)encap - skb->data);
+ skb_reset_network_header(skb);
+ skb->protocol = htons(ETH_P_IP);
+ skb->ip_summed = CHECKSUM_NONE;
+
+ skb_tunnel_rx(skb, reg_dev, dev_net(reg_dev));
+
+ netif_rx(skb);
+
+ return NET_RX_SUCCESS;
+}
+#else
+static struct net_device *ipmr_reg_vif(struct net *net, struct mr_table *mrt)
+{
+ return NULL;
+}
#endif
/**
@@ -734,10 +779,10 @@ static int vif_add(struct net *net, struct mr_table *mrt,
return -EADDRINUSE;
switch (vifc->vifc_flags) {
-#ifdef CONFIG_IP_PIMSM
case VIFF_REGISTER:
- /*
- * Special Purpose VIF in PIM
+ if (!pimsm_enabled())
+ return -EINVAL;
+ /* Special Purpose VIF in PIM
* All the packets will be sent to the daemon
*/
if (mrt->mroute_reg_vif_num >= 0)
@@ -752,7 +797,6 @@ static int vif_add(struct net *net, struct mr_table *mrt,
return err;
}
break;
-#endif
case VIFF_TUNNEL:
dev = ipmr_new_tunnel(net, vifc);
if (!dev)
@@ -942,34 +986,29 @@ static void ipmr_cache_resolve(struct net *net, struct mr_table *mrt,
}
}
-/*
- * Bounce a cache query up to mrouted. We could use netlink for this but mrouted
- * expects the following bizarre scheme.
+/* Bounce a cache query up to mrouted. We could use netlink for this but mrouted
+ * expects the following bizarre scheme.
*
- * Called under mrt_lock.
+ * Called under mrt_lock.
*/
-
static int ipmr_cache_report(struct mr_table *mrt,
struct sk_buff *pkt, vifi_t vifi, int assert)
{
- struct sk_buff *skb;
const int ihl = ip_hdrlen(pkt);
+ struct sock *mroute_sk;
struct igmphdr *igmp;
struct igmpmsg *msg;
- struct sock *mroute_sk;
+ struct sk_buff *skb;
int ret;
-#ifdef CONFIG_IP_PIMSM
if (assert == IGMPMSG_WHOLEPKT)
skb = skb_realloc_headroom(pkt, sizeof(struct iphdr));
else
-#endif
skb = alloc_skb(128, GFP_ATOMIC);
if (!skb)
return -ENOBUFS;
-#ifdef CONFIG_IP_PIMSM
if (assert == IGMPMSG_WHOLEPKT) {
/* Ugly, but we have no choice with this interface.
* Duplicate old header, fix ihl, length etc.
@@ -987,28 +1026,23 @@ static int ipmr_cache_report(struct mr_table *mrt,
ip_hdr(skb)->ihl = sizeof(struct iphdr) >> 2;
ip_hdr(skb)->tot_len = htons(ntohs(ip_hdr(pkt)->tot_len) +
sizeof(struct iphdr));
- } else
-#endif
- {
-
- /* Copy the IP header */
-
- skb_set_network_header(skb, skb->len);
- skb_put(skb, ihl);
- skb_copy_to_linear_data(skb, pkt->data, ihl);
- ip_hdr(skb)->protocol = 0; /* Flag to the kernel this is a route add */
- msg = (struct igmpmsg *)skb_network_header(skb);
- msg->im_vif = vifi;
- skb_dst_set(skb, dst_clone(skb_dst(pkt)));
-
- /* Add our header */
-
- igmp = (struct igmphdr *)skb_put(skb, sizeof(struct igmphdr));
- igmp->type =
- msg->im_msgtype = assert;
- igmp->code = 0;
- ip_hdr(skb)->tot_len = htons(skb->len); /* Fix the length */
- skb->transport_header = skb->network_header;
+ } else {
+ /* Copy the IP header */
+ skb_set_network_header(skb, skb->len);
+ skb_put(skb, ihl);
+ skb_copy_to_linear_data(skb, pkt->data, ihl);
+ /* Flag to the kernel this is a route add */
+ ip_hdr(skb)->protocol = 0;
+ msg = (struct igmpmsg *)skb_network_header(skb);
+ msg->im_vif = vifi;
+ skb_dst_set(skb, dst_clone(skb_dst(pkt)));
+ /* Add our header */
+ igmp = (struct igmphdr *)skb_put(skb, sizeof(struct igmphdr));
+ igmp->type = assert;
+ msg->im_msgtype = assert;
+ igmp->code = 0;
+ ip_hdr(skb)->tot_len = htons(skb->len); /* Fix the length */
+ skb->transport_header = skb->network_header;
}
rcu_read_lock();
@@ -1020,7 +1054,6 @@ static int ipmr_cache_report(struct mr_table *mrt,
}
/* Deliver to mrouted */
-
ret = sock_queue_rcv_skb(mroute_sk, skb);
rcu_read_unlock();
if (ret < 0) {
@@ -1377,11 +1410,12 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
mrt->mroute_do_assert = v;
return 0;
}
-#ifdef CONFIG_IP_PIMSM
case MRT_PIM:
{
int v;
+ if (!pimsm_enabled())
+ return -ENOPROTOOPT;
if (optlen != sizeof(v))
return -EINVAL;
if (get_user(v, (int __user *)optval))
@@ -1397,7 +1431,6 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
rtnl_unlock();
return ret;
}
-#endif
#ifdef CONFIG_IP_MROUTE_MULTIPLE_TABLES
case MRT_TABLE:
{
@@ -1452,9 +1485,7 @@ int ip_mroute_getsockopt(struct sock *sk, int optname, char __user *optval, int
return -ENOENT;
if (optname != MRT_VERSION &&
-#ifdef CONFIG_IP_PIMSM
optname != MRT_PIM &&
-#endif
optname != MRT_ASSERT)
return -ENOPROTOOPT;
@@ -1467,14 +1498,15 @@ int ip_mroute_getsockopt(struct sock *sk, int optname, char __user *optval, int
if (put_user(olr, optlen))
return -EFAULT;
- if (optname == MRT_VERSION)
+ if (optname == MRT_VERSION) {
val = 0x0305;
-#ifdef CONFIG_IP_PIMSM
- else if (optname == MRT_PIM)
+ } else if (optname == MRT_PIM) {
+ if (!pimsm_enabled())
+ return -ENOPROTOOPT;
val = mrt->mroute_do_pim;
-#endif
- else
+ } else {
val = mrt->mroute_do_assert;
+ }
if (copy_to_user(optval, &val, olr))
return -EFAULT;
return 0;
@@ -1707,7 +1739,6 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,
if (!vif->dev)
goto out_free;
-#ifdef CONFIG_IP_PIMSM
if (vif->flags & VIFF_REGISTER) {
vif->pkt_out++;
vif->bytes_out += skb->len;
@@ -1716,7 +1747,6 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,
ipmr_cache_report(mrt, skb, vifi, IGMPMSG_WHOLEPKT);
goto out_free;
}
-#endif
if (vif->flags & VIFF_TUNNEL) {
rt = ip_route_output_ports(net, &fl4, NULL,
@@ -2047,48 +2077,6 @@ dont_forward:
return 0;
}
-#ifdef CONFIG_IP_PIMSM
-/* called with rcu_read_lock() */
-static int __pim_rcv(struct mr_table *mrt, struct sk_buff *skb,
- unsigned int pimlen)
-{
- struct net_device *reg_dev = NULL;
- struct iphdr *encap;
-
- encap = (struct iphdr *)(skb_transport_header(skb) + pimlen);
- /*
- * Check that:
- * a. packet is really sent to a multicast group
- * b. packet is not a NULL-REGISTER
- * c. packet is not truncated
- */
- if (!ipv4_is_multicast(encap->daddr) ||
- encap->tot_len == 0 ||
- ntohs(encap->tot_len) + pimlen > skb->len)
- return 1;
-
- read_lock(&mrt_lock);
- if (mrt->mroute_reg_vif_num >= 0)
- reg_dev = mrt->vif_table[mrt->mroute_reg_vif_num].dev;
- read_unlock(&mrt_lock);
-
- if (!reg_dev)
- return 1;
-
- skb->mac_header = skb->network_header;
- skb_pull(skb, (u8 *)encap - skb->data);
- skb_reset_network_header(skb);
- skb->protocol = htons(ETH_P_IP);
- skb->ip_summed = CHECKSUM_NONE;
-
- skb_tunnel_rx(skb, reg_dev, dev_net(reg_dev));
-
- netif_rx(skb);
-
- return NET_RX_SUCCESS;
-}
-#endif
-
#ifdef CONFIG_IP_PIMSM_V1
/*
* Handle IGMP messages of PIMv1
--
2.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 4/9] net: ipmr: fix code and comment style
2015-11-21 13:01 [PATCH net-next 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
` (2 preceding siblings ...)
2015-11-21 13:01 ` [PATCH net-next 3/9] net: ipmr: remove some pimsm ifdefs and simplify Nikolay Aleksandrov
@ 2015-11-21 13:01 ` Nikolay Aleksandrov
2015-11-21 13:01 ` [PATCH net-next 5/9] net: ipmr: make ip_mroute_getsockopt more understandable Nikolay Aleksandrov
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 13:01 UTC (permalink / raw)
To: netdev; +Cc: davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Trivial code and comment style fixes, also removed some extra newlines,
spaces and tabs.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
include/uapi/linux/mroute.h | 59 ++++++------------
net/ipv4/ipmr.c | 142 ++++++++++++--------------------------------
2 files changed, 54 insertions(+), 147 deletions(-)
diff --git a/include/uapi/linux/mroute.h b/include/uapi/linux/mroute.h
index a382d2c04a42..cf943016930f 100644
--- a/include/uapi/linux/mroute.h
+++ b/include/uapi/linux/mroute.h
@@ -4,15 +4,13 @@
#include <linux/sockios.h>
#include <linux/types.h>
-/*
- * Based on the MROUTING 3.5 defines primarily to keep
- * source compatibility with BSD.
+/* Based on the MROUTING 3.5 defines primarily to keep
+ * source compatibility with BSD.
*
- * See the mrouted code for the original history.
- *
- * Protocol Independent Multicast (PIM) data structures included
- * Carlos Picoto (cap@di.fc.ul.pt)
+ * See the mrouted code for the original history.
*
+ * Protocol Independent Multicast (PIM) data structures included
+ * Carlos Picoto (cap@di.fc.ul.pt)
*/
#define MRT_BASE 200
@@ -34,15 +32,13 @@
#define SIOCGETSGCNT (SIOCPROTOPRIVATE+1)
#define SIOCGETRPF (SIOCPROTOPRIVATE+2)
-#define MAXVIFS 32
+#define MAXVIFS 32
typedef unsigned long vifbitmap_t; /* User mode code depends on this lot */
typedef unsigned short vifi_t;
#define ALL_VIFS ((vifi_t)(-1))
-/*
- * Same idea as select
- */
-
+/* Same idea as select */
+
#define VIFM_SET(n,m) ((m)|=(1<<(n)))
#define VIFM_CLR(n,m) ((m)&=~(1<<(n)))
#define VIFM_ISSET(n,m) ((m)&(1<<(n)))
@@ -50,11 +46,9 @@ typedef unsigned short vifi_t;
#define VIFM_COPY(mfrom,mto) ((mto)=(mfrom))
#define VIFM_SAME(m1,m2) ((m1)==(m2))
-/*
- * Passed by mrouted for an MRT_ADD_VIF - again we use the
- * mrouted 3.6 structures for compatibility
+/* Passed by mrouted for an MRT_ADD_VIF - again we use the
+ * mrouted 3.6 structures for compatibility
*/
-
struct vifctl {
vifi_t vifc_vifi; /* Index of VIF */
unsigned char vifc_flags; /* VIFF_ flags */
@@ -73,10 +67,7 @@ struct vifctl {
#define VIFF_USE_IFINDEX 0x8 /* use vifc_lcl_ifindex instead of
vifc_lcl_addr to find an interface */
-/*
- * Cache manipulation structures for mrouted and PIMd
- */
-
+/* Cache manipulation structures for mrouted and PIMd */
struct mfcctl {
struct in_addr mfcc_origin; /* Origin of mcast */
struct in_addr mfcc_mcastgrp; /* Group in question */
@@ -88,10 +79,7 @@ struct mfcctl {
int mfcc_expire;
};
-/*
- * Group count retrieval for mrouted
- */
-
+/* Group count retrieval for mrouted */
struct sioc_sg_req {
struct in_addr src;
struct in_addr grp;
@@ -100,10 +88,7 @@ struct sioc_sg_req {
unsigned long wrong_if;
};
-/*
- * To get vif packet counts
- */
-
+/* To get vif packet counts */
struct sioc_vif_req {
vifi_t vifi; /* Which iface */
unsigned long icount; /* In packets */
@@ -112,11 +97,9 @@ struct sioc_vif_req {
unsigned long obytes; /* Out bytes */
};
-/*
- * This is the format the mroute daemon expects to see IGMP control
- * data. Magically happens to be like an IP packet as per the original
+/* This is the format the mroute daemon expects to see IGMP control
+ * data. Magically happens to be like an IP packet as per the original
*/
-
struct igmpmsg {
__u32 unused1,unused2;
unsigned char im_msgtype; /* What is this */
@@ -126,21 +109,13 @@ struct igmpmsg {
struct in_addr im_src,im_dst;
};
-/*
- * That's all usermode folks
- */
-
-
+/* That's all usermode folks */
#define MFC_ASSERT_THRESH (3*HZ) /* Maximal freq. of asserts */
-/*
- * Pseudo messages used by mrouted
- */
-
+/* Pseudo messages used by mrouted */
#define IGMPMSG_NOCACHE 1 /* Kern cache fill request to mrouted */
#define IGMPMSG_WRONGVIF 2 /* For PIM assert processing (unused) */
#define IGMPMSG_WHOLEPKT 3 /* For PIM Register processing */
-
#endif /* _UAPI__LINUX_MROUTE_H */
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index e153ab7b17a1..286ede3716ee 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -102,9 +102,7 @@ static inline bool pimsm_enabled(void)
static DEFINE_RWLOCK(mrt_lock);
-/*
- * Multicast router control variables
- */
+/* Multicast router control variables */
#define VIF_EXISTS(_mrt, _idx) ((_mrt)->vif_table[_idx].dev != NULL)
@@ -393,8 +391,7 @@ static void ipmr_del_tunnel(struct net_device *dev, struct vifctl *v)
}
}
-static
-struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
+static struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
{
struct net_device *dev;
@@ -561,8 +558,7 @@ static int __pim_rcv(struct mr_table *mrt, struct sk_buff *skb,
struct iphdr *encap;
encap = (struct iphdr *)(skb_transport_header(skb) + pimlen);
- /*
- * Check that:
+ /* Check that:
* a. packet is really sent to a multicast group
* b. packet is not a NULL-REGISTER
* c. packet is not truncated
@@ -603,7 +599,6 @@ static struct net_device *ipmr_reg_vif(struct net *net, struct mr_table *mrt)
* vif_delete - Delete a VIF entry
* @notify: Set to 1, if the caller is a notifier_call
*/
-
static int vif_delete(struct mr_table *mrt, int vifi, int notify,
struct list_head *head)
{
@@ -673,7 +668,6 @@ static inline void ipmr_cache_free(struct mfc_cache *c)
/* Destroy an unresolved cache entry, killing queued skbs
* and reporting error to netlink readers.
*/
-
static void ipmr_destroy_unres(struct mr_table *mrt, struct mfc_cache *c)
{
struct net *net = read_pnet(&mrt->net);
@@ -701,9 +695,7 @@ static void ipmr_destroy_unres(struct mr_table *mrt, struct mfc_cache *c)
ipmr_cache_free(c);
}
-
/* Timer process for the unresolved queue. */
-
static void ipmr_expire_process(unsigned long arg)
{
struct mr_table *mrt = (struct mr_table *)arg;
@@ -743,7 +735,6 @@ out:
}
/* Fill oifs list. It is called under write locked mrt_lock. */
-
static void ipmr_update_thresholds(struct mr_table *mrt, struct mfc_cache *cache,
unsigned char *ttls)
{
@@ -808,7 +799,6 @@ static int vif_add(struct net *net, struct mr_table *mrt,
return err;
}
break;
-
case VIFF_USE_IFINDEX:
case 0:
if (vifc->vifc_flags == VIFF_USE_IFINDEX) {
@@ -928,9 +918,7 @@ skip:
return ipmr_cache_find_any_parent(mrt, vifi);
}
-/*
- * Allocate a multicast cache entry
- */
+/* Allocate a multicast cache entry */
static struct mfc_cache *ipmr_cache_alloc(void)
{
struct mfc_cache *c = kmem_cache_zalloc(mrt_cachep, GFP_KERNEL);
@@ -951,10 +939,7 @@ static struct mfc_cache *ipmr_cache_alloc_unres(void)
return c;
}
-/*
- * A cache entry has gone into a resolved state from queued
- */
-
+/* A cache entry has gone into a resolved state from queued */
static void ipmr_cache_resolve(struct net *net, struct mr_table *mrt,
struct mfc_cache *uc, struct mfc_cache *c)
{
@@ -962,7 +947,6 @@ static void ipmr_cache_resolve(struct net *net, struct mr_table *mrt,
struct nlmsgerr *e;
/* Play the pending entries through our router */
-
while ((skb = __skb_dequeue(&uc->mfc_un.unres.unresolved))) {
if (ip_hdr(skb)->version == 0) {
struct nlmsghdr *nlh = (struct nlmsghdr *)skb_pull(skb, sizeof(struct iphdr));
@@ -1064,12 +1048,9 @@ static int ipmr_cache_report(struct mr_table *mrt,
return ret;
}
-/*
- * Queue a packet for resolution. It gets locked cache entry!
- */
-
-static int
-ipmr_cache_unresolved(struct mr_table *mrt, vifi_t vifi, struct sk_buff *skb)
+/* Queue a packet for resolution. It gets locked cache entry! */
+static int ipmr_cache_unresolved(struct mr_table *mrt, vifi_t vifi,
+ struct sk_buff *skb)
{
bool found = false;
int err;
@@ -1087,7 +1068,6 @@ ipmr_cache_unresolved(struct mr_table *mrt, vifi_t vifi, struct sk_buff *skb)
if (!found) {
/* Create a new entry if allowable */
-
if (atomic_read(&mrt->cache_resolve_queue_len) >= 10 ||
(c = ipmr_cache_alloc_unres()) == NULL) {
spin_unlock_bh(&mfc_unres_lock);
@@ -1097,13 +1077,11 @@ ipmr_cache_unresolved(struct mr_table *mrt, vifi_t vifi, struct sk_buff *skb)
}
/* Fill in the new cache entry */
-
c->mfc_parent = -1;
c->mfc_origin = iph->saddr;
c->mfc_mcastgrp = iph->daddr;
/* Reflect first query at mrouted. */
-
err = ipmr_cache_report(mrt, skb, vifi, IGMPMSG_NOCACHE);
if (err < 0) {
/* If the report failed throw the cache entry
@@ -1125,7 +1103,6 @@ ipmr_cache_unresolved(struct mr_table *mrt, vifi_t vifi, struct sk_buff *skb)
}
/* See if we can append the packet */
-
if (c->mfc_un.unres.unresolved.qlen > 3) {
kfree_skb(skb);
err = -ENOBUFS;
@@ -1138,9 +1115,7 @@ ipmr_cache_unresolved(struct mr_table *mrt, vifi_t vifi, struct sk_buff *skb)
return err;
}
-/*
- * MFC cache manipulation by user space mroute daemon
- */
+/* MFC cache manipulation by user space mroute daemon */
static int ipmr_mfc_delete(struct mr_table *mrt, struct mfcctl *mfc, int parent)
{
@@ -1211,9 +1186,8 @@ static int ipmr_mfc_add(struct net *net, struct mr_table *mrt,
list_add_rcu(&c->list, &mrt->mfc_cache_array[line]);
- /*
- * Check to see if we resolved a queued list. If so we
- * need to send on the frames and tidy up.
+ /* Check to see if we resolved a queued list. If so we
+ * need to send on the frames and tidy up.
*/
found = false;
spin_lock_bh(&mfc_unres_lock);
@@ -1238,10 +1212,7 @@ static int ipmr_mfc_add(struct net *net, struct mr_table *mrt,
return 0;
}
-/*
- * Close the multicast socket, and clear the vif tables etc
- */
-
+/* Close the multicast socket, and clear the vif tables etc */
static void mroute_clean_tables(struct mr_table *mrt)
{
int i;
@@ -1249,7 +1220,6 @@ static void mroute_clean_tables(struct mr_table *mrt)
struct mfc_cache *c, *next;
/* Shut down all active vif entries */
-
for (i = 0; i < mrt->maxvif; i++) {
if (!(mrt->vif_table[i].flags & VIFF_STATIC))
vif_delete(mrt, i, 0, &list);
@@ -1257,7 +1227,6 @@ static void mroute_clean_tables(struct mr_table *mrt)
unregister_netdevice_many(&list);
/* Wipe the cache */
-
for (i = 0; i < MFC_LINES; i++) {
list_for_each_entry_safe(c, next, &mrt->mfc_cache_array[i], list) {
if (c->mfc_flags & MFC_STATIC)
@@ -1301,11 +1270,10 @@ static void mrtsock_destruct(struct sock *sk)
rtnl_unlock();
}
-/*
- * Socket options and virtual interface manipulation. The whole
- * virtual interface system is a complete heap, but unfortunately
- * that's how BSD mrouted happens to think. Maybe one day with a proper
- * MOSPF/PIM router set up we can clean this up.
+/* Socket options and virtual interface manipulation. The whole
+ * virtual interface system is a complete heap, but unfortunately
+ * that's how BSD mrouted happens to think. Maybe one day with a proper
+ * MOSPF/PIM router set up we can clean this up.
*/
int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsigned int optlen)
@@ -1373,10 +1341,9 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
rtnl_unlock();
return ret;
- /*
- * Manipulate the forwarding caches. These live
- * in a sort of kernel/user symbiosis.
- */
+ /* Manipulate the forwarding caches. These live
+ * in a sort of kernel/user symbiosis.
+ */
case MRT_ADD_MFC:
case MRT_DEL_MFC:
parent = -1;
@@ -1397,9 +1364,7 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
parent);
rtnl_unlock();
return ret;
- /*
- * Control PIM assert.
- */
+ /* Control PIM assert. */
case MRT_ASSERT:
{
int v;
@@ -1456,19 +1421,13 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
return ret;
}
#endif
- /*
- * Spurious command, or MRT_VERSION which you cannot
- * set.
- */
+ /* Spurious command, or MRT_VERSION which you cannot set. */
default:
return -ENOPROTOOPT;
}
}
-/*
- * Getsock opt support for the multicast routing system.
- */
-
+/* Getsock opt support for the multicast routing system. */
int ip_mroute_getsockopt(struct sock *sk, int optname, char __user *optval, int __user *optlen)
{
int olr;
@@ -1512,10 +1471,7 @@ int ip_mroute_getsockopt(struct sock *sk, int optname, char __user *optval, int
return 0;
}
-/*
- * The IP multicast ioctl support routines.
- */
-
+/* The IP multicast ioctl support routines. */
int ipmr_ioctl(struct sock *sk, int cmd, void __user *arg)
{
struct sioc_sg_req sr;
@@ -1648,7 +1604,6 @@ int ipmr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
}
#endif
-
static int ipmr_device_event(struct notifier_block *this, unsigned long event, void *ptr)
{
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
@@ -1670,17 +1625,14 @@ static int ipmr_device_event(struct notifier_block *this, unsigned long event, v
return NOTIFY_DONE;
}
-
static struct notifier_block ip_mr_notifier = {
.notifier_call = ipmr_device_event,
};
-/*
- * Encapsulate a packet by attaching a valid IPIP header to it.
- * This avoids tunnel drivers and other mess and gives us the speed so
- * important for multicast video.
+/* Encapsulate a packet by attaching a valid IPIP header to it.
+ * This avoids tunnel drivers and other mess and gives us the speed so
+ * important for multicast video.
*/
-
static void ip_encap(struct net *net, struct sk_buff *skb,
__be32 saddr, __be32 daddr)
{
@@ -1722,9 +1674,7 @@ static inline int ipmr_forward_finish(struct net *net, struct sock *sk,
return dst_output(net, sk, skb);
}
-/*
- * Processing handlers for ipmr_forward
- */
+/* Processing handlers for ipmr_forward */
static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,
struct sk_buff *skb, struct mfc_cache *c, int vifi)
@@ -1773,7 +1723,6 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,
* allow to send ICMP, so that packets will disappear
* to blackhole.
*/
-
IP_INC_STATS(net, IPSTATS_MIB_FRAGFAILS);
ip_rt_put(rt);
goto out_free;
@@ -1805,8 +1754,7 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,
IPCB(skb)->flags |= IPSKB_FORWARDED;
- /*
- * RFC1584 teaches, that DVMRP/PIM router must deliver packets locally
+ /* RFC1584 teaches, that DVMRP/PIM router must deliver packets locally
* not only before forwarding, but after forwarding on all output
* interfaces. It is clear, if mrouter runs a multicasting
* program, it should receive packets not depending to what interface
@@ -1837,7 +1785,6 @@ static int ipmr_find_vif(struct mr_table *mrt, struct net_device *dev)
}
/* "local" means that we should preserve one skb (for local delivery) */
-
static void ip_mr_forward(struct net *net, struct mr_table *mrt,
struct sk_buff *skb, struct mfc_cache *cache,
int local)
@@ -1862,9 +1809,7 @@ static void ip_mr_forward(struct net *net, struct mr_table *mrt,
goto forward;
}
- /*
- * Wrong interface: drop packet and (maybe) send PIM assert.
- */
+ /* Wrong interface: drop packet and (maybe) send PIM assert. */
if (mrt->vif_table[vif].dev != skb->dev) {
if (rt_is_output_route(skb_rtable(skb))) {
/* It is our own packet, looped back.
@@ -1903,9 +1848,7 @@ forward:
mrt->vif_table[vif].pkt_in++;
mrt->vif_table[vif].bytes_in += skb->len;
- /*
- * Forward the frame
- */
+ /* Forward the frame */
if (cache->mfc_origin == htonl(INADDR_ANY) &&
cache->mfc_mcastgrp == htonl(INADDR_ANY)) {
if (true_vifi >= 0 &&
@@ -1979,11 +1922,9 @@ static struct mr_table *ipmr_rt_fib_lookup(struct net *net, struct sk_buff *skb)
return mrt;
}
-/*
- * Multicast packets for forwarding arrive here
- * Called with rcu_read_lock();
+/* Multicast packets for forwarding arrive here
+ * Called with rcu_read_lock();
*/
-
int ip_mr_input(struct sk_buff *skb)
{
struct mfc_cache *cache;
@@ -2034,9 +1975,7 @@ int ip_mr_input(struct sk_buff *skb)
vif);
}
- /*
- * No usable cache entry
- */
+ /* No usable cache entry */
if (!cache) {
int vif;
@@ -2078,10 +2017,7 @@ dont_forward:
}
#ifdef CONFIG_IP_PIMSM_V1
-/*
- * Handle IGMP messages of PIMv1
- */
-
+/* Handle IGMP messages of PIMv1 */
int pim_rcv_v1(struct sk_buff *skb)
{
struct igmphdr *pim;
@@ -2406,9 +2342,8 @@ done:
}
#ifdef CONFIG_PROC_FS
-/*
- * The /proc interfaces to multicast routing :
- * /proc/net/ip_mr_cache & /proc/net/ip_mr_vif
+/* The /proc interfaces to multicast routing :
+ * /proc/net/ip_mr_cache & /proc/net/ip_mr_vif
*/
struct ipmr_vif_iter {
struct seq_net_private p;
@@ -2692,10 +2627,7 @@ static const struct net_protocol pim_protocol = {
};
#endif
-
-/*
- * Setup for IP multicast routing
- */
+/* Setup for IP multicast routing */
static int __net_init ipmr_net_init(struct net *net)
{
int err;
--
2.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 5/9] net: ipmr: make ip_mroute_getsockopt more understandable
2015-11-21 13:01 [PATCH net-next 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
` (3 preceding siblings ...)
2015-11-21 13:01 ` [PATCH net-next 4/9] net: ipmr: fix code and comment style Nikolay Aleksandrov
@ 2015-11-21 13:01 ` Nikolay Aleksandrov
2015-11-21 13:01 ` [PATCH net-next 6/9] net: ipmr: drop an instance of CONFIG_IP_MROUTE_MULTIPLE_TABLES Nikolay Aleksandrov
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 13:01 UTC (permalink / raw)
To: netdev; +Cc: davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Use a switch to determine if optname is correct and set val accordingly.
This produces a much more straight-forward and readable code.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/ipv4/ipmr.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 286ede3716ee..694fecf7838e 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1443,29 +1443,29 @@ int ip_mroute_getsockopt(struct sock *sk, int optname, char __user *optval, int
if (!mrt)
return -ENOENT;
- if (optname != MRT_VERSION &&
- optname != MRT_PIM &&
- optname != MRT_ASSERT)
+ switch (optname) {
+ case MRT_VERSION:
+ val = 0x0305;
+ break;
+ case MRT_PIM:
+ if (!pimsm_enabled())
+ return -ENOPROTOOPT;
+ val = mrt->mroute_do_pim;
+ break;
+ case MRT_ASSERT:
+ val = mrt->mroute_do_assert;
+ break;
+ default:
return -ENOPROTOOPT;
+ }
if (get_user(olr, optlen))
return -EFAULT;
-
olr = min_t(unsigned int, olr, sizeof(int));
if (olr < 0)
return -EINVAL;
-
if (put_user(olr, optlen))
return -EFAULT;
- if (optname == MRT_VERSION) {
- val = 0x0305;
- } else if (optname == MRT_PIM) {
- if (!pimsm_enabled())
- return -ENOPROTOOPT;
- val = mrt->mroute_do_pim;
- } else {
- val = mrt->mroute_do_assert;
- }
if (copy_to_user(optval, &val, olr))
return -EFAULT;
return 0;
--
2.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 6/9] net: ipmr: drop an instance of CONFIG_IP_MROUTE_MULTIPLE_TABLES
2015-11-21 13:01 [PATCH net-next 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
` (4 preceding siblings ...)
2015-11-21 13:01 ` [PATCH net-next 5/9] net: ipmr: make ip_mroute_getsockopt more understandable Nikolay Aleksandrov
@ 2015-11-21 13:01 ` Nikolay Aleksandrov
2015-11-21 13:01 ` [PATCH net-next 7/9] net: ipmr: remove SLAB_PANIC Nikolay Aleksandrov
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 13:01 UTC (permalink / raw)
To: netdev; +Cc: davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Trivial replace of ifdef with IS_BUILTIN().
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/ipv4/ipmr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 694fecf7838e..a006d96d6cd9 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1396,11 +1396,12 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
rtnl_unlock();
return ret;
}
-#ifdef CONFIG_IP_MROUTE_MULTIPLE_TABLES
case MRT_TABLE:
{
u32 v;
+ if (!IS_BUILTIN(CONFIG_IP_MROUTE_MULTIPLE_TABLES))
+ return -ENOPROTOOPT;
if (optlen != sizeof(u32))
return -EINVAL;
if (get_user(v, (u32 __user *)optval))
@@ -1420,7 +1421,6 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
rtnl_unlock();
return ret;
}
-#endif
/* Spurious command, or MRT_VERSION which you cannot set. */
default:
return -ENOPROTOOPT;
--
2.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 7/9] net: ipmr: remove SLAB_PANIC
2015-11-21 13:01 [PATCH net-next 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
` (5 preceding siblings ...)
2015-11-21 13:01 ` [PATCH net-next 6/9] net: ipmr: drop an instance of CONFIG_IP_MROUTE_MULTIPLE_TABLES Nikolay Aleksandrov
@ 2015-11-21 13:01 ` Nikolay Aleksandrov
2015-11-21 14:23 ` Eric Dumazet
2015-11-21 13:01 ` [PATCH net-next 8/9] net: ipmr: rearrange and cleanup setsockopt Nikolay Aleksandrov
2015-11-21 13:01 ` [PATCH net-next 9/9] net: ipmr: factor out common vif init code Nikolay Aleksandrov
8 siblings, 1 reply; 12+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 13:01 UTC (permalink / raw)
To: netdev; +Cc: davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
It's not necessary to panic upon allocation failure, returning an error
at that point is okay because user-space won't be able to use any of the
ops since they didn't get registered and the default table is null.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/ipv4/ipmr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index a006d96d6cd9..2c7fa584a274 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2675,7 +2675,7 @@ int __init ip_mr_init(void)
mrt_cachep = kmem_cache_create("ip_mrt_cache",
sizeof(struct mfc_cache),
- 0, SLAB_HWCACHE_ALIGN | SLAB_PANIC,
+ 0, SLAB_HWCACHE_ALIGN,
NULL);
if (!mrt_cachep)
return -ENOMEM;
--
2.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next 7/9] net: ipmr: remove SLAB_PANIC
2015-11-21 13:01 ` [PATCH net-next 7/9] net: ipmr: remove SLAB_PANIC Nikolay Aleksandrov
@ 2015-11-21 14:23 ` Eric Dumazet
2015-11-21 14:27 ` Nikolay Aleksandrov
0 siblings, 1 reply; 12+ messages in thread
From: Eric Dumazet @ 2015-11-21 14:23 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, davem, Nikolay Aleksandrov
On Sat, 2015-11-21 at 14:01 +0100, Nikolay Aleksandrov wrote:
> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>
> It's not necessary to panic upon allocation failure, returning an error
> at that point is okay because user-space won't be able to use any of the
> ops since they didn't get registered and the default table is null.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> net/ipv4/ipmr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index a006d96d6cd9..2c7fa584a274 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -2675,7 +2675,7 @@ int __init ip_mr_init(void)
>
> mrt_cachep = kmem_cache_create("ip_mrt_cache",
> sizeof(struct mfc_cache),
> - 0, SLAB_HWCACHE_ALIGN | SLAB_PANIC,
> + 0, SLAB_HWCACHE_ALIGN,
> NULL);
> if (!mrt_cachep)
> return -ENOMEM;
This runs at boot time.
I very much prefer a panic instead of having to deal with a host with a
probable bug in mm layer at this point.
For IPv6, it is a different matter as it might be a module, thus
ip6_mr_init() does not use SLAB_PANIC
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net-next 7/9] net: ipmr: remove SLAB_PANIC
2015-11-21 14:23 ` Eric Dumazet
@ 2015-11-21 14:27 ` Nikolay Aleksandrov
0 siblings, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 14:27 UTC (permalink / raw)
To: Eric Dumazet, Nikolay Aleksandrov; +Cc: netdev, davem
On 11/21/2015 03:23 PM, Eric Dumazet wrote:
> On Sat, 2015-11-21 at 14:01 +0100, Nikolay Aleksandrov wrote:
>> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>>
>> It's not necessary to panic upon allocation failure, returning an error
>> at that point is okay because user-space won't be able to use any of the
>> ops since they didn't get registered and the default table is null.
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> ---
>> net/ipv4/ipmr.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
>> index a006d96d6cd9..2c7fa584a274 100644
>> --- a/net/ipv4/ipmr.c
>> +++ b/net/ipv4/ipmr.c
>> @@ -2675,7 +2675,7 @@ int __init ip_mr_init(void)
>>
>> mrt_cachep = kmem_cache_create("ip_mrt_cache",
>> sizeof(struct mfc_cache),
>> - 0, SLAB_HWCACHE_ALIGN | SLAB_PANIC,
>> + 0, SLAB_HWCACHE_ALIGN,
>> NULL);
>> if (!mrt_cachep)
>> return -ENOMEM;
>
> This runs at boot time.
>
> I very much prefer a panic instead of having to deal with a host with a
> probable bug in mm layer at this point.
>
Right, I was unsure about this one, I tried to rely on the ipmr_get_table() returning NULL but
I guess it's better to be safe than sorry.
I'll respin with the panic in place and removed null check.
> For IPv6, it is a different matter as it might be a module, thus
> ip6_mr_init() does not use SLAB_PANIC
Yep, I saw. It wasn't the reason I removed this one.
Thanks for the review!
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next 8/9] net: ipmr: rearrange and cleanup setsockopt
2015-11-21 13:01 [PATCH net-next 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
` (6 preceding siblings ...)
2015-11-21 13:01 ` [PATCH net-next 7/9] net: ipmr: remove SLAB_PANIC Nikolay Aleksandrov
@ 2015-11-21 13:01 ` Nikolay Aleksandrov
2015-11-21 13:01 ` [PATCH net-next 9/9] net: ipmr: factor out common vif init code Nikolay Aleksandrov
8 siblings, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 13:01 UTC (permalink / raw)
To: netdev; +Cc: davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Take rtnl in the beginning unconditionally as most options already need
it (one exception - MRT_DONE, see the comment inside), make the
lock/unlock places central and move out the switch() local variables.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/ipv4/ipmr.c | 191 +++++++++++++++++++++++++++++++-------------------------
1 file changed, 107 insertions(+), 84 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 2c7fa584a274..f2ea97fbb8a8 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1276,38 +1276,45 @@ static void mrtsock_destruct(struct sock *sk)
* MOSPF/PIM router set up we can clean this up.
*/
-int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsigned int optlen)
+int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval,
+ unsigned int optlen)
{
- int ret, parent = 0;
- struct vifctl vif;
- struct mfcctl mfc;
struct net *net = sock_net(sk);
+ int val, ret = 0, parent = 0;
struct mr_table *mrt;
+ struct vifctl vif;
+ struct mfcctl mfc;
+ u32 uval;
+ /* There's one exception to the lock - MRT_DONE which needs to unlock */
+ rtnl_lock();
if (sk->sk_type != SOCK_RAW ||
- inet_sk(sk)->inet_num != IPPROTO_IGMP)
- return -EOPNOTSUPP;
+ inet_sk(sk)->inet_num != IPPROTO_IGMP) {
+ ret = -EOPNOTSUPP;
+ goto out_unlock;
+ }
mrt = ipmr_get_table(net, raw_sk(sk)->ipmr_table ? : RT_TABLE_DEFAULT);
- if (!mrt)
- return -ENOENT;
-
+ if (!mrt) {
+ ret = -ENOENT;
+ goto out_unlock;
+ }
if (optname != MRT_INIT) {
if (sk != rcu_access_pointer(mrt->mroute_sk) &&
- !ns_capable(net->user_ns, CAP_NET_ADMIN))
- return -EACCES;
+ !ns_capable(net->user_ns, CAP_NET_ADMIN)) {
+ ret = -EACCES;
+ goto out_unlock;
+ }
}
switch (optname) {
case MRT_INIT:
if (optlen != sizeof(int))
- return -EINVAL;
-
- rtnl_lock();
- if (rtnl_dereference(mrt->mroute_sk)) {
- rtnl_unlock();
- return -EADDRINUSE;
- }
+ ret = -EINVAL;
+ if (rtnl_dereference(mrt->mroute_sk))
+ ret = -EADDRINUSE;
+ if (ret)
+ break;
ret = ip_ra_control(sk, 1, mrtsock_destruct);
if (ret == 0) {
@@ -1317,30 +1324,41 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
NETCONFA_IFINDEX_ALL,
net->ipv4.devconf_all);
}
- rtnl_unlock();
- return ret;
+ break;
case MRT_DONE:
- if (sk != rcu_access_pointer(mrt->mroute_sk))
- return -EACCES;
- return ip_ra_control(sk, 0, NULL);
+ if (sk != rcu_access_pointer(mrt->mroute_sk)) {
+ ret = -EACCES;
+ } else {
+ /* We need to unlock here because mrtsock_destruct takes
+ * care of rtnl itself and we can't change that due to
+ * the IP_ROUTER_ALERT setsockopt which runs without it.
+ */
+ rtnl_unlock();
+ ret = ip_ra_control(sk, 0, NULL);
+ goto out;
+ }
+ break;
case MRT_ADD_VIF:
case MRT_DEL_VIF:
- if (optlen != sizeof(vif))
- return -EINVAL;
- if (copy_from_user(&vif, optval, sizeof(vif)))
- return -EFAULT;
- if (vif.vifc_vifi >= MAXVIFS)
- return -ENFILE;
- rtnl_lock();
+ if (optlen != sizeof(vif)) {
+ ret = -EINVAL;
+ break;
+ }
+ if (copy_from_user(&vif, optval, sizeof(vif))) {
+ ret = -EFAULT;
+ break;
+ }
+ if (vif.vifc_vifi >= MAXVIFS) {
+ ret = -ENFILE;
+ break;
+ }
if (optname == MRT_ADD_VIF) {
ret = vif_add(net, mrt, &vif,
sk == rtnl_dereference(mrt->mroute_sk));
} else {
ret = vif_delete(mrt, vif.vifc_vifi, 0, NULL);
}
- rtnl_unlock();
- return ret;
-
+ break;
/* Manipulate the forwarding caches. These live
* in a sort of kernel/user symbiosis.
*/
@@ -1349,82 +1367,87 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
parent = -1;
case MRT_ADD_MFC_PROXY:
case MRT_DEL_MFC_PROXY:
- if (optlen != sizeof(mfc))
- return -EINVAL;
- if (copy_from_user(&mfc, optval, sizeof(mfc)))
- return -EFAULT;
+ if (optlen != sizeof(mfc)) {
+ ret = -EINVAL;
+ break;
+ }
+ if (copy_from_user(&mfc, optval, sizeof(mfc))) {
+ ret = -EFAULT;
+ break;
+ }
if (parent == 0)
parent = mfc.mfcc_parent;
- rtnl_lock();
if (optname == MRT_DEL_MFC || optname == MRT_DEL_MFC_PROXY)
ret = ipmr_mfc_delete(mrt, &mfc, parent);
else
ret = ipmr_mfc_add(net, mrt, &mfc,
sk == rtnl_dereference(mrt->mroute_sk),
parent);
- rtnl_unlock();
- return ret;
+ break;
/* Control PIM assert. */
case MRT_ASSERT:
- {
- int v;
- if (optlen != sizeof(v))
- return -EINVAL;
- if (get_user(v, (int __user *)optval))
- return -EFAULT;
- mrt->mroute_do_assert = v;
- return 0;
- }
+ if (optlen != sizeof(val)) {
+ ret = -EINVAL;
+ break;
+ }
+ if (get_user(val, (int __user *)optval)) {
+ ret = -EFAULT;
+ break;
+ }
+ mrt->mroute_do_assert = val;
+ break;
case MRT_PIM:
- {
- int v;
-
- if (!pimsm_enabled())
- return -ENOPROTOOPT;
- if (optlen != sizeof(v))
- return -EINVAL;
- if (get_user(v, (int __user *)optval))
- return -EFAULT;
- v = !!v;
+ if (!pimsm_enabled()) {
+ ret = -ENOPROTOOPT;
+ break;
+ }
+ if (optlen != sizeof(val)) {
+ ret = -EINVAL;
+ break;
+ }
+ if (get_user(val, (int __user *)optval)) {
+ ret = -EFAULT;
+ break;
+ }
- rtnl_lock();
- ret = 0;
- if (v != mrt->mroute_do_pim) {
- mrt->mroute_do_pim = v;
- mrt->mroute_do_assert = v;
+ val = !!val;
+ if (val != mrt->mroute_do_pim) {
+ mrt->mroute_do_pim = val;
+ mrt->mroute_do_assert = val;
}
- rtnl_unlock();
- return ret;
- }
+ break;
case MRT_TABLE:
- {
- u32 v;
-
- if (!IS_BUILTIN(CONFIG_IP_MROUTE_MULTIPLE_TABLES))
- return -ENOPROTOOPT;
- if (optlen != sizeof(u32))
- return -EINVAL;
- if (get_user(v, (u32 __user *)optval))
- return -EFAULT;
+ if (!IS_BUILTIN(CONFIG_IP_MROUTE_MULTIPLE_TABLES)) {
+ ret = -ENOPROTOOPT;
+ break;
+ }
+ if (optlen != sizeof(uval)) {
+ ret = -EINVAL;
+ break;
+ }
+ if (get_user(uval, (u32 __user *)optval)) {
+ ret = -EFAULT;
+ break;
+ }
- rtnl_lock();
- ret = 0;
if (sk == rtnl_dereference(mrt->mroute_sk)) {
ret = -EBUSY;
} else {
- mrt = ipmr_new_table(net, v);
+ mrt = ipmr_new_table(net, uval);
if (IS_ERR(mrt))
ret = PTR_ERR(mrt);
else
- raw_sk(sk)->ipmr_table = v;
+ raw_sk(sk)->ipmr_table = uval;
}
- rtnl_unlock();
- return ret;
- }
+ break;
/* Spurious command, or MRT_VERSION which you cannot set. */
default:
- return -ENOPROTOOPT;
+ ret = -ENOPROTOOPT;
}
+out_unlock:
+ rtnl_unlock();
+out:
+ return ret;
}
/* Getsock opt support for the multicast routing system. */
--
2.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 9/9] net: ipmr: factor out common vif init code
2015-11-21 13:01 [PATCH net-next 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
` (7 preceding siblings ...)
2015-11-21 13:01 ` [PATCH net-next 8/9] net: ipmr: rearrange and cleanup setsockopt Nikolay Aleksandrov
@ 2015-11-21 13:01 ` Nikolay Aleksandrov
8 siblings, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 13:01 UTC (permalink / raw)
To: netdev; +Cc: davem, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Factor out common vif init code used in both tunnel and pimreg
initialization and create ipmr_init_vif_indev() function.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/ipv4/ipmr.c | 40 +++++++++++++++++++---------------------
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index f2ea97fbb8a8..73361c908e5e 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -391,6 +391,23 @@ static void ipmr_del_tunnel(struct net_device *dev, struct vifctl *v)
}
}
+/* Initialize ipmr pimreg/tunnel in_device */
+static bool ipmr_init_vif_indev(const struct net_device *dev)
+{
+ struct in_device *in_dev;
+
+ ASSERT_RTNL();
+
+ in_dev = __in_dev_get_rtnl(dev);
+ if (!in_dev)
+ return false;
+ ipv4_devconf_setall(in_dev);
+ neigh_parms_data_state_setall(in_dev->arp_parms);
+ IPV4_DEVCONF(in_dev->cnf, RP_FILTER) = 0;
+
+ return true;
+}
+
static struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
{
struct net_device *dev;
@@ -402,7 +419,6 @@ static struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
int err;
struct ifreq ifr;
struct ip_tunnel_parm p;
- struct in_device *in_dev;
memset(&p, 0, sizeof(p));
p.iph.daddr = v->vifc_rmt_addr.s_addr;
@@ -427,15 +443,8 @@ static struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
if (err == 0 &&
(dev = __dev_get_by_name(net, p.name)) != NULL) {
dev->flags |= IFF_MULTICAST;
-
- in_dev = __in_dev_get_rtnl(dev);
- if (!in_dev)
+ if (!ipmr_init_vif_indev(dev))
goto failure;
-
- ipv4_devconf_setall(in_dev);
- neigh_parms_data_state_setall(in_dev->arp_parms);
- IPV4_DEVCONF(in_dev->cnf, RP_FILTER) = 0;
-
if (dev_open(dev))
goto failure;
dev_hold(dev);
@@ -502,7 +511,6 @@ static void reg_vif_setup(struct net_device *dev)
static struct net_device *ipmr_reg_vif(struct net *net, struct mr_table *mrt)
{
struct net_device *dev;
- struct in_device *in_dev;
char name[IFNAMSIZ];
if (mrt->id == RT_TABLE_DEFAULT)
@@ -522,18 +530,8 @@ static struct net_device *ipmr_reg_vif(struct net *net, struct mr_table *mrt)
return NULL;
}
- rcu_read_lock();
- in_dev = __in_dev_get_rcu(dev);
- if (!in_dev) {
- rcu_read_unlock();
+ if (!ipmr_init_vif_indev(dev))
goto failure;
- }
-
- ipv4_devconf_setall(in_dev);
- neigh_parms_data_state_setall(in_dev->arp_parms);
- IPV4_DEVCONF(in_dev->cnf, RP_FILTER) = 0;
- rcu_read_unlock();
-
if (dev_open(dev))
goto failure;
--
2.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread