On Mon, Apr 30, 2012 at 10:22:23AM +0000, Alexander V. Chernikov wrote:
> Author: melifaro
> Date: Mon Apr 30 10:22:23 2012
> New Revision: 234834
> URL: http://svn.freebsd.org/changeset/base/234834
>
> Log:
> Move several enums and structures required for L2 filtering from
> ip_fw_private.h to ip_fw.h.
I would be really grateful if you could revert this back and discuss
what you wanted to achieve with this change other than saving one
entry in the list of includes.
As clearly mentioned in the commit logs
http://svnweb.freebsd.org/base?view=revision&revision=200580
when i did the last revision of the ipfw+dummynet code i tried
to put a strong separation between what is visible in userland
(ip_fw.h and ip_dummynet.h) and kernel specific stuff.
This way changes in the kernel code do not need to affect userland,
modify installed headers and so on.
This is why kernel-specific definitions were put in private files.
We may discuss on the filename, ip_fw_kernel.h may be a better fit,
but merging back kernel and userland defs is a bad design decision.
20-30 years ago there were good reasons to use a single header
for all sorts of definitions: user-only, kernel-only, and kernel-userland API.
Machines were slow, disks were small, portability was not a big deal.
These days none of these conditions apply and keeping things
separate helps maintainance and avoid accidental pollution of
definitions and their misuse.
Besides, keep in mind that ipfw and dummynet are meant to work
on multiple platforms so this change is causing portability troubles.
cheers
luigi
> Approved by: ae(mentor)
> MFC after: 2 weeks
>
> Modified:
> head/sys/contrib/pf/net/pf.c
> head/sys/net/if_bridge.c
> head/sys/net/if_ethersubr.c
> head/sys/netinet/ip_fw.h
> head/sys/netinet/ipfw/ip_fw_private.h
> head/sys/ofed/drivers/infiniband/ulp/ipoib/ipoib.h
>
> Modified: head/sys/contrib/pf/net/pf.c
> ==============================================================================
> --- head/sys/contrib/pf/net/pf.c Mon Apr 30 09:46:05 2012
> (r234833)
> +++ head/sys/contrib/pf/net/pf.c Mon Apr 30 10:22:23 2012
> (r234834)
> @@ -122,7 +122,6 @@ __FBSDID("$FreeBSD$");
> #include <netinet/if_ether.h>
> #ifdef __FreeBSD__
> #include <netinet/ip_fw.h>
> -#include <netinet/ipfw/ip_fw_private.h> /* XXX: only for DIR_IN/DIR_OUT */
> #endif
>
> #ifndef __FreeBSD__
>
> Modified: head/sys/net/if_bridge.c
> ==============================================================================
> --- head/sys/net/if_bridge.c Mon Apr 30 09:46:05 2012 (r234833)
> +++ head/sys/net/if_bridge.c Mon Apr 30 10:22:23 2012 (r234834)
> @@ -132,7 +132,6 @@ __FBSDID("$FreeBSD$");
>
> #include <net/route.h>
> #include <netinet/ip_fw.h>
> -#include <netinet/ipfw/ip_fw_private.h>
>
> /*
> * Size of the route hash table. Must be a power of two.
>
> Modified: head/sys/net/if_ethersubr.c
> ==============================================================================
> --- head/sys/net/if_ethersubr.c Mon Apr 30 09:46:05 2012
> (r234833)
> +++ head/sys/net/if_ethersubr.c Mon Apr 30 10:22:23 2012
> (r234834)
> @@ -72,7 +72,6 @@
> #include <netinet/ip_carp.h>
> #include <netinet/ip_var.h>
> #include <netinet/ip_fw.h>
> -#include <netinet/ipfw/ip_fw_private.h>
> #endif
> #ifdef INET6
> #include <netinet6/nd6.h>
>
> Modified: head/sys/netinet/ip_fw.h
> ==============================================================================
> --- head/sys/netinet/ip_fw.h Mon Apr 30 09:46:05 2012 (r234833)
> +++ head/sys/netinet/ip_fw.h Mon Apr 30 10:22:23 2012 (r234834)
> @@ -545,6 +545,88 @@ struct ipfw_flow_id {
>
> #define IS_IP6_FLOW_ID(id) ((id)->addr_type == 6)
>
> +#ifdef _KERNEL
> +/* Return values from ipfw_[ether_]chk() */
> +enum {
> + IP_FW_PASS = 0,
> + IP_FW_DENY,
> + IP_FW_DIVERT,
> + IP_FW_TEE,
> + IP_FW_DUMMYNET,
> + IP_FW_NETGRAPH,
> + IP_FW_NGTEE,
> + IP_FW_NAT,
> + IP_FW_REASS,
> +};
> +
> +/*
> + * Hooks sometime need to know the direction of the packet
> + * (divert, dummynet, netgraph, ...)
> + * We use a generic definition here, with bit0-1 indicating the
> + * direction, bit 2 indicating layer2 or 3, bit 3-4 indicating the
> + * specific protocol (if necessary)
> + */
> +enum {
> + DIR_MASK = 0x3,
> + DIR_OUT = 0,
> + DIR_IN = 1,
> + DIR_FWD = 2,
> + DIR_DROP = 3,
> + PROTO_LAYER2 = 0x4, /* set for layer 2 */
> + /* PROTO_DEFAULT = 0, */
> + PROTO_IPV4 = 0x08,
> + PROTO_IPV6 = 0x10,
> + PROTO_IFB = 0x0c, /* layer2 + ifbridge */
> + /* PROTO_OLDBDG = 0x14, unused, old bridge */
> +};
> +
> +/*
> + * Structure for collecting parameters to dummynet for ip6_output forwarding
> + */
> +struct _ip6dn_args {
> + struct ip6_pktopts *opt_or;
> + struct route_in6 ro_or;
> + int flags_or;
> + struct ip6_moptions *im6o_or;
> + struct ifnet *origifp_or;
> + struct ifnet *ifp_or;
> + struct sockaddr_in6 dst_or;
> + u_long mtu_or;
> + struct route_in6 ro_pmtu_or;
> +};
> +
> +/*
> + * Arguments for calling ipfw_chk() and dummynet_io(). We put them
> + * all into a structure because this way it is easier and more
> + * efficient to pass variables around and extend the interface.
> + */
> +struct ip_fw_args {
> + struct mbuf *m; /* the mbuf chain */
> + struct ifnet *oif; /* output interface */
> + struct sockaddr_in *next_hop; /* forward address */
> + struct sockaddr_in6 *next_hop6; /* ipv6 forward address */
> +
> + /*
> + * On return, it points to the matching rule.
> + * On entry, rule.slot > 0 means the info is valid and
> + * contains the starting rule for an ipfw search.
> + * If chain_id == chain->id && slot >0 then jump to that slot.
> + * Otherwise, we locate the first rule >= rulenum:rule_id
> + */
> + struct ipfw_rule_ref rule; /* match/restart info */
> +
> + struct ether_header *eh; /* for bridged packets */
> +
> + struct ipfw_flow_id f_id; /* grabbed from IP header */
> + //uint32_t cookie; /* a cookie depending on rule action */
> + struct inpcb *inp;
> +
> + struct _ip6dn_args dummypar; /* dummynet->ip6_output */
> + struct sockaddr_in hopstore; /* store here if cannot use a pointer */
> +};
> +
> +#endif /* _KERNEL */
> +
> /*
> * Dynamic ipfw rule.
> */
>
> Modified: head/sys/netinet/ipfw/ip_fw_private.h
> ==============================================================================
> --- head/sys/netinet/ipfw/ip_fw_private.h Mon Apr 30 09:46:05 2012
> (r234833)
> +++ head/sys/netinet/ipfw/ip_fw_private.h Mon Apr 30 10:22:23 2012
> (r234834)
> @@ -48,89 +48,8 @@
> #define SYSEND
> #endif
>
> -/* Return values from ipfw_chk() */
> -enum {
> - IP_FW_PASS = 0,
> - IP_FW_DENY,
> - IP_FW_DIVERT,
> - IP_FW_TEE,
> - IP_FW_DUMMYNET,
> - IP_FW_NETGRAPH,
> - IP_FW_NGTEE,
> - IP_FW_NAT,
> - IP_FW_REASS,
> -};
> -
> -/*
> - * Structure for collecting parameters to dummynet for ip6_output forwarding
> - */
> -struct _ip6dn_args {
> - struct ip6_pktopts *opt_or;
> - struct route_in6 ro_or;
> - int flags_or;
> - struct ip6_moptions *im6o_or;
> - struct ifnet *origifp_or;
> - struct ifnet *ifp_or;
> - struct sockaddr_in6 dst_or;
> - u_long mtu_or;
> - struct route_in6 ro_pmtu_or;
> -};
> -
> -
> -/*
> - * Arguments for calling ipfw_chk() and dummynet_io(). We put them
> - * all into a structure because this way it is easier and more
> - * efficient to pass variables around and extend the interface.
> - */
> -struct ip_fw_args {
> - struct mbuf *m; /* the mbuf chain */
> - struct ifnet *oif; /* output interface */
> - struct sockaddr_in *next_hop; /* forward address */
> - struct sockaddr_in6 *next_hop6; /* ipv6 forward address */
> -
> - /*
> - * On return, it points to the matching rule.
> - * On entry, rule.slot > 0 means the info is valid and
> - * contains the starting rule for an ipfw search.
> - * If chain_id == chain->id && slot >0 then jump to that slot.
> - * Otherwise, we locate the first rule >= rulenum:rule_id
> - */
> - struct ipfw_rule_ref rule; /* match/restart info */
> -
> - struct ether_header *eh; /* for bridged packets */
> -
> - struct ipfw_flow_id f_id; /* grabbed from IP header */
> - //uint32_t cookie; /* a cookie depending on rule action */
> - struct inpcb *inp;
> -
> - struct _ip6dn_args dummypar; /* dummynet->ip6_output */
> - struct sockaddr_in hopstore; /* store here if cannot use a pointer */
> -};
> -
> MALLOC_DECLARE(M_IPFW);
>
> -/*
> - * Hooks sometime need to know the direction of the packet
> - * (divert, dummynet, netgraph, ...)
> - * We use a generic definition here, with bit0-1 indicating the
> - * direction, bit 2 indicating layer2 or 3, bit 3-4 indicating the
> - * specific protocol
> - * indicating the protocol (if necessary)
> - */
> -enum {
> - DIR_MASK = 0x3,
> - DIR_OUT = 0,
> - DIR_IN = 1,
> - DIR_FWD = 2,
> - DIR_DROP = 3,
> - PROTO_LAYER2 = 0x4, /* set for layer 2 */
> - /* PROTO_DEFAULT = 0, */
> - PROTO_IPV4 = 0x08,
> - PROTO_IPV6 = 0x10,
> - PROTO_IFB = 0x0c, /* layer2 + ifbridge */
> - /* PROTO_OLDBDG = 0x14, unused, old bridge */
> -};
> -
> /* wrapper for freeing a packet, in case we need to do more work */
> #ifndef FREE_PKT
> #if defined(__linux__) || defined(_WIN32)
>
> Modified: head/sys/ofed/drivers/infiniband/ulp/ipoib/ipoib.h
> ==============================================================================
> --- head/sys/ofed/drivers/infiniband/ulp/ipoib/ipoib.h Mon Apr 30
> 09:46:05 2012 (r234833)
> +++ head/sys/ofed/drivers/infiniband/ulp/ipoib/ipoib.h Mon Apr 30
> 10:22:23 2012 (r234834)
> @@ -68,7 +68,6 @@
> #include <netinet/if_ether.h>
> #include <netinet/ip_var.h>
> #include <netinet/ip_fw.h>
> -#include <netinet/ipfw/ip_fw_private.h>
> #endif
> #ifdef INET6
> #include <netinet6/nd6.h>
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
