The patches are now broken into different parts and attached as files to avoid the line breaking. While I was at it I added a patch that allows one to specify promiscuous mode from the command line while running as master or slave (this was quite valuable for me).
Thanks again, - James On Fri, Dec 11, 2009 at 10:59 AM, Jan Kiszka <[email protected]> wrote: > James Kilts wrote: > > Here are a few patches that correct the memory alignment issues. The > first > > file also contains a change from "break" to "return" because the variable > > "stage_1_cfg" is used after the loop and is really just "rtskb->data" > which > > is freed using kfree_rtskb() before the break. The change to > rtmac_proto.c > > was useful to me while tracking down an alignment issue, but if this last > > change was "accidentally" lost it wouldn't matter. :-) > > > > Syntactically I prefer dereferencing since it's easier to see what the > code > > does at a glance (perhaps someone has a preference toward get_unaligned() > > and put_unaligned() instead). But consistency with the rest of the code > > seems to demand memcpy(). > > All good catches, thanks! I just have a few style remarks below, and I > would like to ask you to properly slit up the patches: > - unaligned access fixes > - break->return fix > - instrumentation enhancement > > Moreover, when reposting, make sure that those patches are not > line-wrapped like this one. > > > > > - James > > > > > > > > _____________________________ > > > > > > > > > > Signed-off-by: James Kilts <jameskilts <at> google's mail service> > > Your mail shows up in clear text when you go to public mailing lists, no > need to obfuscate, and I also prefer readable addresses in the git log. > > > --- > > > > > > diff -ru rtnet-0.9.11_original/stack/rtcfg/rtcfg_client_event.c > > rtnet-0.9.11/stack/rtcfg/rtcfg_client_event.c > > --- rtnet-0.9.11_original/stack/rtcfg/rtcfg_client_event.c 2009-11-12 > > 11:36:56.000000000 +0100 > > +++ rtnet-0.9.11/stack/rtcfg/rtcfg_client_event.c 2009-12-10 > > 16:07:16.000000000 +0100 > > @@ -598,24 +622,24 @@ > > if (rtskb->len < sizeof(struct rtcfg_frm_stage_1_cfg) + > > 2*RTCFG_ADDRSIZE_IP) { > > rtdm_mutex_unlock(&rtcfg_dev->dev_mutex); > > RTCFG_DEBUG(1, "RTcfg: received invalid stage_1_cfg " > > "frame\n"); > > kfree_rtskb(rtskb); > > - break; > > + return; > > } > > > > rtdev = rtskb->rtdev; > > > > - daddr = *(u32*)stage_1_cfg->client_addr; > > + memcpy(&daddr, stage_1_cfg->client_addr, 4); > > stage_1_cfg = (struct rtcfg_frm_stage_1_cfg *) > > (((u8 *)stage_1_cfg) + RTCFG_ADDRSIZE_IP); > > > > - saddr = *(u32*)stage_1_cfg->server_addr; > > + memcpy(&saddr, stage_1_cfg->server_addr, 4); > > stage_1_cfg = (struct rtcfg_frm_stage_1_cfg *) > > (((u8 *)stage_1_cfg) + RTCFG_ADDRSIZE_IP); > > > > __rtskb_pull(rtskb, 2*RTCFG_ADDRSIZE_IP); > > > > /* Broadcast: IP is used to address client */ > > if (rtskb->pkt_type == PACKET_BROADCAST) { > > /* directed to us? */ > > @@ -748,6 +773,7 @@ > > struct rtcfg_frm_announce *announce_frm; > > struct rtcfg_device *rtcfg_dev = &device[ifindex]; > > u32 i; > > + u32 announce_from_addr; > > int result; > > > > > > @@ -771,8 +797,10 @@ > > return -EINVAL; > > } > > > > + memcpy(&announce_from_addr, announce_frm->addr, 4); > > + > > /* update routing table */ > > - rt_ip_route_add_host(*(u32 *)announce_frm->addr, > > + rt_ip_route_add_host(announce_from_addr, > > rtskb->mac.ethernet->h_source, > > rtskb->rtdev); > > > > announce_frm = (struct rtcfg_frm_announce *) > > @@ -1045,7 +1073,7 @@ > > return; > > } > > > > - ip = *(u32 *)dead_station_frm->logical_addr; > > + memcpy(&ip, dead_station_frm->logical_addr, 4); > > > > /* only delete remote IPs from routing table */ > > if (rtskb->rtdev->local_ip != ip) > > @@ -1132,11 +1160,11 @@ > > > > rtdev = rtskb->rtdev; > > > > - daddr = *(u32*)stage_1_cfg->client_addr; > > + memcpy(&daddr, stage_1_cfg->client_addr, 4); > > stage_1_cfg = (struct rtcfg_frm_stage_1_cfg *) > > (((u8 *)stage_1_cfg) + RTCFG_ADDRSIZE_IP); > > > > - saddr = *(u32*)stage_1_cfg->server_addr; > > + memcpy(&saddr, stage_1_cfg->server_addr, 4); > > stage_1_cfg = (struct rtcfg_frm_stage_1_cfg *) > > (((u8 *)stage_1_cfg) + RTCFG_ADDRSIZE_IP); > > > > diff -ru rtnet-0.9.11_original/stack/rtcfg/rtcfg_event.c > > rtnet-0.9.11/stack/rtcfg/rtcfg_event.c > > --- rtnet-0.9.11_original/stack/rtcfg/rtcfg_event.c 2009-11-12 > > 11:36:56.000000000 +0100 > > +++ rtnet-0.9.11/stack/rtcfg/rtcfg_event.c 2009-12-10 > 16:22:28.000000000 > > +0100 > > @@ -523,7 +532,7 @@ > > #ifdef CONFIG_RTNET_RTIPV4 > > case RTCFG_ADDR_IP: > > if (((conn->addr_type & RTCFG_ADDR_MASK) == > RTCFG_ADDR_IP) > > && > > - (*(u32 *)announce->addr == conn->addr.ip_addr)) { > > + (memcmp(announce->addr, &(conn->addr.ip_addr), 4) == > > 0)) { > > Better go via a local variable here that should carry announce->addr for > the comparison. > > > /* save MAC address - Ethernet-specific! */ > > memcpy(conn->mac_addr, > rtskb->mac.ethernet->h_source, > > ETH_ALEN); > > > > diff -ru rtnet-0.9.11_original/stack/rtcfg/rtcfg_frame.c > > rtnet-0.9.11/stack/rtcfg/rtcfg_frame.c > > --- rtnet-0.9.11_original/stack/rtcfg/rtcfg_frame.c 2009-11-12 > > 11:36:56.000000000 +0100 > > +++ rtnet-0.9.11/stack/rtcfg/rtcfg_frame.c 2009-12-10 > 16:13:04.000000000 > > +0100 > > @@ -170,12 +170,12 @@ > > if (stage_1_frm->addr_type == RTCFG_ADDR_IP) { > > rtskb_put(rtskb, 2*RTCFG_ADDRSIZE_IP); > > > > - *(u32*)stage_1_frm->client_addr = conn->addr.ip_addr; > > + memcpy(stage_1_frm->client_addr, &(conn->addr.ip_addr), 4); > > > > stage_1_frm = (struct rtcfg_frm_stage_1_cfg *) > > (((u8 *)stage_1_frm) + RTCFG_ADDRSIZE_IP); > > > > - *(u32*)stage_1_frm->server_addr = rtdev->local_ip; > > + memcpy(stage_1_frm->server_addr, &(rtdev->local_ip), 4); > > > > stage_1_frm = (struct rtcfg_frm_stage_1_cfg *) > > (((u8 *)stage_1_frm) + RTCFG_ADDRSIZE_IP); > > @@ -332,7 +332,7 @@ > > if (announce_new->addr_type == RTCFG_ADDR_IP) { > > rtskb_put(rtskb, RTCFG_ADDRSIZE_IP); > > > > - *(u32*)announce_new->addr = rtdev->local_ip; > > + memcpy(announce_new->addr, &(rtdev->local_ip), 4); > > > > announce_new = (struct rtcfg_frm_announce *) > > (((u8 *)announce_new) + RTCFG_ADDRSIZE_IP); > > @@ -388,7 +388,7 @@ > > if (announce_rpl->addr_type == RTCFG_ADDR_IP) { > > rtskb_put(rtskb, RTCFG_ADDRSIZE_IP); > > > > - *(u32*)announce_rpl->addr = rtdev->local_ip; > > + memcpy(announce_rpl->addr, &(rtdev->local_ip), 4); > > > > announce_rpl = (struct rtcfg_frm_announce *) > > (((u8 *)announce_rpl) + RTCFG_ADDRSIZE_IP); > > @@ -512,7 +512,7 @@ > > if (dead_station_frm->addr_type == RTCFG_ADDR_IP) { > > rtskb_put(rtskb, RTCFG_ADDRSIZE_IP); > > > > - *(u32*)dead_station_frm->logical_addr = conn->addr.ip_addr; > > + memcpy(dead_station_frm->logical_addr, &(conn->addr.ip_addr), > 4); > > > > dead_station_frm = (struct rtcfg_frm_dead_station *) > > (((u8 *)dead_station_frm) + RTCFG_ADDRSIZE_IP); > > > > diff -ru rtnet-0.9.11_original/stack/rtmac/rtmac_proto.c > > rtnet-0.9.11/stack/rtmac/rtmac_proto.c > > --- rtnet-0.9.11_original/stack/rtmac/rtmac_proto.c 2009-11-12 > > 11:36:56.000000000 +0100 > > +++ rtnet-0.9.11/stack/rtmac/rtmac_proto.c 2009-12-10 > 12:31:44.000000000 > > +0100 > > @@ -50,7 +50,8 @@ > > > > if (hdr->ver != RTMAC_VERSION) { > > rtdm_printk("RTmac: received unsupported RTmac protocol version > on > > " > > - "device %s\n", skb->rtdev->name); > > + "device %s. Got 0x%02x but expected 0x%02x.\n", > > + skb->rtdev->name, hdr->ver, RTMAC_VERSION); > > goto error; > > } > > > > > > > > Thanks again, > Jan > > -- > Siemens AG, Corporate Technology, CT T DE IT 1 > Corporate Competence Center Embedded Linux >
Signed-off-by: James Kilts <[email protected]> --- diff -U 3 -HbdprN -- rtnet-0.9.11_original/stack/rtcfg/rtcfg_client_event.c rtnet-0.9.11/stack/rtcfg/rtcfg_client_event.c --- rtnet-0.9.11_fresh_backup/stack/rtcfg/rtcfg_client_event.c 2009-11-12 11:36:56.000000000 +0100 +++ rtnet-0.9.11/stack/rtcfg/rtcfg_client_event.c 2009-12-11 15:39:20.000000000 +0100 @@ -598,24 +601,24 @@ static void rtcfg_client_recv_stage_1(in RTCFG_DEBUG(1, "RTcfg: received invalid stage_1_cfg " "frame\n"); kfree_rtskb(rtskb); - break; + return; } rtdev = rtskb->rtdev;
Signed-off-by: James Kilts <[email protected]> --- diff -U 3 -HbdprN -- rtnet-0.9.11_original/stack/rtmac/rtmac_proto.c rtnet-0.9.11/stack/rtmac/rtmac_proto.c --- rtnet-0.9.11_original/stack/rtmac/rtmac_proto.c 2009-11-12 11:36:56.000000000 +0100 +++ rtnet-0.9.11/stack/rtmac/rtmac_proto.c 2009-12-10 12:31:44.000000000 +0100 @@ -50,7 +50,8 @@ int rtmac_proto_rx(struct rtskb *skb, st if (hdr->ver != RTMAC_VERSION) { rtdm_printk("RTmac: received unsupported RTmac protocol version on " - "device %s\n", skb->rtdev->name); + "device %s. Got 0x%x but expected 0x%x\n", + skb->rtdev->name, hdr->ver, RTMAC_VERSION); goto error; }
Signed-off-by: James Kilts <[email protected]> --- diff -U 3 -H -b -d -p -r -N -- rtnet-0.9.11_original/tools/rtnet rtnet-0.9.11/tools/rtnet --- rtnet-0.9.11_original/tools/rtnet 2009-11-30 11:32:16.000000000 +0100 +++ rtnet-0.9.11/tools/rtnet 2009-12-11 16:09:36.000000000 +0100 @@ -15,16 +15,18 @@ debug_func() { usage() { cat << EOF Usage: - $0 [-cf <config-file>] [-v] {start|stop} + $0 [-cf <config-file>] [-v] [-p] {start|stop} Start or stop station according to configuration file - $0 [-cf <config-file>] [-v] master <slave_ip1> [<slave_ip2> ...] + $0 [-cf <config-file>] [-v] [-p] master <slave_ip1> [<slave_ip2> ...] Start station as master for given list of slaves $0 [-cf <config-file>] [-v] capture Start only passive realtime capturing The additional switch -v enables verbose output. + The additional switch -p enables promiscuous mode to allow use of a network + analyzer such as Wireshark (if rtnet was built with --enable-rtcap). EOF } @@ -254,6 +256,11 @@ if [ "$1" = "-v" ]; then shift 1 fi +if [ "$1" = "-p" ]; then + RTCAP="yes" + shift 1 +fi + NETMASK_OPT= if [ ! "$NETMASK" = "" ]; then NETMASK_OPT="netmask $NETMASK"
Signed-off-by: James Kilts <[email protected]> --- diff -U 3 -HbdprN -- rtnet-0.9.11_original/stack/rtcfg/rtcfg_client_event.c rtnet-0.9.11/stack/rtcfg/rtcfg_client_event.c --- rtnet-0.9.11_original/stack/rtcfg/rtcfg_client_event.c 2009-11-12 11:36:56.000000000 +0100 +++ rtnet-0.9.11/stack/rtcfg/rtcfg_client_event.c 2009-12-11 15:39:20.000000000 +0100 @@ -602,14 +605,14 @@ static void rtcfg_client_recv_stage_1(in } rtdev = rtskb->rtdev; - daddr = *(u32*)stage_1_cfg->client_addr; + memcpy(&daddr, stage_1_cfg->client_addr, 4); stage_1_cfg = (struct rtcfg_frm_stage_1_cfg *) (((u8 *)stage_1_cfg) + RTCFG_ADDRSIZE_IP); - saddr = *(u32*)stage_1_cfg->server_addr; + memcpy(&saddr, stage_1_cfg->server_addr, 4); stage_1_cfg = (struct rtcfg_frm_stage_1_cfg *) (((u8 *)stage_1_cfg) + RTCFG_ADDRSIZE_IP); __rtskb_pull(rtskb, 2*RTCFG_ADDRSIZE_IP); @@ -748,6 +752,7 @@ static int rtcfg_client_recv_announce(in struct rtcfg_frm_announce *announce_frm; struct rtcfg_device *rtcfg_dev = &device[ifindex]; u32 i; + u32 announce_frm_addr; int result; @@ -771,8 +776,10 @@ static int rtcfg_client_recv_announce(in return -EINVAL; } + memcpy(&announce_frm_addr, announce_frm->addr, 4); + /* update routing table */ - rt_ip_route_add_host(*(u32 *)announce_frm->addr, + rt_ip_route_add_host(announce_frm_addr, rtskb->mac.ethernet->h_source, rtskb->rtdev); announce_frm = (struct rtcfg_frm_announce *) @@ -1045,7 +1052,7 @@ static void rtcfg_client_recv_dead_stati return; } - ip = *(u32 *)dead_station_frm->logical_addr; + memcpy(&ip, dead_station_frm->logical_addr, 4); /* only delete remote IPs from routing table */ if (rtskb->rtdev->local_ip != ip) @@ -1132,11 +1139,11 @@ static void rtcfg_client_update_server(i rtdev = rtskb->rtdev; - daddr = *(u32*)stage_1_cfg->client_addr; + memcpy(&daddr, stage_1_cfg->client_addr, 4); stage_1_cfg = (struct rtcfg_frm_stage_1_cfg *) (((u8 *)stage_1_cfg) + RTCFG_ADDRSIZE_IP); - saddr = *(u32*)stage_1_cfg->server_addr; + memcpy(&saddr, stage_1_cfg->server_addr, 4); stage_1_cfg = (struct rtcfg_frm_stage_1_cfg *) (((u8 *)stage_1_cfg) + RTCFG_ADDRSIZE_IP); diff -U 3 -HbdprN -- rtnet-0.9.11_original/stack/rtcfg/rtcfg_event.c rtnet-0.9.11/stack/rtcfg/rtcfg_event.c --- rtnet-0.9.11_original/stack/rtcfg/rtcfg_event.c 2009-11-12 11:36:56.000000000 +0100 +++ rtnet-0.9.11/stack/rtcfg/rtcfg_event.c 2009-12-11 16:45:07.000000000 +0100 @@ -506,6 +507,7 @@ static int rtcfg_server_recv_announce(in struct list_head *entry; struct rtcfg_frm_announce *announce; struct rtcfg_connection *conn; + u32 announce_addr; if (rtskb->len < sizeof(struct rtcfg_frm_announce)) { @@ -516,6 +518,8 @@ static int rtcfg_server_recv_announce(in announce = (struct rtcfg_frm_announce *)rtskb->data; + memcpy(&announce_addr, announce->addr, 4); + list_for_each(entry, &rtcfg_dev->spec.srv.conn_list) { conn = list_entry(entry, struct rtcfg_connection, entry); @@ -523,7 +527,7 @@ static int rtcfg_server_recv_announce(in #ifdef CONFIG_RTNET_RTIPV4 case RTCFG_ADDR_IP: if (((conn->addr_type & RTCFG_ADDR_MASK) == RTCFG_ADDR_IP) && - (*(u32 *)announce->addr == conn->addr.ip_addr)) { + (announce_addr == conn->addr.ip_addr)) { /* save MAC address - Ethernet-specific! */ memcpy(conn->mac_addr, rtskb->mac.ethernet->h_source, ETH_ALEN); diff -U 3 -HbdprN -- rtnet-0.9.11_original/stack/rtcfg/rtcfg_frame.c rtnet-0.9.11/stack/rtcfg/rtcfg_frame.c --- rtnet-0.9.11_original/stack/rtcfg/rtcfg_frame.c 2009-11-12 11:36:56.000000000 +0100 +++ rtnet-0.9.11/stack/rtcfg/rtcfg_frame.c 2009-12-10 16:13:04.000000000 +0100 @@ -170,12 +170,12 @@ int rtcfg_send_stage_1(struct rtcfg_conn if (stage_1_frm->addr_type == RTCFG_ADDR_IP) { rtskb_put(rtskb, 2*RTCFG_ADDRSIZE_IP); - *(u32*)stage_1_frm->client_addr = conn->addr.ip_addr; + memcpy(stage_1_frm->client_addr, &(conn->addr.ip_addr), 4); stage_1_frm = (struct rtcfg_frm_stage_1_cfg *) (((u8 *)stage_1_frm) + RTCFG_ADDRSIZE_IP); - *(u32*)stage_1_frm->server_addr = rtdev->local_ip; + memcpy(stage_1_frm->server_addr, &(rtdev->local_ip), 4); stage_1_frm = (struct rtcfg_frm_stage_1_cfg *) (((u8 *)stage_1_frm) + RTCFG_ADDRSIZE_IP); @@ -332,7 +332,7 @@ int rtcfg_send_announce_new(int ifindex) if (announce_new->addr_type == RTCFG_ADDR_IP) { rtskb_put(rtskb, RTCFG_ADDRSIZE_IP); - *(u32*)announce_new->addr = rtdev->local_ip; + memcpy(announce_new->addr, &(rtdev->local_ip), 4); announce_new = (struct rtcfg_frm_announce *) (((u8 *)announce_new) + RTCFG_ADDRSIZE_IP); @@ -388,7 +388,7 @@ int rtcfg_send_announce_reply(int ifinde if (announce_rpl->addr_type == RTCFG_ADDR_IP) { rtskb_put(rtskb, RTCFG_ADDRSIZE_IP); - *(u32*)announce_rpl->addr = rtdev->local_ip; + memcpy(announce_rpl->addr, &(rtdev->local_ip), 4); announce_rpl = (struct rtcfg_frm_announce *) (((u8 *)announce_rpl) + RTCFG_ADDRSIZE_IP); @@ -512,7 +512,7 @@ int rtcfg_send_dead_station(struct rtcfg if (dead_station_frm->addr_type == RTCFG_ADDR_IP) { rtskb_put(rtskb, RTCFG_ADDRSIZE_IP); - *(u32*)dead_station_frm->logical_addr = conn->addr.ip_addr; + memcpy(dead_station_frm->logical_addr, &(conn->addr.ip_addr), 4); dead_station_frm = (struct rtcfg_frm_dead_station *) (((u8 *)dead_station_frm) + RTCFG_ADDRSIZE_IP);
------------------------------------------------------------------------------ Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev
_______________________________________________ RTnet-developers mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/rtnet-developers
