2020-09-07 22:05:54

by Paul Davey

[permalink] [raw]
Subject: [PATCH net-next v2 0/3] Allow more than 255 IPv4 multicast interfaces

Currently it is not possible to use more than 255 multicast interfaces
for IPv4 due to the format of the igmpmsg header which only has 8 bits
available for the VIF ID. There is space available in the igmpmsg
header to store the full VIF ID in the form of an unused byte following
the VIF ID field. There is also enough space for the full VIF ID in
the Netlink cache notifications, however the value is currently taken
directly from the igmpmsg header and has thus already been truncated.

Adding the high byte of the VIF ID into the unused3 byte of igmpmsg
allows use of more than 255 IPv4 multicast interfaces. The full VIF ID
is also available in the Netlink notification by assembling it from
both bytes from the igmpmsg.

Additionally this reveals a deficiency in the Netlink cache report
notifications, they lack any means for differentiating cache reports
relating to different multicast routing tables. This is easily
resolved by adding the multicast route table ID to the cache reports.

changes in v2:
- Added high byte of VIF ID to igmpmsg struct replacing unused3
member.
- Assemble VIF ID in Netlink notification from both bytes in igmpmsg
header.

Paul Davey (3):
ipmr: Add route table ID to netlink cache reports
ipmr: Add high byte of VIF ID to igmpmsg
ipmr: Use full VIF ID in netlink cache reports

include/uapi/linux/mroute.h | 5 +++--
net/ipv4/ipmr.c | 14 ++++++++++----
2 files changed, 13 insertions(+), 6 deletions(-)

--
2.28.0


2020-09-07 22:06:36

by Paul Davey

[permalink] [raw]
Subject: [PATCH net-next v2 2/3] ipmr: Add high byte of VIF ID to igmpmsg

Use the unused3 byte in struct igmpmsg to hold the high 8 bits of the
VIF ID.

If using more than 255 IPv4 multicast interfaces it is necessary to have
access to a VIF ID for cache reports that is wider than 8 bits, the VIF
ID present in the igmpmsg reports sent to mroute_sk was only 8 bits wide
in the igmpmsg header. Adding the high 8 bits of the 16 bit VIF ID in
the unused byte allows use of more than 255 IPv4 multicast interfaces.

Signed-off-by: Paul Davey <[email protected]>
---
include/uapi/linux/mroute.h | 4 ++--
net/ipv4/ipmr.c | 8 ++++++--
2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/mroute.h b/include/uapi/linux/mroute.h
index 918f1ef32ffe..1a42f5f9b31b 100644
--- a/include/uapi/linux/mroute.h
+++ b/include/uapi/linux/mroute.h
@@ -113,8 +113,8 @@ struct igmpmsg {
__u32 unused1,unused2;
unsigned char im_msgtype; /* What is this */
unsigned char im_mbz; /* Must be zero */
- unsigned char im_vif; /* Interface (this ought to be a vifi_t!) */
- unsigned char unused3;
+ unsigned char im_vif; /* Low 8 bits of Interface */
+ unsigned char im_vif_hi; /* High 8 bits of Interface */
struct in_addr im_src,im_dst;
};

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 19b2f586319b..4809318f591b 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1038,10 +1038,13 @@ static int ipmr_cache_report(struct mr_table *mrt,
memcpy(msg, skb_network_header(pkt), sizeof(struct iphdr));
msg->im_msgtype = assert;
msg->im_mbz = 0;
- if (assert == IGMPMSG_WRVIFWHOLE)
+ if (assert == IGMPMSG_WRVIFWHOLE) {
msg->im_vif = vifi;
- else
+ msg->im_vif_hi = vifi >> 8;
+ } else {
msg->im_vif = mrt->mroute_reg_vif_num;
+ msg->im_vif_hi = mrt->mroute_reg_vif_num >> 8;
+ }
ip_hdr(skb)->ihl = sizeof(struct iphdr) >> 2;
ip_hdr(skb)->tot_len = htons(ntohs(ip_hdr(pkt)->tot_len) +
sizeof(struct iphdr));
@@ -1054,6 +1057,7 @@ static int ipmr_cache_report(struct mr_table *mrt,
ip_hdr(skb)->protocol = 0;
msg = (struct igmpmsg *)skb_network_header(skb);
msg->im_vif = vifi;
+ msg->im_vif_hi = vifi >> 8;
skb_dst_set(skb, dst_clone(skb_dst(pkt)));
/* Add our header */
igmp = skb_put(skb, sizeof(struct igmphdr));
--
2.28.0

2020-09-07 22:06:57

by Paul Davey

[permalink] [raw]
Subject: [PATCH net-next v2 3/3] ipmr: Use full VIF ID in netlink cache reports

Insert the full 16 bit VIF ID into ipmr Netlink cache reports.

The VIF_ID attribute has 32 bits of space so can store the full VIF ID
extracted from the high and low byte fields in the igmpmsg.

Signed-off-by: Paul Davey <[email protected]>
---
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 4809318f591b..939792a38814 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2432,7 +2432,7 @@ static void igmpmsg_netlink_event(struct mr_table *mrt, struct sk_buff *pkt)
rtgenm = nlmsg_data(nlh);
rtgenm->rtgen_family = RTNL_FAMILY_IPMR;
if (nla_put_u8(skb, IPMRA_CREPORT_MSGTYPE, msg->im_msgtype) ||
- nla_put_u32(skb, IPMRA_CREPORT_VIF_ID, msg->im_vif) ||
+ nla_put_u32(skb, IPMRA_CREPORT_VIF_ID, msg->im_vif | (msg->im_vif_hi << 8)) ||
nla_put_in_addr(skb, IPMRA_CREPORT_SRC_ADDR,
msg->im_src.s_addr) ||
nla_put_in_addr(skb, IPMRA_CREPORT_DST_ADDR,
--
2.28.0

2020-09-07 22:07:10

by Paul Davey

[permalink] [raw]
Subject: [PATCH net-next v2 1/3] ipmr: Add route table ID to netlink cache reports

Insert the multicast route table ID as a Netlink attribute to Netlink
cache report notifications.

When multiple route tables are in use it is necessary to have a way to
determine which route table a given cache report belongs to when
receiving the cache report.

Signed-off-by: Paul Davey <[email protected]>
---
include/uapi/linux/mroute.h | 1 +
net/ipv4/ipmr.c | 4 +++-
2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/mroute.h b/include/uapi/linux/mroute.h
index 11c8c1fc1124..918f1ef32ffe 100644
--- a/include/uapi/linux/mroute.h
+++ b/include/uapi/linux/mroute.h
@@ -169,6 +169,7 @@ enum {
IPMRA_CREPORT_SRC_ADDR,
IPMRA_CREPORT_DST_ADDR,
IPMRA_CREPORT_PKT,
+ IPMRA_CREPORT_TABLE,
__IPMRA_CREPORT_MAX
};
#define IPMRA_CREPORT_MAX (__IPMRA_CREPORT_MAX - 1)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 876fd6ff1ff9..19b2f586319b 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2396,6 +2396,7 @@ static size_t igmpmsg_netlink_msgsize(size_t payloadlen)
+ nla_total_size(4) /* IPMRA_CREPORT_VIF_ID */
+ nla_total_size(4) /* IPMRA_CREPORT_SRC_ADDR */
+ nla_total_size(4) /* IPMRA_CREPORT_DST_ADDR */
+ + nla_total_size(4) /* IPMRA_CREPORT_TABLE */
/* IPMRA_CREPORT_PKT */
+ nla_total_size(payloadlen)
;
@@ -2431,7 +2432,8 @@ static void igmpmsg_netlink_event(struct mr_table *mrt, struct sk_buff *pkt)
nla_put_in_addr(skb, IPMRA_CREPORT_SRC_ADDR,
msg->im_src.s_addr) ||
nla_put_in_addr(skb, IPMRA_CREPORT_DST_ADDR,
- msg->im_dst.s_addr))
+ msg->im_dst.s_addr) ||
+ nla_put_u32(skb, IPMRA_CREPORT_TABLE, mrt->id))
goto nla_put_failure;

nla = nla_reserve(skb, IPMRA_CREPORT_PKT, payloadlen);
--
2.28.0

2020-09-09 08:54:36

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH net-next v2 3/3] ipmr: Use full VIF ID in netlink cache reports

On Tue, Sep 08, 2020 at 10:04:08AM +1200, Paul Davey wrote:
> Insert the full 16 bit VIF ID into ipmr Netlink cache reports.
>
> The VIF_ID attribute has 32 bits of space so can store the full VIF ID
> extracted from the high and low byte fields in the igmpmsg.
>
> Signed-off-by: Paul Davey <[email protected]>
> ---
> 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 4809318f591b..939792a38814 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -2432,7 +2432,7 @@ static void igmpmsg_netlink_event(struct mr_table *mrt, struct sk_buff *pkt)
> rtgenm = nlmsg_data(nlh);
> rtgenm->rtgen_family = RTNL_FAMILY_IPMR;
> if (nla_put_u8(skb, IPMRA_CREPORT_MSGTYPE, msg->im_msgtype) ||
> - nla_put_u32(skb, IPMRA_CREPORT_VIF_ID, msg->im_vif) ||
> + nla_put_u32(skb, IPMRA_CREPORT_VIF_ID, msg->im_vif | (msg->im_vif_hi << 8)) ||

nit: the inner parentheses seem unnecessary

Otherwise, FWIIW, this series looks good to me.

> nla_put_in_addr(skb, IPMRA_CREPORT_SRC_ADDR,
> msg->im_src.s_addr) ||
> nla_put_in_addr(skb, IPMRA_CREPORT_DST_ADDR,
> --
> 2.28.0
>

2020-09-10 19:27:33

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/3] Allow more than 255 IPv4 multicast interfaces

From: Paul Davey <[email protected]>
Date: Tue, 8 Sep 2020 10:04:05 +1200

> Currently it is not possible to use more than 255 multicast interfaces
> for IPv4 due to the format of the igmpmsg header which only has 8 bits
> available for the VIF ID. There is space available in the igmpmsg
> header to store the full VIF ID in the form of an unused byte following
> the VIF ID field. There is also enough space for the full VIF ID in
> the Netlink cache notifications, however the value is currently taken
> directly from the igmpmsg header and has thus already been truncated.
>
> Adding the high byte of the VIF ID into the unused3 byte of igmpmsg
> allows use of more than 255 IPv4 multicast interfaces. The full VIF ID
> is also available in the Netlink notification by assembling it from
> both bytes from the igmpmsg.
>
> Additionally this reveals a deficiency in the Netlink cache report
> notifications, they lack any means for differentiating cache reports
> relating to different multicast routing tables. This is easily
> resolved by adding the multicast route table ID to the cache reports.
>
> changes in v2:
> - Added high byte of VIF ID to igmpmsg struct replacing unused3
> member.
> - Assemble VIF ID in Netlink notification from both bytes in igmpmsg
> header.

Series applied, thank you.