Changeset View
Standalone View
sys/netinet/in_var.h
Show First 20 Lines • Show All 226 Lines • ▼ Show 20 Lines | |||||
/* | /* | ||||
* IPv4 multicast PCB-layer group filter descriptor. | * IPv4 multicast PCB-layer group filter descriptor. | ||||
*/ | */ | ||||
struct in_mfilter { | struct in_mfilter { | ||||
struct ip_msource_tree imf_sources; /* source list for (S,G) */ | struct ip_msource_tree imf_sources; /* source list for (S,G) */ | ||||
u_long imf_nsrc; /* # of source entries */ | u_long imf_nsrc; /* # of source entries */ | ||||
uint8_t imf_st[2]; /* state before/at commit */ | uint8_t imf_st[2]; /* state before/at commit */ | ||||
struct in_multi *imf_inm; /* associated multicast address */ | |||||
STAILQ_ENTRY(in_mfilter) imf_entry; /* list entry */ | |||||
}; | }; | ||||
/* | |||||
* Helper types and functions for IPv4 multicast filters. | |||||
*/ | |||||
STAILQ_HEAD(ip_mfilter_head, in_mfilter); | |||||
markj: Why did you make this a typedef? | |||||
Done Inline ActionsTo make future changes easier, like changing the queue type. hselasky: To make future changes easier, like changing the queue type. | |||||
Done Inline ActionsJust write STAILQ_HEAD(in_mfilter_head, in_mfilter). Then you get a struct in_mfilter_head and you can change the queue type easily. markj: Just write STAILQ_HEAD(in_mfilter_head, in_mfilter). Then you get a struct in_mfilter_head and… | |||||
struct in_mfilter *ip_mfilter_alloc(int mflags, int st0, int st1); | |||||
Done Inline ActionsStyle: extra space after '*'. markj: Style: extra space after '*'. | |||||
void ip_mfilter_free(struct in_mfilter *); | |||||
Done Inline ActionsThe naming is kind of weird IMO. Usually we would call these ip_mfilter_alloc(), ip_mfilter_free(), IP_MFILTER_FOREACH, etc.. markj: The naming is kind of weird IMO. Usually we would call these `ip_mfilter_alloc()`… | |||||
static inline void | |||||
ip_mfilter_init(struct ip_mfilter_head *head) | |||||
Not Done Inline ActionsI can't see why all of this stuff is here and not in ip_var.h. There are some consumers that do not include ip_var.h, but they can be fixed. At least, there should be a comment here or in in_var.h explaining the relationship. markj: I can't see why all of this stuff is here and not in ip_var.h. There are some consumers that do… | |||||
Done Inline ActionsCan you explain a bit more? I believe I've already tried this and that is the reason for the #ifdef _NETINET_IN_VAR_H_ I've added. The structures must be defined in the correct order, because not always we use pointers. Else the order wouldn't matter. hselasky: Can you explain a bit more? I believe I've already tried this and that is the reason for the… | |||||
Done Inline ActionsDoesn't this suggest that we should not expose the mfilter list accessors directly but instead define them in a C file as functions on a ip_moptions structure? markj: Doesn't this suggest that we should not expose the mfilter list accessors directly but instead… | |||||
Not Done Inline ActionsCan we leave this AS-IS for now, hence it is most compatible with the old way of doing things. And then later we can move things around which also will produce more diffs? hselasky: Can we leave this AS-IS for now, hence it is most compatible with the old way of doing things. | |||||
Done Inline ActionsI have no objection to the current diff aside from the one nit, but I would have found this diff easier to review if the conversion from an array to STAILQ was done in a separate diff. markj: I have no objection to the current diff aside from the one nit, but I would have found this… | |||||
Done Inline ActionsOK hselasky: OK | |||||
{ | |||||
STAILQ_INIT(head); | |||||
} | |||||
static inline struct in_mfilter * | |||||
ip_mfilter_first(const struct ip_mfilter_head *head) | |||||
{ | |||||
return (STAILQ_FIRST(head)); | |||||
} | |||||
static inline void | |||||
ip_mfilter_insert(struct ip_mfilter_head *head, struct in_mfilter *imf) | |||||
{ | |||||
STAILQ_INSERT_TAIL(head, imf, imf_entry); | |||||
} | |||||
static inline void | |||||
ip_mfilter_remove(struct ip_mfilter_head *head, struct in_mfilter *imf) | |||||
{ | |||||
Done Inline ActionsThis name causes confusion for me. I think this should be named ip_next_mfilter(). "foreach" implies a usage like the STAILQ_FOREACH() sort of macro. That makes this really confusing to read. In fact, can you convert the callers to just use STAILQ_FOREACH() ? I think that would make this much easier to read. gallatin: This name causes confusion for me. I think this should be named ip_next_mfilter(). "foreach"… | |||||
Done Inline ActionsIndeed, I can't see why one of the _FOREACH macros cannot be used. markj: Indeed, I can't see why one of the _FOREACH macros cannot be used. | |||||
Done Inline ActionsFixed in next patch revision. hselasky: Fixed in next patch revision. | |||||
STAILQ_REMOVE(head, imf, in_mfilter, imf_entry); | |||||
} | |||||
#define IP_MFILTER_FOREACH(imf, head) \ | |||||
STAILQ_FOREACH(imf, head, imf_entry) | |||||
static inline size_t | |||||
ip_mfilter_count(struct ip_mfilter_head *head) | |||||
{ | |||||
struct in_mfilter *imf; | |||||
size_t num = 0; | |||||
STAILQ_FOREACH(imf, head, imf_entry) | |||||
num++; | |||||
return (num); | |||||
} | |||||
/* | /* | ||||
* IPv4 group descriptor. | * IPv4 group descriptor. | ||||
* | * | ||||
* For every entry on an ifnet's if_multiaddrs list which represents | * For every entry on an ifnet's if_multiaddrs list which represents | ||||
* an IP multicast group, there is one of these structures. | * an IP multicast group, there is one of these structures. | ||||
* | * | ||||
* If any source filters are present, then a node will exist in the RB-tree | * If any source filters are present, then a node will exist in the RB-tree | ||||
▲ Show 20 Lines • Show All 188 Lines • Show Last 20 Lines |
Why did you make this a typedef?