Hello,
On Thu, 31 May 2007, KOVACS Krisztian wrote:
> So what about this one?
May be we can try with better coding style. Also, this version
adds undefined behavior for using FLOWI_FLAG_ANYSRC with multicast
oldflp->fl4_dst
> Loosen source address check on IPv4 output
>
> From: KOVACS Krisztian <[EMAIL PROTECTED]>
>
> ip_route_output() contains a check to make sure that no flows with
> non-local source IP addresses are routed. This obviously makes using
> such addresses impossible.
>
> This patch introduces a flowi flag which makes omitting this check
> possible. The new flag provides a way of handling transparent and
> non-transparent connections differently.
>
> Signed-off-by: KOVACS Krisztian <[EMAIL PROTECTED]>
> ---
>
> include/net/flow.h | 1 +
> net/ipv4/route.c | 47 +++++++++++++++++++++++++----------------------
> 2 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/include/net/flow.h b/include/net/flow.h
> index f3cc1f8..1bfc0dc 100644
> --- a/include/net/flow.h
> +++ b/include/net/flow.h
> @@ -49,6 +49,7 @@ struct flowi {
> __u8 proto;
> __u8 flags;
> #define FLOWI_FLAG_MULTIPATHOLDROUTE 0x01
> +#define FLOWI_FLAG_ANYSRC 0x02
> union {
> struct {
> __be16 sport;
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 8603cfb..88d0a79 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2396,7 +2396,7 @@ static int ip_route_output_slow(struct rtable **rp,
> const struct flowi *oldflp)
>
> /* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */
> dev_out = ip_dev_find(oldflp->fl4_src);
> - if (dev_out == NULL)
> + if (dev_out == NULL && !(oldflp->flags & FLOWI_FLAG_ANYSRC))
> goto out;
>
> /* I removed check for oif == dev_out->oif here.
> @@ -2407,29 +2407,32 @@ static int ip_route_output_slow(struct rtable **rp,
> const struct flowi *oldflp)
> of another iface. --ANK
> */
>
> - if (oldflp->oif == 0
> - && (MULTICAST(oldflp->fl4_dst) || oldflp->fl4_dst ==
> htonl(0xFFFFFFFF))) {
> - /* Special hack: user can direct multicasts
> - and limited broadcast via necessary interface
> - without fiddling with IP_MULTICAST_IF or IP_PKTINFO.
> - This hack is not just for fun, it allows
> - vic,vat and friends to work.
> - They bind socket to loopback, set ttl to zero
> - and expect that it will work.
> - From the viewpoint of routing cache they are broken,
> - because we are not allowed to build multicast path
> - with loopback source addr (look, routing cache
> - cannot know, that ttl is zero, so that packet
> - will not leave this host and route is valid).
> - Luckily, this hack is good workaround.
> - */
> + if (dev_out) {
> + if (oldflp->oif == 0
> + && (MULTICAST(oldflp->fl4_dst)
> + || oldflp->fl4_dst == htonl(0xFFFFFFFF))) {
> + /* Special hack: user can direct multicasts
> + and limited broadcast via necessary interface
> + without fiddling with IP_MULTICAST_IF or
> IP_PKTINFO.
> + This hack is not just for fun, it allows
> + vic,vat and friends to work.
> + They bind socket to loopback, set ttl to zero
> + and expect that it will work.
> + From the viewpoint of routing cache they are
> broken,
> + because we are not allowed to build
> multicast path
> + with loopback source addr (look, routing
> cache
> + cannot know, that ttl is zero, so that packet
> + will not leave this host and route is valid).
> + Luckily, this hack is good workaround.
> + */
> +
> + fl.oif = dev_out->ifindex;
> + goto make_route;
> + }
>
> - fl.oif = dev_out->ifindex;
> - goto make_route;
> - }
> - if (dev_out)
> dev_put(dev_out);
> - dev_out = NULL;
> + dev_out = NULL;
> + }
> }
What about something like this, it even reduces checks
in the fast path. You can post new version if the following change
looks good to you and to other developers. If additional sign line is
needed here it is:
Signed-off-by: Julian Anastasov <[EMAIL PROTECTED]>
@@ -2396,8 +2396,6 @@ static int ip_route_output_slow(struct r
/* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */
dev_out = ip_dev_find(oldflp->fl4_src);
- if (dev_out == NULL)
- goto out;
/* I removed check for oif == dev_out->oif here.
It was wrong for two reasons:
@@ -2409,6 +2407,8 @@ static int ip_route_output_slow(struct r
if (oldflp->oif == 0
&& (MULTICAST(oldflp->fl4_dst) || oldflp->fl4_dst ==
htonl(0xFFFFFFFF))) {
+ if (dev_out == NULL)
+ goto out;
/* Special hack: user can direct multicasts
and limited broadcast via necessary interface
without fiddling with IP_MULTICAST_IF or IP_PKTINFO.
@@ -2427,9 +2427,11 @@ static int ip_route_output_slow(struct r
fl.oif = dev_out->ifindex;
goto make_route;
}
- if (dev_out)
+ if (dev_out) {
dev_put(dev_out);
- dev_out = NULL;
+ dev_out = NULL;
+ } else if (!(oldflp->flags & FLOWI_FLAG_ANYSRC))
+ goto out;
}
Or we can go further and to avoid ip_dev_find? For me, this
second variant is preferred because calling ip_dev_find() is useless for
FLOWI_FLAG_ANYSRC.
@@ -2394,11 +2394,6 @@ static int ip_route_output_slow(struct r
ZERONET(oldflp->fl4_src))
goto out;
- /* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */
- dev_out = ip_dev_find(oldflp->fl4_src);
- if (dev_out == NULL)
- goto out;
-
/* I removed check for oif == dev_out->oif here.
It was wrong for two reasons:
1. ip_dev_find(saddr) can return wrong iface, if saddr is
@@ -2409,6 +2404,11 @@ static int ip_route_output_slow(struct r
if (oldflp->oif == 0
&& (MULTICAST(oldflp->fl4_dst) || oldflp->fl4_dst ==
htonl(0xFFFFFFFF))) {
+ /* It is equivalent to inet_addr_type(saddr) ==
RTN_LOCAL */
+ dev_out = ip_dev_find(oldflp->fl4_src);
+ if (dev_out == NULL)
+ goto out;
+
/* Special hack: user can direct multicasts
and limited broadcast via necessary interface
without fiddling with IP_MULTICAST_IF or IP_PKTINFO.
@@ -2427,9 +2427,14 @@ static int ip_route_output_slow(struct r
fl.oif = dev_out->ifindex;
goto make_route;
}
- if (dev_out)
+ if (!(oldflp->flags & FLOWI_FLAG_ANYSRC)) {
+ /* It is equivalent to inet_addr_type(saddr) ==
RTN_LOCAL */
+ dev_out = ip_dev_find(oldflp->fl4_src);
+ if (dev_out == NULL)
+ goto out;
dev_put(dev_out);
- dev_out = NULL;
+ dev_out = NULL;
+ }
}
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
