[Xorp-hackers] First-cut IP filtering XIF.

Pavlin Radoslavov pavlin@icir.org
Mon, 23 Aug 2004 15:44:37 -0700


> Questions/comments/flames...

Few minor editorial suggestions/nits/comments.

> BMS
> 
> --J2SCkAp4GZ/dPZZf
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: attachment; filename="fea_filter.xif"
> 
> /* $XORP$ */
> 
> /*
> ** IP filtering XRL interface.
> */
> interface fea_filter/0.1 {
> 	/**
> 	 * Get global IP filter enable status.
> 	 */
> 	get_fw_enabled -> enabled:bool
> 
> 	/**
> 	 * Set global IP filter enable status.
> 	 *
> 	 * @param enabled Enable or disable IP filtering.
> 	 */
> 	set_fw_enabled ? enabled:bool
> 
> 	/**
> 	 * Get global IP filter default-to-discard status.
> 	 */
> 	get_fw_default_discard -> discard:bool
> 
> 	/**
> 	 * Set global IP filter default-to-discard status.
> 	 *
> 	 * @param accept Discard all datagrams by default if true;
> 	 * otherwise, accept them.
> 	 */
> 	set_fw_default_discard ? discard:bool
> 
> 	/**
> 	 * Get the underlying IP filter provider type in use.
> 	 */
> 	get_fw_provider -> provider:txt
> 
> 	/**
> 	 * Set the underlying IP filter provider type in use.
> 	 * @param provider Name of an IP firewall provider to use on
> 	 * systems which have multiple IP filtering providers.
> 	 */
> 	set_fw_provider ? provider:txt
> 
> 	/**
> 	 *  Get the underlying IP filter provider version in use.
> 	 */
> 	get_fw_version -> version:txt
> 
> 	/**
> 	 * Add an IPv4 family filter rule.
> 	 *
> 	 * @param iface Name of the interface where this filter is to be applied.

You may want to split the line.

> 	 * @param src Source IPv4 address with network prefix.
> 	 * @param dst Destination IPv4 address with network prefix.
> 	 * @param proto IP protocol number for match (actually u8).
> 	 * @param sport Source TCP/UDP port (actually u16).
> 	 * @param dport Destination TCP/UDP port (actually u16).

The name "sport" may be a bit confusing due to the fact there is
already a word spelled exactly same way :)
I'd recommend the following renaming:

sport -> src_port
dport -> dst_port

Also, in the comments about "proto", "sport" and "dport" you may
want to replace "(actually u8)" with something like "(0-255)",
because when the kdoc comments gets picked-up from the
auto-generated *.hh file, the "u32", etc, XRL-specific keywords are
lost.

> 	 * @param action Action to take when this filter is matched.
> 	 */
> 	add_filter_4 \
> 		? \
> 		iface:txt & \
> 		src:ipv4net & \
> 		dst:ipv4net & \
> 		proto:u32 & \
> 		sport:u32 & \
> 		dport:u32 & \
> 		action:txt

The convention so far we have been following is to use "foo4" and
"foo6" (instead of "foo_4" and "foo_6") as the names of the XRLs
that are IPv4 and IPv6 specific, hence for consistency I'd recommend
that you use same convention.

Also, alignment-wise, even though there are no strict guidelines,
many of the *.xif files use slightly different alignment. E.g., the
'\' is <TAB>-ed to the end of the line, '&' goes first on a line,
etc. Example:

	add_filter4 ? iface:txt					\
		    & src:ipv4net				\
		    & dst:ipv4net				\
		    & proto:u32					\
		    & sport:u32					\
		    & dport:u32					\
		    & action:txt


Regards,
Pavlin