Re: [PATCH net-next v2 1/5] ipv4: igmp: get rid of IGMPV3_{QQIC,MRC} and simplify calculation

Ido Schimmel
2026-04-01T07:25:37Z

On Mon, Mar 30, 2026 at 07:16:07PM +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 <royujjal@gmail.com>
> ---
> include/linux/igmp.h | 78 +++++++++++++++++++++++++++++++++++----
> net/bridge/br_multicast.c | 2 +-
> net/ipv4/igmp.c | 6 +--
> 3 files changed, 74 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/igmp.h b/include/linux/igmp.h
> index 073b30a9b850..3c12c0a63492 100644
> --- a/include/linux/igmp.h
> +++ b/include/linux/igmp.h
> @@ -92,15 +92,77 @@ 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)
> + *
> + * If Max Resp Code >= 128, Max Resp Code represents a floating-point
> + * value as follows:
> + * If QQIC >= 128, QQIC represents a floating-point value as follows:

You have "as follows:" twice. Reword it to make it clear that both
fields are encoded the same.

Please also mention that MRT is in units of 100ms (0.1 seconds) and that
QQIC is in units of seconds.

> + *
> + * 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 QQI/MRT 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)
> + */

You are mixing two different styles of comments. Please use netdev style
comments like you have above:

/* IGMPv3 QQI/MRT 8-bit exponential field decode.
* [...]
*/

Same in other places throughout the patchset.

> +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)
> + */
> + if (code < IGMPV3_EXP_MIN_THRESHOLD) {
> + return (unsigned long)code;

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));

return (mc_man | 0x10) << (mc_exp + 3);

> + }
> +}
> +
> +/* Calculate Max Resp Time from Maximum Response Code */
> +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
> + */
> + return igmpv3_exp_field_decode(ih3->code);
> +}
> +
> +/* Calculate Querier's Query Interval from Querier's Query Interval Code */
> +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)
> + */
> + 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..8c6102737096 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);

Given you are modifying the line, adjust it to:

max_delay = igmpv3_mrt(ih3) * (HZ / IGMP_TIMER_SCALE);

Same in other places where you are already modifying a line.

> 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
>