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

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

[Adam Seering]

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 unloaded" behavior match the "module was never compiled" behavior.

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

---

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?


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.

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

[David Miller]

New features are not allowed to be submitted at this time because
the net-next tree is closed.

Please pay attention to the netdev mailing list announcements to
learn when the net-next tree is open again and new features and
cleanups are allowed to be submitted again.

Thanks.

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

[Adam Seering]

> New features are not allowed to be submitted at this time because
> the net-next tree is closed.

Thanks for explaining; apologies.  I was following the instructions in
"Documentation/SubmittingPatches".

> 
> Please pay attention to the netdev mailing list announcements to
> learn when the net-next tree is open again and new features and
> cleanups are allowed to be submitted again.

Is the cadence described in "Documentation/networking/netdev-FAQ.txt"
accurate and sufficient?  (netdev is much too high-volume for me,
unfortunately.)

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

[David Miller]

From: Adam Seering <adam@seering.org>
Date: Sun, 17 Jan 2016 20:16:22 -0500

> (netdev is much too high-volume for me, unfortunately.)

Seriously?  How in the world do you think it is for me?

You can skim through and read only the announcements, you don't have
to read every single posting like I do.

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

[Adam Seering]

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?


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.

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

[David Miller]

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.

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

[Adam Seering]

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: [PATCH] appletalk: Pass IP-over-DDP packets through when 'ipddp0' interface is not present

[Adam Seering]

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

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

[Adam Seering]

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