Skip to content

Add ability to resolve (and cache) multicast groups for a given family name#12

Open
Ragnt wants to merge 4 commits into
rust-netlink:mainfrom
Ragnt:main
Open

Add ability to resolve (and cache) multicast groups for a given family name#12
Ragnt wants to merge 4 commits into
rust-netlink:mainfrom
Ragnt:main

Conversation

@Ragnt

@Ragnt Ragnt commented Nov 16, 2024

Copy link
Copy Markdown
Contributor

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.

Comment thread src/resolver.rs

use crate::{error::GenetlinkError, GenetlinkHandle};
use futures::{future::Either, StreamExt};
use log::{error, trace, warn};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding

#[macro_use]
extern crate log;

in lib.rs is acceptable.
Since it is widely used in many Rust project today.

@Ragnt Ragnt Nov 24, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hadn't realized I added the logging, obviously it was for debugging so it doesn't matter to me either way.

Comment thread src/resolver.rs Outdated
@Leo1003

Leo1003 commented Nov 20, 2024

Copy link
Copy Markdown
Contributor

I found that I also don't have write access to this repository...

Co-authored-by: Shao-Fu Chen <leo881003@gmail.com>

@Sh3Rm4n Sh3Rm4n left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

I've stumbled upon this PR as I have a usecase for this and got away with it by querying the family id manually.

What must be done to bring this PR forward? Are you still interested to continue this PR @Ragnt?

Comment thread src/resolver.rs
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)],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also work with GenlCtrlAttrs::FamilyName so you don't have to resolve the family id here.

Comment thread src/resolver.rs
}
}

pub fn query_family_multicast_groups(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 thread src/resolver.rs
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);
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/resolver.rs

}

#[cfg(all(test, tokio_socket))]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are actually never run and always disabled ...

Suggested change
#[cfg(all(test, tokio_socket))]
#[cfg(all(test, feature = "tokio_socket"))]

@cathay4t

Copy link
Copy Markdown
Member

It been a while. Any progress?

@flaviosousamato244-design flaviosousamato244-design left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hola seus exorcista

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants