| Message ID | 1472661168-22396-1-git-send-email-grose@lightfleet.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 2016-08-31 at 09:32 -0700, Greg Rose wrote: > I'm not sure why this hasn't been done before because it seems obvious, > so maybe there is some reason that memcpy is used instead of > ether_addr_copy in this code. But let's try this anyway. > > Change memcpy to ether_addr_copy. I looked at this once and gave up given the need to verify all the possible callers that require an __aligned(2) source address.
On Wed, 2016-08-31 at 09:41 -0700, Joe Perches wrote: > On Wed, 2016-08-31 at 09:32 -0700, Greg Rose wrote: > > I'm not sure why this hasn't been done before because it seems obvious, > > so maybe there is some reason that memcpy is used instead of > > ether_addr_copy in this code. But let's try this anyway. > > > > Change memcpy to ether_addr_copy. > > I looked at this once and gave up given the need to verify all > the possible callers that require an __aligned(2) source address. > Ha, I figured there must be some reason for it. OK, patch withdrawn. Thanks!
On Wed, 2016-08-31 at 09:32 -0700, Greg Rose wrote: > I'm not sure why this hasn't been done before because it seems obvious, > so maybe there is some reason that memcpy is used instead of > ether_addr_copy in this code. But let's try this anyway. > > Change memcpy to ether_addr_copy. ... > > @@ -211,7 +211,7 @@ EXPORT_SYMBOL(eth_type_trans); > int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr) > { > const struct ethhdr *eth = eth_hdr(skb); > - memcpy(haddr, eth->h_source, ETH_ALEN); > + ether_addr_copy(haddr, eth->h_source); Please carefully read ether_addr_copy() comments. Not all arches are like x86
On Wed, 2016-08-31 at 11:32 -0700, Eric Dumazet wrote: > On Wed, 2016-08-31 at 09:32 -0700, Greg Rose wrote: > > I'm not sure why this hasn't been done before because it seems obvious, > > so maybe there is some reason that memcpy is used instead of > > ether_addr_copy in this code. But let's try this anyway. > > > > Change memcpy to ether_addr_copy. > > ... > > > > > @@ -211,7 +211,7 @@ EXPORT_SYMBOL(eth_type_trans); > > int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr) > > { > > const struct ethhdr *eth = eth_hdr(skb); > > - memcpy(haddr, eth->h_source, ETH_ALEN); > > + ether_addr_copy(haddr, eth->h_source); > > > Please carefully read ether_addr_copy() comments. > > Not all arches are like x86 Thanks Eric, Joe set me straight already. - Greg > > >
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c index 66dff5e..cbcdb97 100644 --- a/net/ethernet/eth.c +++ b/net/ethernet/eth.c @@ -95,10 +95,10 @@ int eth_header(struct sk_buff *skb, struct net_device *dev, if (!saddr) saddr = dev->dev_addr; - memcpy(eth->h_source, saddr, ETH_ALEN); + ether_addr_copy(eth->h_source, saddr); if (daddr) { - memcpy(eth->h_dest, daddr, ETH_ALEN); + ether_addr_copy(eth->h_dest, daddr); return ETH_HLEN; } @@ -211,7 +211,7 @@ EXPORT_SYMBOL(eth_type_trans); int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr) { const struct ethhdr *eth = eth_hdr(skb); - memcpy(haddr, eth->h_source, ETH_ALEN); + ether_addr_copy(haddr, eth->h_source); return ETH_ALEN; } EXPORT_SYMBOL(eth_header_parse); @@ -236,8 +236,8 @@ int eth_header_cache(const struct neighbour *neigh, struct hh_cache *hh, __be16 return -1; eth->h_proto = type; - memcpy(eth->h_source, dev->dev_addr, ETH_ALEN); - memcpy(eth->h_dest, neigh->ha, ETH_ALEN); + ether_addr_copy(eth->h_source, dev->dev_addr); + ether_addr_copy(eth->h_dest, neigh->ha); hh->hh_len = ETH_HLEN; return 0; } @@ -255,8 +255,8 @@ void eth_header_cache_update(struct hh_cache *hh, const struct net_device *dev, const unsigned char *haddr) { - memcpy(((u8 *) hh->hh_data) + HH_DATA_OFF(sizeof(struct ethhdr)), - haddr, ETH_ALEN); + ether_addr_copy(((u8 *)hh->hh_data) + + HH_DATA_OFF(sizeof(struct ethhdr)), haddr); } EXPORT_SYMBOL(eth_header_cache_update); @@ -286,7 +286,7 @@ void eth_commit_mac_addr_change(struct net_device *dev, void *p) { struct sockaddr *addr = p; - memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN); + ether_addr_copy(dev->dev_addr, addr->sa_data); } EXPORT_SYMBOL(eth_commit_mac_addr_change);
I'm not sure why this hasn't been done before because it seems obvious, so maybe there is some reason that memcpy is used instead of ether_addr_copy in this code. But let's try this anyway. Change memcpy to ether_addr_copy. Signed-off-by: Greg Rose <grose@lightfleet.com> --- net/ethernet/eth.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)