In message <Pine.LNX.4.44.0310051151430.1204-101000@xxxxxxxxxxxx> you write:
> Hello,
>
> Attached is a patch that includes the changes from
> Rusty about handling non-linear skbs correctly and modified
> a bit from Wensong Zhang and me. This is a huge patch that changes
> interfaces for many functions. It looks difficult to split it in
> parts but if required we can try to do it. It is ready for
> inclusion.
Hi Julian!
I diffed our two patches. Is see you still use iph temp vars:
fair enough: I removed mine after some subtle bugs (although it does
make the code a bit uglier).
Looks really good. Could you just clarify a couple of things for me please?
@@ -527,10 +528,7 @@ struct ip_vs_conn {
struct ip_vs_dest *dest; /* real server */
atomic_t in_pkts; /* incoming packet counter */
- /* packet transmitter for different forwarding methods. If it
- mangles the packet, it must return NF_DROP or NF_STOLEN, otherwise
- this must be changed to a sk_buff **.
- */
+ /* packet transmitter for different forwarding methods */
int (*packet_xmit)(struct sk_buff *skb, struct ip_vs_conn *cp,
struct ip_vs_protocol *pp);
This comment is still true: the skb pointer from the caller is
useless, so xmit *must* return NF_DROP or NF_STOLEN. I thought about
making it return void and the callers always return NF_STOLEN, but
there was enough change already. You might want to put a comment in
there.
ip_vs_make_skb_writable(): how is this different from
skb_ip_make_writable(), except you have to maintain it yourself? The
advantage of the general one is that Dave looks after it for us 8)
In ip_vs_out_icmp():
/* Is the embedded protocol header present? */
if (unlikely(ciph.frag_off & __constant_htons(IP_OFFSET) &&
- /* FIXME: Remove minhlen, and surely dont_defrag
- * test is backwards? --RR */
(pp->minhlen || pp->dont_defrag)))
return NF_ACCEPT;
If the protocol says "don't defrag" they never see fragmented packets.
AFAICT, minhlen and dont_defrag are now the same everywhere (minhlen
is not respected: protocols are expected to catch skb_copy_bits
failing on their own, and they do). Perhaps drop minhlen altogether,
and just keep dont_defrag?
Hey, thanks for doing the hard work for me!
Cheers,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
|