Add ability to resolve (and cache) multicast groups for a given family name #12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Leo1003
suggested changes
Nov 20, 2024
| @@ -2,29 +2,36 @@ | |||
|
|
|||
| use crate::{error::GenetlinkError, GenetlinkHandle}; | |||
| use futures::{future::Either, StreamExt}; | |||
| use log::{error, trace, warn}; | |||
There was a problem hiding this comment.
I think adding
#[macro_use]
extern crate log;in lib.rs is acceptable.
Since it is widely used in many Rust project today.
There was a problem hiding this comment.
I hadn't realized I added the logging, obviously it was for debugging so it doesn't matter to me either way.
|
I found that I also don't have write access to this repository... |
Co-authored-by: Shao-Fu Chen <[email protected]>
Sh3Rm4n
reviewed
Jul 11, 2025
| trace!("Creating GenlMessage for CTRL_CMD_GETFAMILY"); | ||
| let mut genlmsg: GenlMessage<GenlCtrl> = GenlMessage::from_payload(GenlCtrl { | ||
| cmd: GenlCtrlCmd::GetFamily, | ||
| nlas: vec![GenlCtrlAttrs::FamilyId(family_id)], |
There was a problem hiding this comment.
This should also work with GenlCtrlAttrs::FamilyName so you don't have to resolve the family id here.
| @@ -85,9 +92,112 @@ impl Resolver { | |||
| } | |||
| } | |||
|
|
|||
| pub fn query_family_multicast_groups( | |||
There was a problem hiding this comment.
This functions always sends out a netlink request compared to query_family_id. I don't have a preference, but wanted to point out this inconsistency.
Comment on lines
+137
to
+173
| trace!("Processing InnerMessage: {:?}", genlmsg); | ||
| for nla in genlmsg.payload.nlas { | ||
| trace!("Processing NLA: {:?}", nla); | ||
| if let GenlCtrlAttrs::McastGroups(groups) = nla { | ||
| trace!("Found McastGroups: {:?}", groups); | ||
| for group in groups { | ||
| // 'group' is a Vec<McastGrpAttrs> | ||
| let mut group_name = None; | ||
| let mut group_id = None; | ||
|
|
||
| for group_attr in group { | ||
| trace!("Processing group_attr: {:?}", group_attr); | ||
| match group_attr { | ||
| McastGrpAttrs::Name(ref name) => { | ||
| group_name = Some(name.clone()); | ||
| trace!("Found group name: '{}'", name); | ||
| } | ||
| McastGrpAttrs::Id(id) => { | ||
| group_id = Some(id); | ||
| trace!("Found group id: {}", id); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if let (Some(name), Some(id)) = (group_name, group_id) { | ||
| mc_groups.insert(name.clone(), id); | ||
| trace!( | ||
| "Inserted group '{}' with id {} into mc_groups", | ||
| name, | ||
| id | ||
| ); | ||
| } | ||
| } | ||
| } else { | ||
| trace!("Unhandled NLA: {:?}", nla); | ||
| } | ||
| } |
There was a problem hiding this comment.
This could probably be written a little more concise:
Suggested change
| trace!("Processing InnerMessage: {:?}", genlmsg); | |
| for nla in genlmsg.payload.nlas { | |
| trace!("Processing NLA: {:?}", nla); | |
| if let GenlCtrlAttrs::McastGroups(groups) = nla { | |
| trace!("Found McastGroups: {:?}", groups); | |
| for group in groups { | |
| // 'group' is a Vec<McastGrpAttrs> | |
| let mut group_name = None; | |
| let mut group_id = None; | |
| for group_attr in group { | |
| trace!("Processing group_attr: {:?}", group_attr); | |
| match group_attr { | |
| McastGrpAttrs::Name(ref name) => { | |
| group_name = Some(name.clone()); | |
| trace!("Found group name: '{}'", name); | |
| } | |
| McastGrpAttrs::Id(id) => { | |
| group_id = Some(id); | |
| trace!("Found group id: {}", id); | |
| } | |
| } | |
| } | |
| if let (Some(name), Some(id)) = (group_name, group_id) { | |
| mc_groups.insert(name.clone(), id); | |
| trace!( | |
| "Inserted group '{}' with id {} into mc_groups", | |
| name, | |
| id | |
| ); | |
| } | |
| } | |
| } else { | |
| trace!("Unhandled NLA: {:?}", nla); | |
| } | |
| } | |
| // One specific family id was requested, it can be assumed, that the mcast | |
| // groups are part of that family. | |
| let Some(mcast_groups) = genlmsg | |
| .payload | |
| .nlas | |
| .into_iter() | |
| .filter_map(|attr| match attr { | |
| GenlCtrlAttrs::McastGroups(groups) => { | |
| Some(groups) | |
| } | |
| _ => None, | |
| }) | |
| .next() | |
| else { | |
| continue; | |
| }; | |
| for group in mcast_groups.into_iter().filter_map(|attrs| { | |
| match attrs.as_slice() { | |
| [McastGrpAttrs::Name(name), McastGrpAttrs::Id(i)] | | |
| [McastGrpAttrs::Id(i), McastGrpAttrs::Name(name)] => Some((name.clone(), *i)), | |
| _ => None | |
| } | |
| }) { | |
| mc_groups.insert(group.0, group.1); | |
| } |
Sh3Rm4n
reviewed
Jul 11, 2025
| } | ||
|
|
||
| } | ||
|
|
||
| #[cfg(all(test, tokio_socket))] |
There was a problem hiding this comment.
The tests are actually never run and always disabled ...
Suggested change
| #[cfg(all(test, tokio_socket))] | |
| #[cfg(all(test, feature = "tokio_socket"))] |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was a quick add. I didn't want to disturb the existing function as it would break current implementations, but this adds the functionality without (hopefully) breaking anything.