> >> And here, you copy the broadcast into it, and then copy that into the > >> s_rule_hdr_data... > >> > >> But why not just copy directly into the s_rule->hdr_data instead of > >> copying twice? Literally nothing else interacts with mac_train_pkt > >> introduced in this patch, so we needlessly copy, and result in using a > >> value that could be modified by another thread possibly even on another > >> PF since its a global variable... > >> > >> Please fix this. > > > > > > Yeah - I think you are right. I originally wrote this following the pattern > already > > Present in the code, but with this packet starting with all zeros there is > > no > need for > > It to be a const. > > > > Will rewrite this using a local variable. > > > > DaveE > > I still don't understand why you can't just do this: > > ether_addr_copy(s_rule->hdr_data, broadcast) > > and > > ether_addr_copy(s_rule->hdr_data, lag->upper_netdev->dev_addr) > > without the extra temporary buffer? > > The memcpy is using ICE_TRAIN_PKT_LEN.. is that not the same size as the > dev_addr? Is it because ICE_TRAIN_PKT_LEN is 16 bytes, and you're > copying a bunch of zeros over as well? > > But why not just memset and then copy the address in? > > memset(s_rule->hdr_data, 0, sizeof(s_rule->hdr_data)); > ether_addr_copy(s_rule->hdr_data, lag->upper_netdev->dev_addr); > > Thanks, > Jake
memset and ether_addr_copy (or eth_broadcast_addr) is actually what I ended up doing 😊 DaveE
