[Bug 1001656] New: FreeBSD: add AF_PACKET socket familiy

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] New: FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

           Summary: FreeBSD: add AF_PACKET socket familiy
           Product: eCos
           Version: CVS
          Platform: All
        OS/Version: All
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: high
         Component: Patches and contributions
        AssignedTo: [hidden email]
        ReportedBy: [hidden email]
                CC: [hidden email]
             Class: Advice Request


Hello,

this patch implements the Raw Ethernet Packet Sockets in the Free BSD stack.

That is a pretty cool feature which can be found nowhere else.

A fiew minor bugs are fixed too:
  - strict aliasing fixed (IPv6)
  - mbuf leaks
  - multicast output, when no gateway is configured
  - IFF_ALLMULTI changeable
  - added SIOCGIFINDEX as in linux
  - fixed SIOCADDMULTI/SIOCDELMULTI work as in linux
  - error handling in accept() looses socket resource
  - send() behaves different on SS_NBIO and MSG_DONTWAIT
  - packet loss every 20 minutes when ARP timeout expires
  - sbcompress() optimized to create less fragments

--
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

--- Comment #1 from Bernd Edlinger <[hidden email]> 2012-08-17 14:53:49 BST ---
Created an attachment (id=1900)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1900)
patch to implement packet sockets

--
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

--- Comment #2 from Bernd Edlinger <[hidden email]> 2012-08-17 16:11:56 BST ---
Regarding AF_PACKET:

The protocol AF_PACKET will only be available when you call
cyg_use_af_packet() somewhere in your application.

If this function is not called the af_packet.c is not linked and
the binary will not become any larger due to this enhancement.

After this is done, these sockets can be used exactly as described here:

man 7 packet

--
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

Bernd Edlinger <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1900|0                           |1
        is obsolete|                            |

--- Comment #3 from Bernd Edlinger <[hidden email]> 2012-09-14 15:08:25 BST ---
Created an attachment (id=1937)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1937)
patch to implement packet sockets, Updated.

Hello,

now I added a missing lock around sblock() and sbunlock().

This can cause problems if different threads use sendto() on the same socket.
or recv() and shutdown(SD_RECV) on the same socket at the same time.

See the discussion on [hidden email]: "BSD socket stall"‏

--
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

Bernd Edlinger <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1937|0                           |1
        is obsolete|                            |

--- Comment #4 from Bernd Edlinger <[hidden email]> ---
Created attachment 2268
  --> http://bugs.ecos.sourceware.org/attachment.cgi?id=2268&action=edit
patch to implement packet sockets and various bug-fixes.

Hello,

my previus change introduced an unbalanced splnet/splx sequence in sosend().

this will be fixed with the attached update of the patch.

the only change is:
moved the splx in sosend() inside the do { } while loop.

-     splx(s);
      mp = &top;
      space -= clen;
      do {
+          splx(s);

           ....

           s = splnet();                               /* XXX */

           ...

      while (resid && space > 0);

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

Juergen Lambrecht <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #5 from Juergen Lambrecht <[hidden email]> ---
(In reply to comment #2)

> Regarding AF_PACKET:
>
> The protocol AF_PACKET will only be available when you call
> cyg_use_af_packet() somewhere in your application.
>
> If this function is not called the af_packet.c is not linked and
> the binary will not become any larger due to this enhancement.
>
> After this is done, these sockets can be used exactly as described here:
>
> man 7 packet

Hi Bernd,

Because I am porting the busybox dhcp server to eCos, I am using your raw
packet patch.
When I do 'man 7 packet' on my linux, I get a slightly different definition of
'struct sockaddr_ll' (as also used by busybox): I have 'int sll_ifindex;'
instead of 'u_short sll_index;' and 'unsigned char sll_addr[8];' instead of
'u_char sll_addr[22];'.
I guess the naming difference is because your code is based on the freeBSD, and
the busybox is based on Linux.
But why 22 bytes for the address ('ssl_addr'), you only use 6B of it
(EHTER_ADDR_LEN)?

Kind regards,
Jürgen

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

--- Comment #6 from Bernd Edlinger <[hidden email]> ---
(In reply to comment #5)
Hi Jürgen,

> Because I am porting the busybox dhcp server to eCos, I am using your raw
> packet patch.
> When I do 'man 7 packet' on my linux, I get a slightly different definition
> of 'struct sockaddr_ll' (as also used by busybox): I have 'int sll_ifindex;'
> instead of 'u_short sll_index;' and 'unsigned char sll_addr[8];' instead of
> 'u_char sll_addr[22];'.
> I guess the naming difference is because your code is based on the freeBSD,
> and the busybox is based on Linux.
> But why 22 bytes for the address ('ssl_addr'), you only use 6B of it
> (EHTER_ADDR_LEN)?

good questions...

1. actually the name should be sll_ifindex. I somehow missed that typo.

2. the data type that is used by the bsd stack to index the interfaces
   is u_short, therefore I thought it would be better to use that instead.
   Same for the SIOCGIFINDEX ioctl, which uses only u_short.

3. in linux sizeof (struct sockaddr_ll) = 20 which is larger than
   sizeof(struct sockaddr) = 16.
   But on eCos the sockaddr is 32 bytes. Therefore the sockaddr_ll
   must be at least 32 bytes. Therefore I enlarged the sll_addr to 22.

Note: All socket addressses should be exactly 32 bytes in eCos,
because of this code in ./io/fileio/current/src/socket.cxx:

__externC int   bind (int s, const struct sockaddr *sa, unsigned int len)
{
...
    struct sockaddr sa2 = *sa;



regading 1, I will change the name.

regarding 2, I could change the type to int, and cast it
to u_short later, but only if that improves protability.

Is it your impression that this change would improve the
portability of the busybox dhcp server?

regarding 3, the sizeof sll_addr should be irrelevant to
the application as only 6 bytes are really used.

Does the existion code use the size of the sll_addr array
for anything? Does it break unless this array is exactly 8 bytes?


Regards
Bernd.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

--- Comment #7 from Juergen Lambrecht <[hidden email]> ---
(In reply to comment #6)

> (In reply to comment #5)
> Hi Jürgen,
>
> > Because I am porting the busybox dhcp server to eCos, I am using your raw
> > packet patch.
> > When I do 'man 7 packet' on my linux, I get a slightly different definition
> > of 'struct sockaddr_ll' (as also used by busybox): I have 'int sll_ifindex;'
> > instead of 'u_short sll_index;' and 'unsigned char sll_addr[8];' instead of
> > 'u_char sll_addr[22];'.
> > I guess the naming difference is because your code is based on the freeBSD,
> > and the busybox is based on Linux.
> > But why 22 bytes for the address ('ssl_addr'), you only use 6B of it
> > (EHTER_ADDR_LEN)?
>
> good questions...
>
> 1. actually the name should be sll_ifindex. I somehow missed that typo.
>
> 2. the data type that is used by the bsd stack to index the interfaces
>    is u_short, therefore I thought it would be better to use that instead.
>    Same for the SIOCGIFINDEX ioctl, which uses only u_short.
>
> 3. in linux sizeof (struct sockaddr_ll) = 20 which is larger than
>    sizeof(struct sockaddr) = 16.
>    But on eCos the sockaddr is 32 bytes. Therefore the sockaddr_ll
>    must be at least 32 bytes. Therefore I enlarged the sll_addr to 22.
>
> Note: All socket addressses should be exactly 32 bytes in eCos,
> because of this code in ./io/fileio/current/src/socket.cxx:
>
> __externC int   bind (int s, const struct sockaddr *sa, unsigned int len)
> {
> ...
>     struct sockaddr sa2 = *sa;
>
>
>
> regading 1, I will change the name.
OK, I did the same
>
> regarding 2, I could change the type to int, and cast it
> to u_short later, but only if that improves protability.
>
> Is it your impression that this change would improve the
> portability of the busybox dhcp server?
No, it's OK. The code compiled without warnings so far.
>
> regarding 3, the sizeof sll_addr should be irrelevant to
> the application as only 6 bytes are really used.
>
> Does the existion code use the size of the sll_addr array
> for anything? Does it break unless this array is exactly 8 bytes?
No. Busybox code also only uses 6B instead of the 8 they have.
So it is OK.

Thanks for your reply,
Jürgen
>
>
> Regards
> Bernd.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

Bernd Edlinger <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2268|0                           |1
        is obsolete|                            |

--- Comment #8 from Bernd Edlinger <[hidden email]> ---
Created attachment 2337
  --> http://bugs.ecos.sourceware.org/attachment.cgi?id=2337&action=edit
patch to implement packet sockets and various bug-fixes

renamed sll_index to sll_ifindex due to recent discussion.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

--- Comment #9 from D.Zebralla <[hidden email]> ---
Created attachment 2388
  --> http://bugs.ecos.sourceware.org/attachment.cgi?id=2388&action=edit
Simple test to send a raw ethernet packet and to receive a raw ethernet packet

Hello,

I just wanted to confirm that this patch is working, applied to eCos revision
3272 (checked out from ecoscentric mercurial repository).

I attached a c-file with two simple functions:
* sendPacket() - sends a raw packet with a given ethernet type
* receivePacket() - busy wait until a packet is received with the given
ethernet type

I DID NOT test the effects of the additional bug fixes, so I can not give
feedback on those!

I would like to see this patch included into the ecos repository sometime soon!

Regards
- Daniel

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

--- Comment #10 from Juergen Lambrecht <[hidden email]> ---
(In reply to comment #9)
> Created attachment 2388 [details]
> Simple test to send a raw ethernet packet and to receive a raw ethernet
> packet
>
> Hello,
>
> I just wanted to confirm that this patch is working, applied to eCos
> revision 3272 (checked out from ecoscentric mercurial repository).
I also want to confirm this patch works for me. I updated to the ecos CVS
repository from 7/2013.
>
> I attached a c-file with two simple functions:
> * sendPacket() - sends a raw packet with a given ethernet type
> * receivePacket() - busy wait until a packet is received with the given
> ethernet type
>
> I DID NOT test the effects of the additional bug fixes, so I can not give
> feedback on those!
Same for me, but we are testing 3 applications with it for the past months and
no eCos issues so far.
>
> I would like to see this patch included into the ecos repository sometime
> soon!
same for me
>
> Regards
> - Daniel

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

--- Comment #11 from D.Zebralla <[hidden email]> ---
I stumbled across a problem sending normal UDP packets with this patch.
I'm not sure whether the problem is introduced by this patch or our way of
opening a UDP socket is wrong.

I'm referring to this change of the patch:

diff -Nur ecos-cvs-120723/packages/net/bsd_tcpip/current/src/sys/kern/sockio.c
ecos/packages/net/bsd_tcpip/current/src/sys/kern/sockio.c
--- ecos-cvs-120723/packages/net/bsd_tcpip/current/src/sys/kern/sockio.c  
2009-01-29 18:49:56.000000000 +0100
+++ ecos/packages/net/bsd_tcpip/current/src/sys/kern/sockio.c    2012-08-02
10:15:18.000000000 +0200
@@ -234,7 +234,8 @@
 {
     int error;
     sockaddr sa1=*sa;
-    
+    sa1.sa_len = len;
+
     error = sobind((struct socket *)fp->f_data, (sockaddr *)&sa1, 0);
     return error;
 }


We're starting the UDP connection like this (error checking omitted for
readability):
    memset(&hints, 0, sizeof hints); // make sure the struct is empty
    hints.ai_family = AF_INET6; // IPv6
    hints.ai_socktype = SOCK_DGRAM; // UDP
    hints.ai_flags = AI_PASSIVE; // fill in my IP for me

    getaddrinfo(NULL, "12345", &hints, &servinfo);

    acceptSocket = socket(servinfo->ai_family, servinfo->ai_socktype,
servinfo->ai_protocol);

    setsockopt(acceptSocket, SOL_SOCKET, SO_REUSEADDR, (char *)&on,sizeof(on);

    bind(acceptSocket, servinfo->ai_addr, servinfo->ai_addrlen);

servinfo->ai_addrlen was set to 32 Bytes (size of struct sockaddr) inside
alloc_addrinfo-function:
    struct sockaddr * sa;
    [...]
    nai->ai_addrlen = sizeof(*sa);


Due to the patch inside bsd_bind sa1.sa_len is now changed from 28 (size of
struct sockaddr_in6) to 32 (size of struct sockaddr).

After finally arriving in the udp6_output function, the following sanity check
now fails because addr6->m_len is now 32 instead of 28:
    if (addr6) {
    [...]
        sin6 = mtod(addr6, struct sockaddr_in6 *);

        if (addr6->m_len != sizeof(*sin6))
            return(EINVAL);
    [...]
    }


Can someone please confirm this or point me to errors we may have made?

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

--- Comment #12 from Bernd Edlinger <[hidden email]> ---
Hi,

thanks for reporting this.
I had not done much testing in the IPv6 area.

The issue with the bind is, that

 sockaddr sa1=*sa;

copies exacly 32 bytes. So all sockaddr_xx structures should
have at least 32 bytes, otherwise we get access beyound the end
at this point.

I had not seen that sockaddr_in6 is shorter than sockaddr.
that looks wrong indeed.

I'd suggest you try to make sockaddr 4 bytes larger

struct sockaddr_in6 {
        u_int8_t        sin6_len;       /* length of this struct(sa_family_t)*/
        u_int8_t        sin6_family;    /* AF_INET6 (sa_family_t) */
        u_int16_t       sin6_port;      /* Transport layer port # (in_port_t)*/
        u_int32_t       sin6_flowinfo;  /* IP6 flow information */
        struct in6_addr sin6_addr;      /* IP6 address */
        u_int32_t       sin6_scope_id;  /* scope zone index */
        char            sin6_zero[4];   /* padding */
};

does this work?

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

--- Comment #13 from D.Zebralla <[hidden email]> ---
After the first quick test: Yes, it looks like the IPv6 UDP packet is now sent.

However, I am wondering if it's really the right thing to stuff padding bytes
into sockaddr_in6. I am not familiar with the whole networking code stuff, but
I saw that in the Windows world and partly in various unix flavors 28 Bytes for
sockaddr_in6 or even little less seem common.

Is there any RFC, ISO or POSIX standard where it's said that these structures
should have the same size?

What about this sockaddr_storage struct?

On a sidenote: Why was there even a 'len'-parameter in bsd_bind() when it was
not used up until your patch?

Thanks in advance!

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

--- Comment #14 from Bernd Edlinger <[hidden email]> ---
(In reply to comment #13)
> After the first quick test: Yes, it looks like the IPv6 UDP packet is now
> sent.

Good.

> However, I am wondering if it's really the right thing to stuff
> padding bytes into sockaddr_in6. I am not familiar with the whole networking
> code stuff, but I saw that in the Windows world and partly in various unix
> flavors 28 Bytes for sockaddr_in6 or even little less seem common.
> Is there
> any RFC, ISO or POSIX standard where it's said that these structures should
> have the same size?

probably not. The goal is to be compatible to other O/S where possible.
However for eCos an exact binary compatiblity is not required.
Even the defines for AF_INET and AF_INET6 may be different from what they
are on linux.

For whatever reason eCos chose to enlarge struct sockaddr from 16 bytes
(as it is on linux) to 32 bytes. I always assumed that was a smart move,
which was done to make struct sockaddr large enough to hold a sockaddr_in6,
but I never checked that, and I did not much testing with IPv6 either...

Well, struct sockaddr is the base-class of all socket types,
and therefore all other socket types should be at least as large as 32 bytes.

As a consequence sockaddr_in has a padding enlarged from 8 (as it is on linux)
enlarged to 24. But sockaddr_in6 should have done the same.

> sidenote: Why was there even a 'len'-parameter in bsd_bind() when it was not
> used up until your patch?

Definitely a BUG.

Fact is:
The code in bsd_bind does _only_ work under the assumption that
len == sizeof(sockaddr) otherwise you access beyond the end of *sa
or you only copy a part of the underlying structure.

I did not want to add more complexity here, like using dynamic allocations,
because with AF_INET/AF_INET6/AF_PACKET I can make sure that all socket
structures are exactly the same size.

On the other hand, on linux nobody sets "sa_len" but uses the len
parameter instead to tell bind() how long the structure is.
So I figured it would be better to use the len parameter to set
that member, because inside the network stack, there is no "len",
but always sa_len.

So, my intention was just to improve compatibilty to linux here,
and still have the right data in sa_len for the network stack.


Regards
Bernd.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

Grant Edwards <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #15 from Grant Edwards <[hidden email]> ---
First, thanks for taking the time to find/fix all these bugs.

I'd like to start working on getting these committed.  After a
preliminary look through the patches I have a few suggestions on how
to more quickly get things committed.

The patches contain hundreds diffs where nothing has changed but
whitespace (usually at the end of the line):

  diff -Nur ecos-cvs-120723/packages/io/eth/current/src/net/eth_drv.c
ecos/packages/io/eth/current/src/net/eth_drv.c
  --- ecos-cvs-120723/packages/io/eth/current/src/net/eth_drv.c    2009-01-29
18:49:45.000000000 +0100
  +++ ecos/packages/io/eth/current/src/net/eth_drv.c    2012-08-02
14:30:54.000000000 +0200
  @@ -146,8 +146,8 @@
   static int
   simulate_fail( struct eth_drv_sc *sc, int which )
   {
  -    struct simulated_failure_state *s;  
  -    
  +    struct simulated_failure_state *s;
  +
       for ( s = &simulated_failure_states[0]; s <
&simulated_failure_states[2];
             s++ ) {
           if ( 0 == s->sc ) {

Having to wade through the non-changes visually searching for the real
changes makes it that much more difficult to review.  Applying the
patches and then using a diff tool like meld that can ignore
whitespace changes is one work-around, but being able to use
Bugzilla's built-in (though somewhat less intelligent) tools makes
life easier for anybody reviewing the code.

I realize that deleting trailing whitespace is a side-effect of using
certain editors and probably wasn't done intentionally.  Cleaning up
whitespace from ends of lines is fine, but it it should probably be in
a separate patch that does nothing but whitespace cleanup.

Splitting the changes up into smaller patches would also make it much
easier.  A separate patch for the AF_PACKET support and for each of
the 10 bugs you listed would be ideal.  That way somebody reviewing
the code doesn't have to try to figure out which bug a particular
change is addressing.  It also means that the work of reviewing and
committing the changes can be divided up among multiple maintainers.

Next is the question of copyright assignment.  I'm not sure where the
threshold is for "trivial" changes above which a copyright assignment
is needed, and hopefully some other maintainers to comment on
that. There's probably a good chance that for a contribution this
large a copyright assignment will be needed.

--
Grant

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

--- Comment #16 from Bernd Edlinger <[hidden email]> ---
(In reply to comment #15)
> First, thanks for taking the time to find/fix all these bugs.
>
> I'd like to start working on getting these committed.  After a
> preliminary look through the patches I have a few suggestions on how
> to more quickly get things committed.
>
> The patches contain hundreds diffs where nothing has changed but
> whitespace (usually at the end of the line):
>

OK, separating the white-space changes is easy I will do that.

> Splitting the changes up into smaller patches would also make it much
> easier.  A separate patch for the AF_PACKET support and for each of
> the 10 bugs you listed would be ideal.  That way somebody reviewing
> the code doesn't have to try to figure out which bug a particular
> change is addressing.  It also means that the work of reviewing and
> committing the changes can be divided up among multiple maintainers.
>

Well, maybe I should start with writing a change-log.

I am not sure how to test each part of that patch separately.
But I will try to isolate obviously independent parts and
separate that where possible.

> Next is the question of copyright assignment.  I'm not sure where the
> threshold is for "trivial" changes above which a copyright assignment
> is needed, and hopefully some other maintainers to comment on
> that. There's probably a good chance that for a contribution this
> large a copyright assignment will be needed.
>

The copyright-assignment was signed by
my company (softing.com) on 2012-10-25
and counter-signed by FSF on 2012-12-17

Bernd.

> --
> Grant

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

--- Comment #17 from Bernd Edlinger <[hidden email]> ---
Hi,

Now I have separated the white-space changes from the code changes and put
it in another patch file, which is to be applied after the code changes,
otherwise the context won't match.

And I wrote a change log for all the code changes.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

--- Comment #18 from Bernd Edlinger <[hidden email]> ---
Created attachment 2414
  --> http://bugs.ecos.sourceware.org/attachment.cgi?id=2414&action=edit
Change-Log entry for all the bsd stack changes

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

bugzilla-daemon
In reply to this post by bugzilla-daemon
Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

Bernd Edlinger <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2337|0                           |1
        is obsolete|                            |

--- Comment #19 from Bernd Edlinger <[hidden email]> ---
Created attachment 2415
  --> http://bugs.ecos.sourceware.org/attachment.cgi?id=2415&action=edit
patch to implement packet sockets and various bug-fixes

--
You are receiving this mail because:
You are on the CC list for the bug.
12