On Fri, Apr 03, 2026 at 03:00:46PM +0000, Ujjal Roy wrote:
> Get rid of the IGMPV3_MRC macro and use the igmpv3_mrt() API to
> calculate the Max Resp Time from the Maximum Response Code.
>
> Similarly, for IGMPV3_QQIC, use the igmpv3_qqi() API to calculate
> the Querier's Query Interval from the QQIC field.
>
> Signed-off-by: Ujjal Roy <[email protected]>
Looks fine, but see a few nits below
> ---
> include/linux/igmp.h | 84 +++++++++++++++++++++++++++++++++++----
> net/bridge/br_multicast.c | 2 +-
> net/ipv4/igmp.c | 6 +--
> 3 files changed, 80 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/igmp.h b/include/linux/igmp.h
> index 073b30a9b850..0624410e75c6 100644
> --- a/include/linux/igmp.h
> +++ b/include/linux/igmp.h
> @@ -92,15 +92,83 @@ struct ip_mc_list {
> struct rcu_head rcu;
> };
>
> +/* RFC3376, relevant sections:
> + * - 4.1.1. Maximum Response Code
> + * - 4.1.7. QQIC (Querier's Query Interval Code)
> + *
> + * For both MRC and QQIC, values >= 128 use the same floating-point
> + * encoding as follows:
> + *
> + * 0 1 2 3 4 5 6 7
> + * +-+-+-+-+-+-+-+-+
> + * |1| exp | mant |
> + * +-+-+-+-+-+-+-+-+
> + */
> +#define IGMPV3_FP_EXP(value) (((value) >> 4) & 0x07)
> +#define IGMPV3_FP_MAN(value) ((value) & 0x0f)
> +
> +/* IGMPV3 floating-point exponential field threshold */
> +#define IGMPV3_EXP_MIN_THRESHOLD 128
> +
> /* V3 exponential field decoding */
> -#define IGMPV3_MASK(value, nb) ((nb)>=32 ? (value) : ((1<<(nb))-1) & (value))
> -#define IGMPV3_EXP(thresh, nbmant, nbexp, value) \
> - ((value) < (thresh) ? (value) : \
> - ((IGMPV3_MASK(value, nbmant) | (1<<(nbmant))) << \
> - (IGMPV3_MASK((value) >> (nbmant), nbexp) + (nbexp))))
> -
> -#define IGMPV3_QQIC(value) IGMPV3_EXP(0x80, 4, 3, value)
> -#define IGMPV3_MRC(value) IGMPV3_EXP(0x80, 4, 3, value)
> +
> +/* IGMPv3 QQIC/MRC 8-bit exponential field decode.
> + *
> + * RFC3376, 4.1.1 & 4.1.7. defines the decoding formula:
> + * 0 1 2 3 4 5 6 7
> + * +-+-+-+-+-+-+-+-+
> + * |1| exp | mant |
> + * +-+-+-+-+-+-+-+-+
> + * Max Resp Time = (mant | 0x10) << (exp + 3)
> + * QQI = (mant | 0x10) << (exp + 3)
> + */
> +static inline unsigned long igmpv3_exp_field_decode(const u8 code)
> +{
> + /* RFC3376, relevant sections:
> + * - 4.1.1. Maximum Response Code
> + * - 4.1.7. QQIC (Querier's Query Interval Code)
> + */
I find it weird to have a comment at the beginning of the function when
there's already a comment above the function. Let's remove this comment
since this information is already present in the MRC/QQIC functions
below.
> + if (code < IGMPV3_EXP_MIN_THRESHOLD) {
> + return code;
> + } else {
> + unsigned long mc_man, mc_exp;
> +
> + mc_exp = IGMPV3_FP_EXP(code);
> + mc_man = IGMPV3_FP_MAN(code);
> +
> + return (mc_man | 0x10) << (mc_exp + 3);
> + }
> +}
> +
> +/* Calculate Max Resp Time from Maximum Response Code
> + *
> + * After decode, MRC represents the Maximum Response Time (MRT) in units
> + * of 0.1 seconds (100 ms).
> + */
> +static inline unsigned long igmpv3_mrt(const struct igmpv3_query *ih3)
> +{
> + /* RFC3376, relevant sections:
> + * - 4.1.1. Maximum Response Code
> + * - 8.3. Query Response Interval
> + */
Please move this to the comment above the function
> + return igmpv3_exp_field_decode(ih3->code);
> +}
> +
> +/* Calculate Querier's Query Interval from Querier's Query Interval Code
> + *
> + * After decode, QQIC represents the Querier's Query Interval in units
> + * of seconds.
> + */
> +static inline unsigned long igmpv3_qqi(const struct igmpv3_query *ih3)
> +{
> + /* RFC3376, relevant sections:
> + * - 4.1.7. QQIC (Querier's Query Interval Code)
> + * - 8.2. Query Interval
> + * - 8.12. Older Version Querier Present Timeout
> + * (the [Query Interval] in the last Query received)
> + */
Likewise
> + return igmpv3_exp_field_decode(ih3->qqic);
> +}
>
> static inline int ip_mc_may_pull(struct sk_buff *skb, unsigned int len)
> {
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 881d866d687a..9fec76e887bc 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -3518,7 +3518,7 @@ static void br_ip4_multicast_query(struct
> net_bridge_mcast *brmctx,
> goto out;
>
> max_delay = ih3->code ?
> - IGMPV3_MRC(ih3->code) * (HZ / IGMP_TIMER_SCALE) : 1;
> + igmpv3_mrt(ih3) * (HZ / IGMP_TIMER_SCALE) : 1;
> } else {
> goto out;
> }
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index a674fb44ec25..d7eff36d98c3 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -991,7 +991,7 @@ static bool igmp_heard_query(struct in_device *in_dev,
> struct sk_buff *skb,
> * different encoding. We use the v3 encoding as more likely
> * to be intended in a v3 query.
> */
> - max_delay = IGMPV3_MRC(ih3->code)*(HZ/IGMP_TIMER_SCALE);
> + max_delay = igmpv3_mrt(ih3) * (HZ / IGMP_TIMER_SCALE);
> if (!max_delay)
> max_delay = 1; /* can't mod w/ 0 */
> } else { /* v3 */
> @@ -1006,7 +1006,7 @@ static bool igmp_heard_query(struct in_device *in_dev,
> struct sk_buff *skb,
> ih3 = igmpv3_query_hdr(skb);
> }
>
> - max_delay = IGMPV3_MRC(ih3->code)*(HZ/IGMP_TIMER_SCALE);
> + max_delay = igmpv3_mrt(ih3) * (HZ / IGMP_TIMER_SCALE);
> if (!max_delay)
> max_delay = 1; /* can't mod w/ 0 */
> WRITE_ONCE(in_dev->mr_maxdelay, max_delay);
> @@ -1016,7 +1016,7 @@ static bool igmp_heard_query(struct in_device *in_dev,
> struct sk_buff *skb,
> * configured value.
> */
> in_dev->mr_qrv = ih3->qrv ?:
> READ_ONCE(net->ipv4.sysctl_igmp_qrv);
> - in_dev->mr_qi = IGMPV3_QQIC(ih3->qqic)*HZ ?:
> IGMP_QUERY_INTERVAL;
> + in_dev->mr_qi = igmpv3_qqi(ih3) * HZ ? : IGMP_QUERY_INTERVAL;
>
> /* RFC3376, 8.3. Query Response Interval:
> * The number of seconds represented by the [Query Response
> --
> 2.43.0
>
