diff mbox

appletalk: Pass IP-over-DDP packets through when 'ipddp0' interface is not present

Message ID 1456237153.4577.35.camel@seering.org
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Adam Seering Feb. 23, 2016, 2:19 p.m. UTC
Let userspace programs transmit and receive raw IP-over-DDP packets
with a kernel where "ipddp" was compiled as a module but is not loaded
(so no "ipddp0" network interface is exposed).  This makes the "module
is not loaded" behavior match the "module was never compiled" behavior.

Signed-off-by: Adam Seering <adam@seering.org>

---

[edit] Re-sending at hopefully a more-appropriate time.

This is a small proposed change to the ddp code.  It's also my first
attempt at a kernel patch; I'd appreciate any feedback or discussion. 
I'm working on the Linux "macipgw" port; it duplicates some
functionality of the "ipddp" module, but the latter depends on
userspace utilities which I can no longer find up-to-date working
copies of?

Comments

David Miller Feb. 25, 2016, 7:33 p.m. UTC | #1
From: Adam Seering <adam@seering.org>
Date: Tue, 23 Feb 2016 09:19:13 -0500

> Let userspace programs transmit and receive raw IP-over-DDP packets
> with a kernel where "ipddp" was compiled as a module but is not loaded
> (so no "ipddp0" network interface is exposed).  This makes the "module
> is not loaded" behavior match the "module was never compiled" behavior.
> 
> Signed-off-by: Adam Seering <adam@seering.org>

I think a better approache is to somehow autoload the module.
Adam Seering Feb. 26, 2016, 12:46 a.m. UTC | #2
On Thu, 2016-02-25 at 14:33 -0500, David Miller wrote:
> From: Adam Seering <adam@seering.org>
> Date: Tue, 23 Feb 2016 09:19:13 -0500
> 
> > Let userspace programs transmit and receive raw IP-over-DDP packets
> > with a kernel where "ipddp" was compiled as a module but is not
> loaded
> > (so no "ipddp0" network interface is exposed).  This makes the
> "module
> > is not loaded" behavior match the "module was never compiled"
> behavior.
> > 
> > Signed-off-by: Adam Seering <adam@seering.org>
> 
> I think a better approache is to somehow autoload the module.

Could you elaborate?  Specifically: the kernel currently suppresses
packets on behalf of the module even after the module is unloaded.  How
would autoloading the module help with that?
Adam Seering Feb. 27, 2016, 4:49 a.m. UTC | #3
On Thu, 2016-02-25 at 19:46 -0500, Adam Seering wrote:
> On Thu, 2016-02-25 at 14:33 -0500, David Miller wrote:
> > From: Adam Seering <adam@seering.org>
> > Date: Tue, 23 Feb 2016 09:19:13 -0500
> > 
> > > Let userspace programs transmit and receive raw IP-over-DDP
> > > packets
> > > with a kernel where "ipddp" was compiled as a module but is not
> > loaded
> > > (so no "ipddp0" network interface is exposed).  This makes the
> > "module
> > > is not loaded" behavior match the "module was never compiled"
> > behavior.
> > > 
> > > Signed-off-by: Adam Seering <adam@seering.org>
> > 
> > I think a better approache is to somehow autoload the module.
> 
> Could you elaborate?  Specifically: the kernel currently suppresses
> packets on behalf of the module even after the module is unloaded. 
>  How
> would autoloading the module help with that?

Re-reading this thread -- perhaps I didn't explain the problem well. 
 Let me elaborate.  Apologies if this is obvious to folks here:

I want my userspace program to send and receive DDP packets that
encapsulate IP traffic.

Problem:  On some kernel builds, these DDP packets are never delivered
to the DDP socket opened by my program.

The "ipddp" module is supposed to prevent those packets from being
delivered to DDP sockets when it is loaded -- it handles them itself. 
 Ok, that's fine; I just want to unload that module, right?

Wrong!  Unloading the module is not sufficient.  I have to re-compile
the kernel with the module disabled completely.  (No other config
options; simply setting the module to not build does the trick.)
whose sole purpose is to handle it.  If not, unload it.  This patch
makes that happen.  Thoughts?

Thanks,
Adam
Adam Seering March 20, 2016, 9:19 p.m. UTC | #4
On Thu, 2016-02-25 at 19:46 -0500, Adam Seering wrote:
> On Thu, 2016-02-25 at 14:33 -0500, David Miller wrote:
> > From: Adam Seering <adam@seering.org>
> > Date: Tue, 23 Feb 2016 09:19:13 -0500
> > 
> > > Let userspace programs transmit and receive raw IP-over-DDP
> > > packets
> > > with a kernel where "ipddp" was compiled as a module but is not
> > loaded
> > > (so no "ipddp0" network interface is exposed).  This makes the
> > "module
> > > is not loaded" behavior match the "module was never compiled"
> > behavior.
> > > 
> > > Signed-off-by: Adam Seering <adam@seering.org>
> > 
> > I think a better approache is to somehow autoload the module.
> 
> Could you elaborate?  Specifically: the kernel currently suppresses
> packets on behalf of the module even after the module is unloaded. 
>  How
> would autoloading the module help with that?
> 

I realize and appreciate that I'm not supposed to nag you folks for
feedback.  But I don't know how to proceed.  So, rather than nag you,
I've filed a bug:

https://bugzilla.kernel.org/show_bug.cgi?id=115031

I've tested my patch; I believe it to be the simplest change that
resolves this bug.  I'm happy to write something else if it would be
cleaner and/or help others, but I would need a more-detailed
explanation of what you're looking for.

I've also filed a ticket downstream:  Red Hat appears to not build the
ipddp module with their stock kernel release, so their users don't hit
this bug.  But Ubuntu does build/ship ipddp; if they likewise didn't,
that would also solve my problem:

https://bugs.launchpad.net/ubuntu/+source/linux-lts-wily/+bug/1559772

Adam
diff mbox

Patch

diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index d5871ac..d30e55f 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1284,17 +1284,10 @@  static __inline__ int is_ip_over_ddp(struct sk_buff *skb)
        return skb->data[12] == 22;
 }
 
-static int handle_ip_over_ddp(struct sk_buff *skb)
+static int handle_ip_over_ddp(struct sk_buff *skb, struct net_device *dev)
 {
-       struct net_device *dev = __dev_get_by_name(&init_net, "ipddp0");
        struct net_device_stats *stats;
 
-       /* This needs to be able to handle ipddp"N" devices */
-       if (!dev) {
-               kfree_skb(skb);
-               return NET_RX_DROP;
-       }
-
        skb->protocol = htons(ETH_P_IP);
        skb_pull(skb, 13);
        skb->dev   = dev;
@@ -1308,7 +1301,7 @@  static int handle_ip_over_ddp(struct sk_buff *skb)
 #else
 /* make it easy for gcc to optimize this test out, i.e. kill the code */
 #define is_ip_over_ddp(skb) 0
-#define handle_ip_over_ddp(skb) 0
+#define handle_ip_over_ddp(skb, dev) 0
 #endif
 
 static int atalk_route_packet(struct sk_buff *skb, struct net_device *dev,
@@ -1418,6 +1411,8 @@  static int atalk_rcv(struct sk_buff *skb, struct net_device *dev,
        struct sock *sock;
        struct atalk_iface *atif;
        struct sockaddr_at tosat;
+       struct net_device *ipddp_dev;
+
        int origlen;
        __u16 len_hops;
 
@@ -1473,9 +1468,14 @@  static int atalk_rcv(struct sk_buff *skb, struct net_device *dev,
                return atalk_route_packet(skb, dev, ddp, len_hops, origlen);
        }
 
-       /* if IP over DDP is not selected this code will be optimized out */
-       if (is_ip_over_ddp(skb))
-               return handle_ip_over_ddp(skb);
+       /* if IP over DDP is not selected this code should be optimized out */
+       if (is_ip_over_ddp(skb)) {
+               ipddp_dev = __dev_get_by_name(&init_net, "ipddp0");
+
+               /* This needs to be able to handle ipddp"N" devices */
+               if (ipddp_dev)
+                       return handle_ip_over_ddp(skb, ipddp_dev);
+       }
        /*
         * Which socket - atalk_search_socket() looks for a *full match*
         * of the <net, node, port> tuple.