On Sat, 14 Feb 2004, Mika Penttilä wrote:
> James Morris wrote:
>
> >The proposed solution below is to copy the skb rather than clone it, to
> >ensure that the original and looped back packets are independent.
> >
>
> This is unneeded overhead for the common case. The right fix is to make
> sure the modifier (netfilter etc) makes the copy if needed. Actually,
> this is what skb_ip_make_writable() is doing.
The common case here will be only for locally generated multicast and
broadcast packets.
If the netfilter core code is modified instead, we will end up adding
skb_ip_make_writable() to nf_hook_slow() which will be called for every
packet with an output device which uses hardware checksums.
Not sure which is worse, but here's a proposed patch which does this.
- James
--
James Morris
<jmorris@xxxxxxxxxx>
diff -urN -X dontdiff linux-2.6.3-rc2-mm1.o/include/linux/netfilter.h
linux-2.6.3-rc2-mm1.w2/include/linux/netfilter.h
--- linux-2.6.3-rc2-mm1.o/include/linux/netfilter.h 2004-02-03
22:43:48.000000000 -0500
+++ linux-2.6.3-rc2-mm1.w2/include/linux/netfilter.h 2004-02-15
00:16:30.000000000 -0500
@@ -162,6 +162,12 @@
/* FIXME: Before cache is ever used, this must be implemented for real. */
extern void nf_invalidate_cache(int pf);
+/* Call this before modifying an existing IP packet: ensures it is
+ modifiable and linear to the point you care about (writable_len).
+ Returns true or false. */
+extern int skb_ip_make_writable(struct sk_buff **pskb,
+ unsigned int writable_len);
+
#else /* !CONFIG_NETFILTER */
#define NF_HOOK(pf, hook, skb, indev, outdev, okfn) (okfn)(skb)
#endif /*CONFIG_NETFILTER*/
diff -urN -X dontdiff linux-2.6.3-rc2-mm1.o/include/linux/netfilter_ipv4.h
linux-2.6.3-rc2-mm1.w2/include/linux/netfilter_ipv4.h
--- linux-2.6.3-rc2-mm1.o/include/linux/netfilter_ipv4.h 2004-02-03
22:44:26.000000000 -0500
+++ linux-2.6.3-rc2-mm1.w2/include/linux/netfilter_ipv4.h 2004-02-15
00:16:13.000000000 -0500
@@ -78,11 +78,6 @@
extern int ip_route_me_harder(struct sk_buff **pskb);
-/* Call this before modifying an existing IP packet: ensures it is
- modifiable and linear to the point you care about (writable_len).
- Returns true or false. */
-extern int skb_ip_make_writable(struct sk_buff **pskb,
- unsigned int writable_len);
#endif /*__KERNEL__*/
#endif /*__LINUX_IP_NETFILTER_H*/
diff -urN -X dontdiff linux-2.6.3-rc2-mm1.o/net/core/netfilter.c
linux-2.6.3-rc2-mm1.w2/net/core/netfilter.c
--- linux-2.6.3-rc2-mm1.o/net/core/netfilter.c 2004-02-03 22:43:46.000000000
-0500
+++ linux-2.6.3-rc2-mm1.w2/net/core/netfilter.c 2004-02-15 00:57:05.000000000
-0500
@@ -509,6 +509,13 @@
if (outdev == NULL) {
skb->ip_summed = CHECKSUM_NONE;
} else {
+ int wlen = skb->h.raw - skb->data + skb->csum;
+
+ /* packet is about to be modified */
+ if (!skb_ip_make_writable(&skb, wlen)) {
+ kfree(skb);
+ return -ENOMEM;
+ }
skb_checksum_help(skb);
}
}
|