* [PATCH net] net: bridge: mcast: QRI must be less than QI
@ 2021-10-20 2:36 Hangbin Liu
2021-10-20 2:40 ` [PATCHv2 " Hangbin Liu
0 siblings, 1 reply; 6+ messages in thread
From: Hangbin Liu @ 2021-10-20 2:36 UTC (permalink / raw)
To: netdev
Cc: Nikolay Aleksandrov, roopa, bridge, davem, kuba,
Nikolay Aleksandrov, Hangbin Liu
Based on RFC3376 8.3:
The number of seconds represented by the [Query Response Interval]
must be less than the [Query Interval].
Fixes: d902eee43f19 ("bridge: Add multicast count/interval sysfs entries")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/bridge/br_multicast.c | 27 +++++++++++++++++++++++++++
net/bridge/br_netlink.c | 8 ++++++--
net/bridge/br_private.h | 4 ++++
net/bridge/br_sysfs_br.c | 6 ++----
net/bridge/br_vlan_options.c | 8 ++++++--
5 files changed, 45 insertions(+), 8 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index f3d751105343..1a865d08a87f 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -4522,6 +4522,33 @@ int br_multicast_set_mld_version(struct net_bridge_mcast *brmctx,
}
#endif
+/* RFC3376 8.3: The number of seconds represented by the
+ * [Query Response Interval] must be less than the [Query Interval].
+ */
+int br_multicast_set_qi(struct net_bridge_mcast *brmctx, unsigned long val,
+ struct netlink_ext_ack *extack)
+{
+ if (val > brmctx->multicast_query_response_interval) {
+ brmctx->multicast_query_interval = val;
+ return 0;
+ } else {
+ NL_SET_ERR_MSG(extack, "Invalid QI, must greater than QRI");
+ return -EINVAL;
+ }
+}
+
+int br_multicast_set_qri(struct net_bridge_mcast *brmctx, unsigned long val,
+ struct netlink_ext_ack *extack)
+{
+ if (val < brmctx->multicast_query_interval) {
+ brmctx->multicast_query_response_interval = val;
+ return 0;
+ } else {
+ NL_SET_ERR_MSG(extack, "Invalid QRI, must less than QI");
+ return -EINVAL;
+ }
+}
+
/**
* br_multicast_list_adjacent - Returns snooped multicast addresses
* @dev: The bridge port adjacent to which to retrieve addresses
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 5c6c4305ed23..2b32d7d2ce31 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1357,13 +1357,17 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
if (data[IFLA_BR_MCAST_QUERY_INTVL]) {
u64 val = nla_get_u64(data[IFLA_BR_MCAST_QUERY_INTVL]);
- br->multicast_ctx.multicast_query_interval = clock_t_to_jiffies(val);
+ err = br_multicast_set_qi(&br->multicast_ctx, clock_t_to_jiffies(val), extack);
+ if (err)
+ return err;
}
if (data[IFLA_BR_MCAST_QUERY_RESPONSE_INTVL]) {
u64 val = nla_get_u64(data[IFLA_BR_MCAST_QUERY_RESPONSE_INTVL]);
- br->multicast_ctx.multicast_query_response_interval = clock_t_to_jiffies(val);
+ err = br_multicast_set_qri(&br->multicast_ctx, clock_t_to_jiffies(val), extack);
+ if (err)
+ return err;
}
if (data[IFLA_BR_MCAST_STARTUP_QUERY_INTVL]) {
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 37ca76406f1e..5019c601f689 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -906,6 +906,10 @@ int br_multicast_set_igmp_version(struct net_bridge_mcast *brmctx,
int br_multicast_set_mld_version(struct net_bridge_mcast *brmctx,
unsigned long val);
#endif
+int br_multicast_set_qi(struct net_bridge_mcast *brmctx, unsigned long val,
+ struct netlink_ext_ack *extack);
+int br_multicast_set_qri(struct net_bridge_mcast *brmctx, unsigned long val,
+ struct netlink_ext_ack *extack);
struct net_bridge_mdb_entry *
br_mdb_ip_get(struct net_bridge *br, struct br_ip *dst);
struct net_bridge_mdb_entry *
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index d9a89ddd0331..f794652f8592 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -658,8 +658,7 @@ static ssize_t multicast_query_interval_show(struct device *d,
static int set_query_interval(struct net_bridge *br, unsigned long val,
struct netlink_ext_ack *extack)
{
- br->multicast_ctx.multicast_query_interval = clock_t_to_jiffies(val);
- return 0;
+ return br_multicast_set_qi(&br->multicast_ctx, clock_t_to_jiffies(val), extack);
}
static ssize_t multicast_query_interval_store(struct device *d,
@@ -682,8 +681,7 @@ static ssize_t multicast_query_response_interval_show(
static int set_query_response_interval(struct net_bridge *br, unsigned long val,
struct netlink_ext_ack *extack)
{
- br->multicast_ctx.multicast_query_response_interval = clock_t_to_jiffies(val);
- return 0;
+ return br_multicast_set_qri(&br->multicast_ctx, clock_t_to_jiffies(val), extack);
}
static ssize_t multicast_query_response_interval_store(
diff --git a/net/bridge/br_vlan_options.c b/net/bridge/br_vlan_options.c
index 8ffd4ed2563c..71e94ff9d926 100644
--- a/net/bridge/br_vlan_options.c
+++ b/net/bridge/br_vlan_options.c
@@ -521,14 +521,18 @@ static int br_vlan_process_global_one_opts(const struct net_bridge *br,
u64 val;
val = nla_get_u64(tb[BRIDGE_VLANDB_GOPTS_MCAST_QUERY_INTVL]);
- v->br_mcast_ctx.multicast_query_interval = clock_t_to_jiffies(val);
+ err = br_multicast_set_qi(&v->br_mcast_ctx, clock_t_to_jiffies(val), extack);
+ if (err)
+ return err;
*changed = true;
}
if (tb[BRIDGE_VLANDB_GOPTS_MCAST_QUERY_RESPONSE_INTVL]) {
u64 val;
val = nla_get_u64(tb[BRIDGE_VLANDB_GOPTS_MCAST_QUERY_RESPONSE_INTVL]);
- v->br_mcast_ctx.multicast_query_response_interval = clock_t_to_jiffies(val);
+ err = br_multicast_set_qri(&v->br_mcast_ctx, clock_t_to_jiffies(val), extack);
+ if (err)
+ return err;
*changed = true;
}
if (tb[BRIDGE_VLANDB_GOPTS_MCAST_STARTUP_QUERY_INTVL]) {
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCHv2 net] net: bridge: mcast: QRI must be less than QI
2021-10-20 2:36 [PATCH net] net: bridge: mcast: QRI must be less than QI Hangbin Liu
@ 2021-10-20 2:40 ` Hangbin Liu
2021-10-20 9:49 ` Nikolay Aleksandrov
0 siblings, 1 reply; 6+ messages in thread
From: Hangbin Liu @ 2021-10-20 2:40 UTC (permalink / raw)
To: netdev
Cc: Nikolay Aleksandrov, roopa, bridge, davem, kuba,
Nikolay Aleksandrov, Hangbin Liu
Based on RFC3376 8.3:
The number of seconds represented by the [Query Response Interval]
must be less than the [Query Interval].
Fixes: d902eee43f19 ("bridge: Add multicast count/interval sysfs entries")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/bridge/br_multicast.c | 27 +++++++++++++++++++++++++++
net/bridge/br_netlink.c | 8 ++++++--
net/bridge/br_private.h | 4 ++++
net/bridge/br_sysfs_br.c | 6 ++----
net/bridge/br_vlan_options.c | 8 ++++++--
5 files changed, 45 insertions(+), 8 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index f3d751105343..5931f7c81519 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -4522,6 +4522,33 @@ int br_multicast_set_mld_version(struct net_bridge_mcast *brmctx,
}
#endif
+/* RFC3376 8.3: The number of seconds represented by the
+ * [Query Response Interval] must be less than the [Query Interval].
+ */
+int br_multicast_set_qi(struct net_bridge_mcast *brmctx, unsigned long val,
+ struct netlink_ext_ack *extack)
+{
+ if (val > brmctx->multicast_query_response_interval) {
+ brmctx->multicast_query_interval = val;
+ return 0;
+ }
+
+ NL_SET_ERR_MSG(extack, "Invalid QI, must greater than QRI");
+ return -EINVAL;
+}
+
+int br_multicast_set_qri(struct net_bridge_mcast *brmctx, unsigned long val,
+ struct netlink_ext_ack *extack)
+{
+ if (val < brmctx->multicast_query_interval) {
+ brmctx->multicast_query_response_interval = val;
+ return 0;
+ }
+
+ NL_SET_ERR_MSG(extack, "Invalid QRI, must less than QI");
+ return -EINVAL;
+}
+
/**
* br_multicast_list_adjacent - Returns snooped multicast addresses
* @dev: The bridge port adjacent to which to retrieve addresses
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 5c6c4305ed23..2b32d7d2ce31 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1357,13 +1357,17 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
if (data[IFLA_BR_MCAST_QUERY_INTVL]) {
u64 val = nla_get_u64(data[IFLA_BR_MCAST_QUERY_INTVL]);
- br->multicast_ctx.multicast_query_interval = clock_t_to_jiffies(val);
+ err = br_multicast_set_qi(&br->multicast_ctx, clock_t_to_jiffies(val), extack);
+ if (err)
+ return err;
}
if (data[IFLA_BR_MCAST_QUERY_RESPONSE_INTVL]) {
u64 val = nla_get_u64(data[IFLA_BR_MCAST_QUERY_RESPONSE_INTVL]);
- br->multicast_ctx.multicast_query_response_interval = clock_t_to_jiffies(val);
+ err = br_multicast_set_qri(&br->multicast_ctx, clock_t_to_jiffies(val), extack);
+ if (err)
+ return err;
}
if (data[IFLA_BR_MCAST_STARTUP_QUERY_INTVL]) {
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 37ca76406f1e..5019c601f689 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -906,6 +906,10 @@ int br_multicast_set_igmp_version(struct net_bridge_mcast *brmctx,
int br_multicast_set_mld_version(struct net_bridge_mcast *brmctx,
unsigned long val);
#endif
+int br_multicast_set_qi(struct net_bridge_mcast *brmctx, unsigned long val,
+ struct netlink_ext_ack *extack);
+int br_multicast_set_qri(struct net_bridge_mcast *brmctx, unsigned long val,
+ struct netlink_ext_ack *extack);
struct net_bridge_mdb_entry *
br_mdb_ip_get(struct net_bridge *br, struct br_ip *dst);
struct net_bridge_mdb_entry *
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index d9a89ddd0331..f794652f8592 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -658,8 +658,7 @@ static ssize_t multicast_query_interval_show(struct device *d,
static int set_query_interval(struct net_bridge *br, unsigned long val,
struct netlink_ext_ack *extack)
{
- br->multicast_ctx.multicast_query_interval = clock_t_to_jiffies(val);
- return 0;
+ return br_multicast_set_qi(&br->multicast_ctx, clock_t_to_jiffies(val), extack);
}
static ssize_t multicast_query_interval_store(struct device *d,
@@ -682,8 +681,7 @@ static ssize_t multicast_query_response_interval_show(
static int set_query_response_interval(struct net_bridge *br, unsigned long val,
struct netlink_ext_ack *extack)
{
- br->multicast_ctx.multicast_query_response_interval = clock_t_to_jiffies(val);
- return 0;
+ return br_multicast_set_qri(&br->multicast_ctx, clock_t_to_jiffies(val), extack);
}
static ssize_t multicast_query_response_interval_store(
diff --git a/net/bridge/br_vlan_options.c b/net/bridge/br_vlan_options.c
index 8ffd4ed2563c..71e94ff9d926 100644
--- a/net/bridge/br_vlan_options.c
+++ b/net/bridge/br_vlan_options.c
@@ -521,14 +521,18 @@ static int br_vlan_process_global_one_opts(const struct net_bridge *br,
u64 val;
val = nla_get_u64(tb[BRIDGE_VLANDB_GOPTS_MCAST_QUERY_INTVL]);
- v->br_mcast_ctx.multicast_query_interval = clock_t_to_jiffies(val);
+ err = br_multicast_set_qi(&v->br_mcast_ctx, clock_t_to_jiffies(val), extack);
+ if (err)
+ return err;
*changed = true;
}
if (tb[BRIDGE_VLANDB_GOPTS_MCAST_QUERY_RESPONSE_INTVL]) {
u64 val;
val = nla_get_u64(tb[BRIDGE_VLANDB_GOPTS_MCAST_QUERY_RESPONSE_INTVL]);
- v->br_mcast_ctx.multicast_query_response_interval = clock_t_to_jiffies(val);
+ err = br_multicast_set_qri(&v->br_mcast_ctx, clock_t_to_jiffies(val), extack);
+ if (err)
+ return err;
*changed = true;
}
if (tb[BRIDGE_VLANDB_GOPTS_MCAST_STARTUP_QUERY_INTVL]) {
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCHv2 net] net: bridge: mcast: QRI must be less than QI
2021-10-20 2:40 ` [PATCHv2 " Hangbin Liu
@ 2021-10-20 9:49 ` Nikolay Aleksandrov
2021-10-20 10:19 ` Hangbin Liu
0 siblings, 1 reply; 6+ messages in thread
From: Nikolay Aleksandrov @ 2021-10-20 9:49 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: Nikolay Aleksandrov, roopa, bridge, davem, kuba
On 20/10/2021 05:40, Hangbin Liu wrote:
> Based on RFC3376 8.3:
> The number of seconds represented by the [Query Response Interval]
> must be less than the [Query Interval].
>
> Fixes: d902eee43f19 ("bridge: Add multicast count/interval sysfs entries")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> net/bridge/br_multicast.c | 27 +++++++++++++++++++++++++++
> net/bridge/br_netlink.c | 8 ++++++--
> net/bridge/br_private.h | 4 ++++
> net/bridge/br_sysfs_br.c | 6 ++----
> net/bridge/br_vlan_options.c | 8 ++++++--
> 5 files changed, 45 insertions(+), 8 deletions(-)
>
Nacked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
I think we just discussed this a day ago? It is the same problem -
while we all agree the values should follow the RFC, users have had
the option to set any values forever (even non-RFC compliant ones).
This change risks breaking user-space.
Users are free to follow the RFC or not, we can't force them at this
point. This should've been done when the config option was added long
time ago.
Thanks,
Nik
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCHv2 net] net: bridge: mcast: QRI must be less than QI
2021-10-20 9:49 ` Nikolay Aleksandrov
@ 2021-10-20 10:19 ` Hangbin Liu
2021-10-20 12:10 ` Hangbin Liu
0 siblings, 1 reply; 6+ messages in thread
From: Hangbin Liu @ 2021-10-20 10:19 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, Nikolay Aleksandrov, roopa, bridge, davem, kuba
On Wed, Oct 20, 2021 at 12:49:17PM +0300, Nikolay Aleksandrov wrote:
> Nacked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
>
> I think we just discussed this a day ago? It is the same problem -
> while we all agree the values should follow the RFC, users have had
> the option to set any values forever (even non-RFC compliant ones).
> This change risks breaking user-space.
OK, I misunderstood your reply in last mail. I thought you only object to
disabling no meaning values(e.g. set timer to 0, which not is forbid by the
RFC). I don't know you also reject to follow a *MUST* rule defined in the RFC.
>
> Users are free to follow the RFC or not, we can't force them at this
> point. This should've been done when the config option was added long
> time ago.
OK, I will stop working on this path.
Thanks
Hangbin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2 net] net: bridge: mcast: QRI must be less than QI
2021-10-20 10:19 ` Hangbin Liu
@ 2021-10-20 12:10 ` Hangbin Liu
2021-10-20 12:14 ` Nikolay Aleksandrov
0 siblings, 1 reply; 6+ messages in thread
From: Hangbin Liu @ 2021-10-20 12:10 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, Nikolay Aleksandrov, roopa, bridge, davem, kuba
On Wed, Oct 20, 2021 at 06:19:25PM +0800, Hangbin Liu wrote:
> On Wed, Oct 20, 2021 at 12:49:17PM +0300, Nikolay Aleksandrov wrote:
> > Nacked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
> >
> > I think we just discussed this a day ago? It is the same problem -
> > while we all agree the values should follow the RFC, users have had
> > the option to set any values forever (even non-RFC compliant ones).
> > This change risks breaking user-space.
>
> OK, I misunderstood your reply in last mail. I thought you only object to
> disabling no meaning values(e.g. set timer to 0, which not is forbid by the
> RFC). I don't know you also reject to follow a *MUST* rule defined in the RFC.
I know you denied the patch due to user-space compatibility. Forgive me
if my last reply sound a little aggressive.
Thanks
Hangbin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2 net] net: bridge: mcast: QRI must be less than QI
2021-10-20 12:10 ` Hangbin Liu
@ 2021-10-20 12:14 ` Nikolay Aleksandrov
0 siblings, 0 replies; 6+ messages in thread
From: Nikolay Aleksandrov @ 2021-10-20 12:14 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Nikolay Aleksandrov, roopa, bridge, davem, kuba
On 20/10/2021 15:10, Hangbin Liu wrote:
> On Wed, Oct 20, 2021 at 06:19:25PM +0800, Hangbin Liu wrote:
>> On Wed, Oct 20, 2021 at 12:49:17PM +0300, Nikolay Aleksandrov wrote:
>>> Nacked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
>>>
>>> I think we just discussed this a day ago? It is the same problem -
>>> while we all agree the values should follow the RFC, users have had
>>> the option to set any values forever (even non-RFC compliant ones).
>>> This change risks breaking user-space.
>>
>> OK, I misunderstood your reply in last mail. I thought you only object to
>> disabling no meaning values(e.g. set timer to 0, which not is forbid by the
>> RFC). I don't know you also reject to follow a *MUST* rule defined in the RFC.
>
> I know you denied the patch due to user-space compatibility. Forgive me
> if my last reply sound a little aggressive.
>
> Thanks
> Hangbin
>
No worries. :) I obviously agree that it should be RFC compliant, but we must do it
in a different way that doesn't risk breaking users, it goes also for how the values are
computed. In the future when more of the RFC is implemented we might need to force
compliance and that might require adding a new option, I guess we'll see when we get there.
Cheers,
Nik
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-20 12:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-20 2:36 [PATCH net] net: bridge: mcast: QRI must be less than QI Hangbin Liu
2021-10-20 2:40 ` [PATCHv2 " Hangbin Liu
2021-10-20 9:49 ` Nikolay Aleksandrov
2021-10-20 10:19 ` Hangbin Liu
2021-10-20 12:10 ` Hangbin Liu
2021-10-20 12:14 ` Nikolay Aleksandrov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).