[PATCH net-next] net: bridge: use mld2r_ngrec instead of icmpv6_dataun

From: MichelleJin <redacted>

using icmp6h->mld2r_ngrec instead of icmp6h->icmp6_dataun.un_data16[1].

Signed-off-by: MichelleJin <redacted>
---
 net/bridge/br_multicast.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 2c437d4bf632..8e38e02208bd 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c

@@ -2731,8 +2731,8 @@ static int br_ip6_multicast_mld2_report(struct net_bridge_mcast *brmctx,
 	struct net_bridge_mdb_entry *mdst;
 	struct net_bridge_port_group *pg;
 	unsigned int nsrcs_offset;
+	struct mld2_report *mld2r;
 	const unsigned char *src;
-	struct icmp6hdr *icmp6h;
 	struct in6_addr *h_addr;
 	struct mld2_grec *grec;
 	unsigned int grec_len;

@@ -2740,12 +2740,12 @@ static int br_ip6_multicast_mld2_report(struct net_bridge_mcast *brmctx,
 	int i, len, num;
 	int err = 0;
 
-	if (!ipv6_mc_may_pull(skb, sizeof(*icmp6h)))
+	if (!ipv6_mc_may_pull(skb, sizeof(*mld2r)))
 		return -EINVAL;
 
-	icmp6h = icmp6_hdr(skb);
-	num = ntohs(icmp6h->icmp6_dataun.un_data16[1]);
-	len = skb_transport_offset(skb) + sizeof(*icmp6h);
+	mld2r = (struct mld2_report *) icmp6_hdr(skb);
+	num = ntohs(mld2r->mld2r_ngrec);
+	len = skb_transport_offset(skb) + sizeof(*mld2r);
 
 	for (i = 0; i < num; i++) {
 		__be16 *_nsrcs, __nsrcs;

-- 
2.25.1

On 28/08/2021 11:43, shjy180909@gmail.com wrote:

Indeed it should be equivalent, have you run the bridge selftests with this change?

2021년 8월 28일 (토) 오후 6:51, Nikolay Aleksandrov [off-list ref]님이 작성:

On 28/08/2021 11:43, shjy180909@gmail.com wrote:

quoted

From: MichelleJin <redacted>

using icmp6h->mld2r_ngrec instead of icmp6h->icmp6_dataun.un_data16[1].

Signed-off-by: MichelleJin <redacted>
---
 net/bridge/br_multicast.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 2c437d4bf632..8e38e02208bd 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c

@@ -2731,8 +2731,8 @@ static int br_ip6_multicast_mld2_report(struct net_bridge_mcast *brmctx,
      struct net_bridge_mdb_entry *mdst;
      struct net_bridge_port_group *pg;
      unsigned int nsrcs_offset;
+     struct mld2_report *mld2r;
      const unsigned char *src;
-     struct icmp6hdr *icmp6h;
      struct in6_addr *h_addr;
      struct mld2_grec *grec;
      unsigned int grec_len;

@@ -2740,12 +2740,12 @@ static int br_ip6_multicast_mld2_report(struct net_bridge_mcast *brmctx,
      int i, len, num;
      int err = 0;

-     if (!ipv6_mc_may_pull(skb, sizeof(*icmp6h)))
+     if (!ipv6_mc_may_pull(skb, sizeof(*mld2r)))
              return -EINVAL;

-     icmp6h = icmp6_hdr(skb);
-     num = ntohs(icmp6h->icmp6_dataun.un_data16[1]);
-     len = skb_transport_offset(skb) + sizeof(*icmp6h);
+     mld2r = (struct mld2_report *) icmp6_hdr(skb);
+     num = ntohs(mld2r->mld2r_ngrec);
+     len = skb_transport_offset(skb) + sizeof(*mld2r);

      for (i = 0; i < num; i++) {
              __be16 *_nsrcs, __nsrcs;

Indeed it should be equivalent, have you run the bridge selftests with this change?

I did printk tests.

There's no difference between mld2r->mld2r_ngrec and

icmp6h->icmp6_dataun.un_data16[1].

Also, there is no difference between sizeof(*mld2r) and sizeof(*icmp6h).

test code:
static int br_ip6_multicast_mld2_report(struct net_bridge_mcast *brmctx,
                                        struct net_bridge_mcast_port *pmctx,
                                        struct sk_buff *skb,
                                        u16 vid)
{
...
        struct mld2_report *mld2r; //
        struct icmp6hdr *icmp6h; //
...

        mld2r = (struct mld2_report *) icmp6_hdr(skb); //
        icmp6h = icmp6_hdr(skb); //

        num = ntohs(mld2r->mld2r_ngrec);
        printk("[TEST]%s %u num1 = %u num2 = %u \n",
                  __func__, __LINE__,
                  ntohs(mld2r->mld2r_ngrec),
                  ntohs(icmp6h->icmp6_dataun.un_data16[1]));
        printk("[TEST]%s %u size1 = %lu size2 = %lu \n",
                  __func__, __LINE__,
                  sizeof(*mld2r),
                  sizeof(*icmp6h));

        len = skb_transport_offset(skb) + sizeof(*mld2r);


result:
[ 4932.345658] [TEST]br_ip6_multicast_mld2_report 2751 num1 = 2 num2 = 2
[ 4932.345665] [TEST]br_ip6_multicast_mld2_report 2752 size1 = 8 size2 = 8

The num1 and num2 are indeed equivalent.
Also, the size1 and size2 are equivalent.

On Sat, Aug 28, 2021 at 08:43:07AM +0000, shjy180909@gmail.com wrote:

From: MichelleJin <redacted>

using icmp6h->mld2r_ngrec instead of icmp6h->icmp6_dataun.un_data16[1].

Please could you expand the commit message to explain why?  I can see
what the patch does by reading it, but i have no idea why?

     Andrew