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 <jan...@si...> 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
>
|