Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Talpey, Thomas]

At 12:57 PM 8/12/2005, Hal Rosenstock wrote:
>Using old arping on IPoIB will get the error on the sendto as the
>hardware type is not available at bind time.

Okay, that's a feature then, instead of "Bus Error - core dumped"
when 20 bytes land on top of 8, they'll get a send failure. :-)

Tom.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Hal Rosenstock]

On Fri, 2005-08-12 at 12:40, Talpey, Thomas wrote:
> Note: Hal's change requires arping to be recompiled too!
> Can't stick 20 bytes into 8 there, either.

Not quite. Old arping binaries will work for non IPoIB links just fine.
Using old arping on IPoIB will get the error on the sendto as the
hardware type is not available at bind time.

-- Hal

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Talpey, Thomas]

At 12:21 PM 8/12/2005, Tom Duffy wrote:
>Can we do an audit of what stuff will break with this change?  If it  
>is a handful of applications that we all have the source to, maybe it  
>won't be that big of a deal.

Right now it looks to me like the app is arping, and it can be fixed
by increasing the size of the storage it allocates in its data segment,
without changing the sockaddr_ll.

Maybe others, haven't bothered to look.

struct sockaddr_ll me -> union { struct sockaddr_ll xx; unsigned char yy[32]; } 
me;

Note: Hal's change requires arping to be recompiled too!
Can't stick 20 bytes into 8 there, either.

Tom.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Christoph Hellwig]

On Fri, Aug 12, 2005 at 09:21:29AM -0700, Tom Duffy wrote:
> I don't want to get into a big debate about this.  If a good solution  
> can be had that will both maintain compatibility and allow for IB, I  
> would welcome that.  On the other hand, most of the interesting apps  
> have broken on Linux in the past few years.  Some examples:
> 
> - Loki games
> - Word Perfect 8
> - Crossover office/plugin
> - java

s/interesting apps/crappy shit that relies on undefined behaviour/

I can still run most of some really old a.out slackware, including
the original doom port on my only x86 box (binfmt_aout complains a lot
about some missing alignment and stuff, though ;))

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Tom Duffy]

On Aug 12, 2005, at 9:07 AM, Roland Dreier wrote:


Tom> But, Fedora will rebuild their binary once this change is in.
Tom> If the Linux developers cared about this sort of thing, it
Tom> would version all its kernel structs and put padding at the
Tom> end to ensure new fields could be added.  It has opted for
Tom> the cleaner (technical) solution of having all the apps
Tom> recompile.  Sure there will be a little bit of growing pain,
Tom> but in the end, it won't have all kinds of backwards
Tom> compatibility cruft lying around.

No, this is absolutely not true.  The kernel-user ABI is very stable,
and with very few exceptions, you should be able to take binaries that
worked on kernel 1.0 and run them on a modern kernel.  For example,


The in-kernel ABI and API can and do change all the time, but that's a
different story.


I don't want to get into a big debate about this.  If a good solution  
can be had that will both maintain compatibility and allow for IB, I  
would welcome that.  On the other hand, most of the interesting apps  
have broken on Linux in the past few years.  Some examples:


- Loki games
- Word Perfect 8
- Crossover office/plugin
- java

I know that lots of that has to do with gcc, threading, or glibc  
instability, but clearly most interesting binaries that were around  
in the 1.0 days will not run on todays stuff.


Can we do an audit of what stuff will break with this change?  If it  
is a handful of applications that we all have the source to, maybe it  
won't be that big of a deal.


Maybe the better approach is to simply submit the struct change.  And  
let the maintainers object if they want ABI stability.   If they do,  
ask them for an elegant solution ;)


-tduffy


___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Roland Dreier]

Tom> But, Fedora will rebuild their binary once this change is in.
Tom> If the Linux developers cared about this sort of thing, it
Tom> would version all its kernel structs and put padding at the
Tom> end to ensure new fields could be added.  It has opted for
Tom> the cleaner (technical) solution of having all the apps
Tom> recompile.  Sure there will be a little bit of growing pain,
Tom> but in the end, it won't have all kinds of backwards
Tom> compatibility cruft lying around.

No, this is absolutely not true.  The kernel-user ABI is very stable,
and with very few exceptions, you should be able to take binaries that
worked on kernel 1.0 and run them on a modern kernel.  For example,


The in-kernel ABI and API can and do change all the time, but that's a
different story.

 - R.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Tom Duffy]

On Aug 12, 2005, at 7:51 AM, Roland Dreier wrote:


Tom> Do we need backward compatibility?

Hal> I'm not sure but I think this was a recommendation from
Hal> Roland.

Yes, of course we need backward compatibility.  We can't put a change
into the kernel that breaks userspace binaries.

For example, the old Fedora binary of arping has to continue to work,
even if you use a new kernel.


But, Fedora will rebuild their binary once this change is in.  If the  
Linux developers cared about this sort of thing, it would version all  
its kernel structs and put padding at the end to ensure new fields  
could be added.  It has opted for the cleaner (technical) solution of  
having all the apps recompile.  Sure there will be a little bit of  
growing pain, but in the end, it won't have all kinds of backwards  
compatibility cruft lying around.


-tduffy
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Hal Rosenstock]

On Fri, 2005-08-12 at 10:32, Talpey, Thomas wrote:
> At 10:12 AM 8/12/2005, Hal Rosenstock wrote:
> >If sockaddr_ll struct is left alone, I think it may be a problem on the
> >receive side where the size of that struct is used.
> 
> Maybe. The receive side builds the incoming sockaddr_ll in the skb->cb.
> But that's 48 bytes and it goes off to your device's hard_header_parse
> to do so...
> 
> You sure you have hard_header_len and all the appropriate vectors
> set up properly? (netdevice.h)

Yes, this is done by ipoib_setup in ipoib_main.c which is called when an
IPoIB port is added as follows:

dev = alloc_netdev((int) sizeof (struct ipoib_dev_priv), name,
   ipoib_setup);

-- Hal

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Roland Dreier]

Tom> Do we need backward compatibility?

Hal> I'm not sure but I think this was a recommendation from
Hal> Roland.

Yes, of course we need backward compatibility.  We can't put a change
into the kernel that breaks userspace binaries.

For example, the old Fedora binary of arping has to continue to work,
even if you use a new kernel.

 - R.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Hal Rosenstock]

On Fri, 2005-08-12 at 10:12, Hal Rosenstock wrote:
> If sockaddr_ll struct is left alone, I think it may be a problem on the
> receive side where the size of that struct is used.

One further thought on this possible approach:

Even if this does work, it seems to me that this pushes all the hokiness
back on the applications which use sockaddr_ll. For example, with the
struct change and a trivial change to arping.c, this now works for
IPoIB. If the structure were not to be changed, there would be a lot of
IPoIB specific changes to each application.

-- Hal

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Talpey, Thomas]

At 10:12 AM 8/12/2005, Hal Rosenstock wrote:
>If sockaddr_ll struct is left alone, I think it may be a problem on the
>receive side where the size of that struct is used.

Maybe. The receive side builds the incoming sockaddr_ll in the skb->cb.
But that's 48 bytes and it goes off to your device's hard_header_parse
to do so...

You sure you have hard_header_len and all the appropriate vectors
set up properly? (netdevice.h)

Tom.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Hal Rosenstock]

On Fri, 2005-08-12 at 09:52, Talpey, Thomas wrote:
> At 09:01 AM 8/12/2005, Hal Rosenstock wrote:
> >It is done to preserve length checks that were already there (on struct
> >msghdr in packet_sendmsg and addr_len in packet_bind). I didn't want to
> >weaken that.
> 
> Are you sure things break if you simply build a message in user space
> that's got the larger address (without changing the sockaddr_ll at all)?
> It looks to me as if msg->msg_namelen/msg_name can be any appropriate
> size which is at least as large as the sockaddr_ll.

I think that's where I started on this. I didn't change sockaddr_ll and
it didn't work for IPoIB link level messages but at this point, I'm no
longer 100% sure so I will check again (it may have been due to some
other problem).

If sockaddr_ll struct is left alone, I think it may be a problem on the
receive side where the size of that struct is used.

-- Hal

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Talpey, Thomas]

At 09:01 AM 8/12/2005, Hal Rosenstock wrote:
>It is done to preserve length checks that were already there (on struct
>msghdr in packet_sendmsg and addr_len in packet_bind). I didn't want to
>weaken that.

Are you sure things break if you simply build a message in user space
that's got the larger address (without changing the sockaddr_ll at all)?
It looks to me as if msg->msg_namelen/msg_name can be any appropriate
size which is at least as large as the sockaddr_ll.

Tom.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Hal Rosenstock]

On Thu, 2005-08-11 at 23:52, Tom Duffy wrote:
> On Aug 11, 2005, at 2:38 PM, Hal Rosenstock wrote:
> > Can anyone think of another approach to do this and keep backward
> > compatibility ?
> 
> Do we need backward compatibility?  

I'm not sure but I think this was a recommendation from Roland. It
certainly would be better if we could. I don't know whether this is a
requirement of the solution or not or just desirable.

> How about the stuff that includes  
> if_packet.h gets rebuilt?  You are adding to the end of the struct,  
> after all.

If one can assume the applications are rebuilt, I think that makes life
easier as the IPoIB specific checks can be removed. I think that was the
primary technical objection (aside from the mechanical one).

-- Hal

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Hal Rosenstock]

On Thu, 2005-08-11 at 23:52, Tom Duffy wrote:
> On Aug 11, 2005, at 2:38 PM, Hal Rosenstock wrote:
> > Can anyone think of another approach to do this and keep backward
> > compatibility ?
> 
> Do we need backward compatibility?  

I'm not sure but I think this was a recommendation from Roland.

> How about the stuff that includes  
> if_packet.h gets rebuilt?  You are adding to the end of the struct,  
> after all.

If one can assume the applications are rebuilt, I think that makes life
easier as the IPoIB specific checks can be removed. I think that was the
primary technical objection (aside from the mechanical one).

-- Hal

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Hal Rosenstock]

On Fri, 2005-08-12 at 07:52, Talpey, Thomas wrote:
> At 11:52 PM 8/11/2005, Tom Duffy wrote:
> >
> >On Aug 11, 2005, at 2:38 PM, Hal Rosenstock wrote:
> >> Can anyone think of another approach to do this and keep backward
> >> compatibility ?
> >
> >Do we need backward compatibility?  How about the stuff that includes  
> >if_packet.h gets rebuilt?  You are adding to the end of the struct,  
> >after all.
> 
> The size of the struct is less of an issue than the test for
> ARPHRD_INFINIBAND. David said as much:
> 
> -- it won't work for anything else without adding
> -- more special tests to that af_packet.c code
> 
> I have to say, SOCKADDR_COMPAT_LL is pretty stinky too.

SOCKADD_COMPAT_LL is to support backwards compatibility (on binaries
built with the old header with only 8 byte sll_addr in sockaddr_ll
struct).

> Hal, why *are* you testing for ARPHRD_INFINIBAND anyway?
> What different action happens in the transport-independent
> code in this special case?

It is done to preserve length checks that were already there (on struct
msghdr in packet_sendmsg and addr_len in packet_bind). I didn't want to
weaken that. In packet_sendmsg, the check is that the message is not
shorter than the link level header. For bind, the check is that the
address length bound to the socket is accurate for the link layer being
used. Those parameters come from sendto and bind calls in the
application.

-- Hal

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Talpey, Thomas]

At 11:52 PM 8/11/2005, Tom Duffy wrote:
>
>On Aug 11, 2005, at 2:38 PM, Hal Rosenstock wrote:
>> Can anyone think of another approach to do this and keep backward
>> compatibility ?
>
>Do we need backward compatibility?  How about the stuff that includes  
>if_packet.h gets rebuilt?  You are adding to the end of the struct,  
>after all.

The size of the struct is less of an issue than the test for
ARPHRD_INFINIBAND. David said as much:

-- it won't work for anything else without adding
-- more special tests to that af_packet.c code

I have to say, SOCKADDR_COMPAT_LL is pretty stinky too.

Hal, why *are* you testing for ARPHRD_INFINIBAND anyway?
What different action happens in the transport-independent
code in this special case?

Tom.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Tom Duffy]

On Aug 11, 2005, at 2:38 PM, Hal Rosenstock wrote:

Can anyone think of another approach to do this and keep backward
compatibility ?


Do we need backward compatibility?  How about the stuff that includes  
if_packet.h gets rebuilt?  You are adding to the end of the struct,  
after all.


-tduffy
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

[openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[Hal Rosenstock]

On Thu, 2005-08-11 at 15:49, David S. Miller wrote:
> From: Hal Rosenstock <[EMAIL PROTECTED]>
> Date: 11 Aug 2005 14:48:37 -0400
> 
> > The patch below is to accomodate IPoIB link layer address in the
> > sockaddr_ll struct so that user space can send and receive IPoIB link
> > later packets. Unfortunately, IPoIB has 20 bytes LL addresses rather
> > than the 8 byte MAC addresses (or under) used by other LLs.
> 
> Two problems.  1) it's a really ugly IPoIB specific hack to extend the
> sockaddr_ll structure, it won't work for anything else without adding
> more special tests to that af_packet.c code 

> Please find another way to extend the structure.

Can anyone think of another approach to do this and keep backward
compatibility ?

-- Hal

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

[openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface

[David S. Miller]

From: Hal Rosenstock <[EMAIL PROTECTED]>
Date: 11 Aug 2005 14:48:37 -0400

> The patch below is to accomodate IPoIB link layer address in the
> sockaddr_ll struct so that user space can send and receive IPoIB link
> later packets. Unfortunately, IPoIB has 20 bytes LL addresses rather
> than the 8 byte MAC addresses (or under) used by other LLs.

Two problems.  1) it's a really ugly IPoIB specific hack to extend the
sockaddr_ll structure, it won't work for anything else without adding
more special tests to that af_packet.c code and 2) you inproperly
rooted your patch, so we get stuff like this:

> --- af_packet.c.orig  2005-06-29 19:00:53.0 -0400
> +++ af_packet.c   2005-08-05 13:28:49.0 -0400

Please find another way to extend the structure.

That's why I didn't respond, this patch was too ugly for words
so it went to the bottom of my prioritized list of things to
do.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general