Interested in contributing towards the items in Wiki
Hi Everybody I am interested in contributing towards some of the items mentioned in the wiki link. Though I am quite new to FreeBSD and have been reading the kernel networking part and in fact working on back porting ECMP fixes to 7.2 from 8.0. I have sent a patch to the Mailing List too and also working on completing it . I am interested in contributing towards the following items 1. Low hanging Kernel fruit 2. IPv6 Cleanup Tasks mentioned in the wiki link http://wiki.freebsd.org/Networking It would be really great if people could reply to this mail and let me know how i could start and probably mentor me to being part of the team. Thanks for your time. Cheers, - Balaji ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
struct sockaddr * and alignment
Greetings,
I've found a few inconsistencies in the sockaddr stuff in the tree.
I'm not sure where to go to get a definitive answer, so I thought I'd
start here.
I got here looking at the recent wake breakage on mips. It turns out
that the warning was:
src/usr.sbin/wake/wake.c: In function 'find_ether':
src/usr.sbin/wake/wake.c:123: warning: cast increases required alignment of
target type
which comes from
sdl = (struct sockaddr_dl *)ifa->ifa_addr;
The problem is that on MIPS struct sockaddr * is byte aligned and
sockaddr_dl * is word aligned, so the compiler is rightly telling us
that there might be a problem here.
However, further digging shows that there will never be a problem
here with alignment. struct sockaddr_storage has a int64 in it to
force it to be __aligned(8). So I thought to myself "why don't I just
add __aligned(8) to the struct sockaddr definition?" After all, the
kernel goes to great lengths to return data so aligned, and user code
also keeps things aligned.
Sure enough, that fixes this warning. Yea. But, sadly, it causes
other problems. If you look at sbin/atm/atmconfig/natm.c you'll see
code like:
static void
store_route(struct rt_msghdr *rtm)
{
...
char *cp
struct sockaddr *sa;
...
cp = (char *)(rtm + 1);
...
sa = (struct sockaddr *)cp;
cp += roundup(sa->sa_len, sizeof(long));
...
which breaks because we're now casting from an __aligned(1) char * to
an __aligned(8) sockaddr *.
And it is only rounding the size of the structure to long, rather than
int64 like sockaddr_storage suggests is the proper alignment. But I
haven't looked in the kernel to see if there's an issue there with
routing sockets or not.
The other extreme is to put __aligned(1) on all the sockaddr_foo
structures. This would solve the compiler warning, but would have a
negative effect on performance in accessing these elements (because
the compiler would have to generate calls to bcopy or equivalent to
access the scalar members that are larger than a byte). This cure
would be worse than the disease.
So the question here is "What is the right solution here?" It has me
stumped. So I dropped WARNS level down from 6 to 3 for wake.c.
Warner
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
RE: struct sockaddr * and alignment
>
> Sure enough, that fixes this warning. Yea. But, sadly, it causes
> other problems. If you look at sbin/atm/atmconfig/natm.c you'll see
> code like:
>
> static void
> store_route(struct rt_msghdr *rtm)
> {
> ...
> char *cp
> struct sockaddr *sa;
> ...
>
> cp = (char *)(rtm + 1);
> ...
> sa = (struct sockaddr *)cp;
> cp += roundup(sa->sa_len, sizeof(long));
> ...
>
> which breaks because we're now casting from an __aligned(1) char * to
> an __aligned(8) sockaddr *.
> And it is only rounding the size of the structure to long, rather than
> int64 like sockaddr_storage suggests is the proper alignment. But I
> haven't looked in the kernel to see if there's an issue there with
> routing sockets or not.
>
In the kernel route.h, the macro SA_SIZE is used by the routing socket
code and may the root of the problem.
-- Qing
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: struct sockaddr * and alignment
On Tue, Feb 09, 2010 at 10:34:11AM -0700, M. Warner Losh wrote:
> Greetings,
>
> I've found a few inconsistencies in the sockaddr stuff in the tree.
> I'm not sure where to go to get a definitive answer, so I thought I'd
> start here.
>
> I got here looking at the recent wake breakage on mips. It turns out
> that the warning was:
>
> src/usr.sbin/wake/wake.c: In function 'find_ether':
> src/usr.sbin/wake/wake.c:123: warning: cast increases required alignment of
> target type
>
> which comes from
> sdl = (struct sockaddr_dl *)ifa->ifa_addr;
>
> The problem is that on MIPS struct sockaddr * is byte aligned and
> sockaddr_dl * is word aligned, so the compiler is rightly telling us
> that there might be a problem here.
>
> However, further digging shows that there will never be a problem
> here with alignment. struct sockaddr_storage has a int64 in it to
> force it to be __aligned(8). So I thought to myself "why don't I just
> add __aligned(8) to the struct sockaddr definition?" After all, the
> kernel goes to great lengths to return data so aligned, and user code
> also keeps things aligned.
>
> Sure enough, that fixes this warning. Yea. But, sadly, it causes
> other problems. If you look at sbin/atm/atmconfig/natm.c you'll see
> code like:
>
> static void
> store_route(struct rt_msghdr *rtm)
> {
> ...
> char *cp
> struct sockaddr *sa;
> ...
>
> cp = (char *)(rtm + 1);
> ...
> sa = (struct sockaddr *)cp;
> cp += roundup(sa->sa_len, sizeof(long));
> ...
>
> which breaks because we're now casting from an __aligned(1) char * to
> an __aligned(8) sockaddr *.
>
> And it is only rounding the size of the structure to long, rather than
> int64 like sockaddr_storage suggests is the proper alignment. But I
> haven't looked in the kernel to see if there's an issue there with
> routing sockets or not.
>
> The other extreme is to put __aligned(1) on all the sockaddr_foo
> structures. This would solve the compiler warning, but would have a
> negative effect on performance in accessing these elements (because
> the compiler would have to generate calls to bcopy or equivalent to
> access the scalar members that are larger than a byte). This cure
> would be worse than the disease.
>
> So the question here is "What is the right solution here?" It has me
> stumped. So I dropped WARNS level down from 6 to 3 for wake.c.
There does not seem to be a good fix for this type of problems
(copying structures with >1 alignment requirements from/to
linearized messages) that does not involve a bcopy.
I think struct sockaddr (the generic versions) are normally not used
in speed-critical paths, so having a bcopy inserted by the compiler
(is this what really happens ?) should not harm too much.
On the other hand, maybe the places where the misalignment occurs
are not too many (and typically occur across ioctl/sockopt or similar
system calls ?) so it might be preferable to expose the alignment
requirements and manually fix the offending code. Many of those
places might be already correct.
cheers
luigi
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: [PATCH] net:: ECMP Phase 1 Fixes for FreeBSD 7.2
On Mon, 8 Feb 2010 10:39, balajig81@ wrote: Hi Everybody Please find attached the patch files which contains the backported code for ECMP for FreeBSD 7.2. I have back ported the code from 8.0. I have done some basic testing with these patches rolled in. These are the phase 1 fixes and i would continue to work on this and back port few other stuff too. It would be great if someone could roll in this to 7.2, test it and give me their valuable feedback. This is my first Patch for FreeBSD community and i am really excited about this and look forward to it. Thanks for your time. Cheers, - Balaji In ecmp_patch1 you have, @@ -320,8 +320,9 @@ #endif extern int in6_inithead(void **head, int off); -extern int in_inithead(void **head, int off); +extern int in_inthead(void **head, int off); ?? Delete keys get in the way some times ;) -- jhell ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: [PATCH] net:: ECMP Phase 1 Fixes for FreeBSD 7.2
Hi Jhell > Delete keys get in the way some times ;) I am sorry i didn't understand your comment. Is it wrong or should i change something, please let me know so that i could create an other patch . PS: I had also sent an email to the list to take up some items mentioned in the Wiki Page under the networking section. It would be nice if by any chance you know who's the contact person :). for those activities. Thanks for your feedback and time :) Thanks, Cheers, - Balaji On Wed, Feb 10, 2010 at 12:11 PM, jhell wrote: > > On Mon, 8 Feb 2010 10:39, balajig81@ wrote: > >> Hi Everybody >> >> Please find attached the patch files which contains the backported code >> for >> ECMP for FreeBSD 7.2. I have back ported the code from 8.0. I have done >> some >> basic testing with these patches rolled in. These are the phase 1 fixes >> and >> i would continue to work on this and back port few other stuff too. It >> would >> be great if someone could roll in this to 7.2, test it and give me their >> valuable feedback. >> >> This is my first Patch for FreeBSD community and i am really excited about >> this and look forward to it. >> >> Thanks for your time. >> >> Cheers, >> - Balaji >> >> > In ecmp_patch1 you have, > > @@ -320,8 +320,9 @@ > #endif > > extern int in6_inithead(void **head, int off); > -extern int in_inithead(void **head, int off); > +extern int in_inthead(void **head, int off); > > ?? > > Delete keys get in the way some times ;) > > -- > > jhell > > ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: Interested in contributing towards the items in Wiki
Balaji , you can take topic of your interest & start working on it. for that first you need to have a test setup for testing your code. Regards, Onkar On Tue, Feb 9, 2010 at 9:40 PM, Balaji G wrote: > Hi Everybody > > I am interested in contributing towards some of the items mentioned in the > wiki link. Though I am quite new to FreeBSD and have been reading the > kernel > networking part and in fact working on back porting ECMP fixes to 7.2 from > 8.0. I have sent a patch to the Mailing List too and also working on > completing it . I am interested in contributing towards the following items > > 1. Low hanging Kernel fruit > 2. IPv6 Cleanup Tasks > > mentioned in the wiki link http://wiki.freebsd.org/Networking > > It would be really great if people could reply to this mail and let me know > how i could start and probably mentor me to being part of the team. > > Thanks for your time. > > Cheers, > - Balaji > ___ > freebsd-net@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-net > To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org" > ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
