On Fri, Jun 18, 2004 at 05:15:58PM +0100, Christoph Hellwig wrote:
> Last time I talked to Paul on MPPE he didn't like those hacks in
> the ppp core.
Adding the linux-ppp list to the discussion.
Rather than have code in ppp_generic.c testing for CI_MPPE and doing
code conditionally (which I agree was ugly), how about adding two new
fields to struct compressor:
+ /* Extra skb space needed by the compressor algorithm */
+ unsigned int comp_skb_extra_space;
+ /* Extra skb space needed by the decompressor algorithm */
+ unsigned int decomp_skb_extra_space;
which the compressor modules can fill in if needed? Presently,
bsd_comp.c and ppp_deflate.c don't need these, so they will be filled
with zeros automatically at struct compressor instantiation.
This hunk may still be contriversial though. It adds a negative
return value to the compress() method, which is only (presently) used
by ppp_mppe (bsd_comp.c and ppp_deflate.c always return 0 if they
couldn't compress), to indicate the frame should be dropped.
- } else {
+ } else if (len == 0) {
/* didn't compress, or CCP not up yet */
kfree_skb(new_skb);
+ } else {
+ /*
+ * (len < 0)
+ * MPPE requires that we do not send unencrypted
+ * frames. The compressor will return -1 if we
+ * should drop the frame. We cannot simply test
+ * the compress_proto because MPPE and MPPC share
+ * the same number.
+ */
+ printk(KERN_ERR "ppp: compressor dropped pkt\n");
+ kfree_skb(new_skb);
+ goto drop;
Thoughts?
Patch below against 2.6.7-bk ppp_generic.c and ppp-comp.h to add such,
for comment only.
Thanks,
Matt
--
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com
===== drivers/net/ppp_generic.c 1.45 vs edited =====
--- 1.45/drivers/net/ppp_generic.c 2004-04-09 18:21:06 -05:00
+++ edited/drivers/net/ppp_generic.c 2004-06-21 10:56:34 -05:00
@@ -1066,8 +1066,9 @@
/* try to do packet compression */
if ((ppp->xstate & SC_COMP_RUN) && ppp->xc_state != 0
&& proto != PPP_LCP && proto != PPP_CCP) {
- new_skb = alloc_skb(ppp->dev->mtu + ppp->dev->hard_header_len,
- GFP_ATOMIC);
+ int new_skb_size = ppp->dev->mtu +
ppp->xcomp->comp_skb_extra_space + ppp->dev->hard_header_len;
+ int compressor_skb_size = ppp->dev->mtu +
ppp->xcomp->comp_skb_extra_space + PPP_HDRLEN;
+ new_skb = alloc_skb(new_skb_size, GFP_ATOMIC);
if (new_skb == 0) {
printk(KERN_ERR "PPP: no memory (comp pkt)\n");
goto drop;
@@ -1079,15 +1080,27 @@
/* compressor still expects A/C bytes in hdr */
len = ppp->xcomp->compress(ppp->xc_state, skb->data - 2,
new_skb->data, skb->len + 2,
- ppp->dev->mtu + PPP_HDRLEN);
+ compressor_skb_size);
if (len > 0 && (ppp->flags & SC_CCP_UP)) {
kfree_skb(skb);
skb = new_skb;
skb_put(skb, len);
skb_pull(skb, 2); /* pull off A/C bytes */
- } else {
+ } else if (len == 0) {
/* didn't compress, or CCP not up yet */
kfree_skb(new_skb);
+ } else {
+ /*
+ * (len < 0)
+ * MPPE requires that we do not send unencrypted
+ * frames. The compressor will return -1 if we
+ * should drop the frame. We cannot simply test
+ * the compress_proto because MPPE and MPPC share
+ * the same number.
+ */
+ printk(KERN_ERR "ppp: compressor dropped pkt\n");
+ kfree_skb(new_skb);
+ goto drop;
}
}
@@ -1596,7 +1609,7 @@
goto err;
if (proto == PPP_COMP) {
- ns = dev_alloc_skb(ppp->mru + PPP_HDRLEN);
+ ns = dev_alloc_skb(ppp->mru +
ppp->rcomp->decomp_skb_extra_space + PPP_HDRLEN);
if (ns == 0) {
printk(KERN_ERR "ppp_decompress_frame: no memory\n");
goto err;
===== include/linux/ppp-comp.h 1.4 vs edited =====
--- 1.4/include/linux/ppp-comp.h 2003-08-07 18:57:19 -05:00
+++ edited/include/linux/ppp-comp.h 2004-06-21 10:59:44 -05:00
@@ -111,6 +111,10 @@
/* Used in locking compressor modules */
struct module *owner;
+ /* Extra skb space needed by the compressor algorithm */
+ unsigned int comp_skb_extra_space;
+ /* Extra skb space needed by the decompressor algorithm */
+ unsigned int decomp_skb_extra_space;
};
/*
pgphtlzZY0C7s.pgp
Description: PGP signature
|