* [PATCH net-next v2 1/9] net: ipmr: move the tbl id check in ipmr_new_table
2015-11-21 14:57 [PATCH net-next v2 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
@ 2015-11-21 14:57 ` Nikolay Aleksandrov
2015-11-21 14:57 ` [PATCH net-next v2 2/9] net: ipmr: always define mroute_reg_vif_num Nikolay Aleksandrov
` (8 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 14:57 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, 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] 17+ messages in thread* [PATCH net-next v2 2/9] net: ipmr: always define mroute_reg_vif_num
2015-11-21 14:57 [PATCH net-next v2 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
2015-11-21 14:57 ` [PATCH net-next v2 1/9] net: ipmr: move the tbl id check in ipmr_new_table Nikolay Aleksandrov
@ 2015-11-21 14:57 ` Nikolay Aleksandrov
2015-11-23 5:23 ` Cong Wang
2015-11-21 14:57 ` [PATCH net-next v2 3/9] net: ipmr: remove some pimsm ifdefs and simplify Nikolay Aleksandrov
` (7 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 14:57 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, 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] 17+ messages in thread* Re: [PATCH net-next v2 2/9] net: ipmr: always define mroute_reg_vif_num
2015-11-21 14:57 ` [PATCH net-next v2 2/9] net: ipmr: always define mroute_reg_vif_num Nikolay Aleksandrov
@ 2015-11-23 5:23 ` Cong Wang
2015-11-23 10:39 ` Nikolay Aleksandrov
0 siblings, 1 reply; 17+ messages in thread
From: Cong Wang @ 2015-11-23 5:23 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, David Miller, Eric Dumazet, Nikolay Aleksandrov
On Sat, Nov 21, 2015 at 6:57 AM, Nikolay Aleksandrov
<razor@blackwall.org> wrote:
> 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.
>
Not sure if this really simplifies the code, since now
mroute_reg_vif_num is hidden
deeper after your patch and there are still some code under CONFIG_IP_PIMSM.
If you really care about it, how about introducing a helper function
to set and get
mrt->mroute_reg_vif_num?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 2/9] net: ipmr: always define mroute_reg_vif_num
2015-11-23 5:23 ` Cong Wang
@ 2015-11-23 10:39 ` Nikolay Aleksandrov
0 siblings, 0 replies; 17+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-23 10:39 UTC (permalink / raw)
To: Cong Wang, Nikolay Aleksandrov; +Cc: netdev, David Miller, Eric Dumazet
On 11/23/2015 06:23 AM, Cong Wang wrote:
> On Sat, Nov 21, 2015 at 6:57 AM, Nikolay Aleksandrov
> <razor@blackwall.org> wrote:
>> 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.
>>
>
> Not sure if this really simplifies the code, since now
> mroute_reg_vif_num is hidden
> deeper after your patch and there are still some code under CONFIG_IP_PIMSM.
>
CONFIG_IP_PIMSM is removed in the next patch, and it's not "hidden" anymore
than it was before.
> If you really care about it, how about introducing a helper function
> to set and get
> mrt->mroute_reg_vif_num?
>
Patches are welcome, if you don't get to it then I will with the next
set.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next v2 3/9] net: ipmr: remove some pimsm ifdefs and simplify
2015-11-21 14:57 [PATCH net-next v2 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
2015-11-21 14:57 ` [PATCH net-next v2 1/9] net: ipmr: move the tbl id check in ipmr_new_table Nikolay Aleksandrov
2015-11-21 14:57 ` [PATCH net-next v2 2/9] net: ipmr: always define mroute_reg_vif_num Nikolay Aleksandrov
@ 2015-11-21 14:57 ` Nikolay Aleksandrov
2015-11-21 14:57 ` [PATCH net-next v2 4/9] net: ipmr: fix code and comment style Nikolay Aleksandrov
` (6 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 14:57 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, 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] 17+ messages in thread* [PATCH net-next v2 4/9] net: ipmr: fix code and comment style
2015-11-21 14:57 [PATCH net-next v2 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
` (2 preceding siblings ...)
2015-11-21 14:57 ` [PATCH net-next v2 3/9] net: ipmr: remove some pimsm ifdefs and simplify Nikolay Aleksandrov
@ 2015-11-21 14:57 ` Nikolay Aleksandrov
2015-11-23 5:30 ` Cong Wang
2015-11-21 14:57 ` [PATCH net-next v2 5/9] net: ipmr: make ip_mroute_getsockopt more understandable Nikolay Aleksandrov
` (5 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 14:57 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, 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] 17+ messages in thread* Re: [PATCH net-next v2 4/9] net: ipmr: fix code and comment style
2015-11-21 14:57 ` [PATCH net-next v2 4/9] net: ipmr: fix code and comment style Nikolay Aleksandrov
@ 2015-11-23 5:30 ` Cong Wang
2015-11-23 10:40 ` Nikolay Aleksandrov
0 siblings, 1 reply; 17+ messages in thread
From: Cong Wang @ 2015-11-23 5:30 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, David Miller, Eric Dumazet, Nikolay Aleksandrov
On Sat, Nov 21, 2015 at 6:57 AM, Nikolay Aleksandrov
<razor@blackwall.org> wrote:
> -
> -/*
> - * Setup for IP multicast routing
> - */
> +/* Setup for IP multicast routing */
> static int __net_init ipmr_net_init(struct net *net)
Comments like this one are never useful so can be just removed.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 4/9] net: ipmr: fix code and comment style
2015-11-23 5:30 ` Cong Wang
@ 2015-11-23 10:40 ` Nikolay Aleksandrov
0 siblings, 0 replies; 17+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-23 10:40 UTC (permalink / raw)
To: Cong Wang, Nikolay Aleksandrov; +Cc: netdev, David Miller, Eric Dumazet
On 11/23/2015 06:30 AM, Cong Wang wrote:
> On Sat, Nov 21, 2015 at 6:57 AM, Nikolay Aleksandrov
> <razor@blackwall.org> wrote:
>> -
>> -/*
>> - * Setup for IP multicast routing
>> - */
>> +/* Setup for IP multicast routing */
>> static int __net_init ipmr_net_init(struct net *net)
>
> Comments like this one are never useful so can be just removed.
>
Will do, this is not the point of the patch thought. There're also
incorrect comments that need to be changed or removed. All in
due time, this is just a small trivial step to abide the style.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next v2 5/9] net: ipmr: make ip_mroute_getsockopt more understandable
2015-11-21 14:57 [PATCH net-next v2 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
` (3 preceding siblings ...)
2015-11-21 14:57 ` [PATCH net-next v2 4/9] net: ipmr: fix code and comment style Nikolay Aleksandrov
@ 2015-11-21 14:57 ` Nikolay Aleksandrov
2015-11-21 14:57 ` [PATCH net-next v2 6/9] net: ipmr: drop an instance of CONFIG_IP_MROUTE_MULTIPLE_TABLES Nikolay Aleksandrov
` (4 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 14:57 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, 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] 17+ messages in thread* [PATCH net-next v2 6/9] net: ipmr: drop an instance of CONFIG_IP_MROUTE_MULTIPLE_TABLES
2015-11-21 14:57 [PATCH net-next v2 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
` (4 preceding siblings ...)
2015-11-21 14:57 ` [PATCH net-next v2 5/9] net: ipmr: make ip_mroute_getsockopt more understandable Nikolay Aleksandrov
@ 2015-11-21 14:57 ` Nikolay Aleksandrov
2015-11-21 14:57 ` [PATCH net-next v2 7/9] net: ipmr: drop ip_mr_init() mrt_cachep null check as we'll panic if it fails Nikolay Aleksandrov
` (3 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 14:57 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, 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] 17+ messages in thread* [PATCH net-next v2 7/9] net: ipmr: drop ip_mr_init() mrt_cachep null check as we'll panic if it fails
2015-11-21 14:57 [PATCH net-next v2 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
` (5 preceding siblings ...)
2015-11-21 14:57 ` [PATCH net-next v2 6/9] net: ipmr: drop an instance of CONFIG_IP_MROUTE_MULTIPLE_TABLES Nikolay Aleksandrov
@ 2015-11-21 14:57 ` Nikolay Aleksandrov
2015-11-21 14:57 ` [PATCH net-next v2 8/9] net: ipmr: rearrange and cleanup setsockopt Nikolay Aleksandrov
` (2 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 14:57 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
It's not necessary to check for null as SLAB_PANIC is used and we'll
panic if the alloc fails, so just drop it.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: new patch, keep SLAB_PANIC and drop the unnecessary null check
net/ipv4/ipmr.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index a006d96d6cd9..50aec313119d 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2677,8 +2677,6 @@ int __init ip_mr_init(void)
sizeof(struct mfc_cache),
0, SLAB_HWCACHE_ALIGN | SLAB_PANIC,
NULL);
- if (!mrt_cachep)
- return -ENOMEM;
err = register_pernet_subsys(&ipmr_net_ops);
if (err)
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH net-next v2 8/9] net: ipmr: rearrange and cleanup setsockopt
2015-11-21 14:57 [PATCH net-next v2 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
` (6 preceding siblings ...)
2015-11-21 14:57 ` [PATCH net-next v2 7/9] net: ipmr: drop ip_mr_init() mrt_cachep null check as we'll panic if it fails Nikolay Aleksandrov
@ 2015-11-21 14:57 ` Nikolay Aleksandrov
2015-11-23 5:44 ` Cong Wang
2015-11-21 14:57 ` [PATCH net-next v2 9/9] net: ipmr: factor out common vif init code Nikolay Aleksandrov
2015-11-23 20:07 ` [PATCH net-next v2 0/9] net: ipmr: cleanups and minor improvements David Miller
9 siblings, 1 reply; 17+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 14:57 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, 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 50aec313119d..e384f39202cb 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] 17+ messages in thread* Re: [PATCH net-next v2 8/9] net: ipmr: rearrange and cleanup setsockopt
2015-11-21 14:57 ` [PATCH net-next v2 8/9] net: ipmr: rearrange and cleanup setsockopt Nikolay Aleksandrov
@ 2015-11-23 5:44 ` Cong Wang
2015-11-23 10:49 ` Nikolay Aleksandrov
0 siblings, 1 reply; 17+ messages in thread
From: Cong Wang @ 2015-11-23 5:44 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, David Miller, Eric Dumazet, Nikolay Aleksandrov
On Sat, Nov 21, 2015 at 6:57 AM, Nikolay Aleksandrov
<razor@blackwall.org> wrote:
> net/ipv4/ipmr.c | 191 +++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 107 insertions(+), 84 deletions(-)
Does this really simplify the code? :-/
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 8/9] net: ipmr: rearrange and cleanup setsockopt
2015-11-23 5:44 ` Cong Wang
@ 2015-11-23 10:49 ` Nikolay Aleksandrov
0 siblings, 0 replies; 17+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-23 10:49 UTC (permalink / raw)
To: Cong Wang, Nikolay Aleksandrov; +Cc: netdev, David Miller, Eric Dumazet
On 11/23/2015 06:44 AM, Cong Wang wrote:
> On Sat, Nov 21, 2015 at 6:57 AM, Nikolay Aleksandrov
> <razor@blackwall.org> wrote:
>> net/ipv4/ipmr.c | 191 +++++++++++++++++++++++++++++++-------------------------
>> 1 file changed, 107 insertions(+), 84 deletions(-)
>
> Does this really simplify the code? :-/
>
Did I really say it does ? :-) Now, to the point it just makes it
much easier to reason about this setsockopt which was doing conditional
locking in some of the cases before, and some were left out, also "v"
was sometimes signed and sometimes unsigned, it's clearer now which type
is used. I've left a comment why the only special case needs to unlock
rtnl (MRT_DONE).
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next v2 9/9] net: ipmr: factor out common vif init code
2015-11-21 14:57 [PATCH net-next v2 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
` (7 preceding siblings ...)
2015-11-21 14:57 ` [PATCH net-next v2 8/9] net: ipmr: rearrange and cleanup setsockopt Nikolay Aleksandrov
@ 2015-11-21 14:57 ` Nikolay Aleksandrov
2015-11-23 20:07 ` [PATCH net-next v2 0/9] net: ipmr: cleanups and minor improvements David Miller
9 siblings, 0 replies; 17+ messages in thread
From: Nikolay Aleksandrov @ 2015-11-21 14:57 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, 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 e384f39202cb..a2d248d9c35c 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] 17+ messages in thread* Re: [PATCH net-next v2 0/9] net: ipmr: cleanups and minor improvements
2015-11-21 14:57 [PATCH net-next v2 0/9] net: ipmr: cleanups and minor improvements Nikolay Aleksandrov
` (8 preceding siblings ...)
2015-11-21 14:57 ` [PATCH net-next v2 9/9] net: ipmr: factor out common vif init code Nikolay Aleksandrov
@ 2015-11-23 20:07 ` David Miller
9 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2015-11-23 20:07 UTC (permalink / raw)
To: razor; +Cc: netdev, eric.dumazet, nikolay
From: Nikolay Aleksandrov <razor@blackwall.org>
Date: Sat, 21 Nov 2015 15:57:23 +0100
> Since I'll have to work with ipmr, I decided to clean it up and do some
> minor improvements. Functionally there're almost no changes except the
> SLAB_PANIC removal. Most of the patches just re-design some functions to
> be clearer and more concise and try to remove the ifdef web that was
> inside. There's more information in each commit. This is the first set,
> the end goal is to introduce complete netlink support and control over
> the mfc and vif devices.
> I've tried to test all of the setsockopt/getsockopt options, and also
> made builds with various ipmr kconfig options turned on and off.
>
> v2: change patch 7 to keep SLAB_PANIC and just drop the unnecessary null
> check
This series is pretty much fine, applied, thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread