netdev - Re: [Intel-wired-lan] [PATCH v1] netdevice: use ifmap isteand of plain fields

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 03 Mar 2023 19:01:48 +0100
From:   "Vincenzo Palazzo" <vincenzopalazzodev@...il.com>
To:     "Paul Menzel" <pmenzel@...gen.mpg.de>
Cc:     "Linus Torvalds" <torvalds@...ux-foundation.org>,
        <netdev@...r.kernel.org>, <intel-wired-lan@...ts.osuosl.org>,
        <davem@...emloft.net>, <jesse.brandeburg@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH v1] netdevice: use ifmap isteand of
 plain fields

On Fri Mar 3, 2023 at 5:48 PM CET, Paul Menzel wrote:
> Dear Vincenzo,
>
>
> Thank you for your patch. There is a small typo in the commit message 
> summary in *instead*.
>
Ah, I also tried to pay attention to avoid typo! Sorry, I will made a v2.

> Am 03.03.23 um 16:08 schrieb Vincenzo Palazzo:
> > clean the code by using the ifmap instead of plain fields,
> > and avoid code duplication.
> > 
> > P.S: I'm giving credit to the author of the FIXME commit.
>
> No idea, what you mean exactly, but you can do that by adding From: in 
> the first line of the commit message body.
>

I ran git blame to find the comment author because the 
refactoring was suggested by the FIXME, but it turn out that was a very
historical one (before git) as Linus point out git blame is wrong in
this case. I will also fix this in the v2

Cheers!

Vincent.


>
> > Suggested-by: Linus Torvalds <torvalds@...ux-foundation.org>
> > Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@...il.com>
> > ---
> >   drivers/net/ethernet/intel/e1000e/netdev.c |  4 ++--
> >   include/linux/netdevice.h                  |  8 +-------
> >   net/core/dev_ioctl.c                       | 12 ++++++------
> >   net/core/rtnetlink.c                       |  6 +++---
> >   4 files changed, 12 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> > index e1eb1de88bf9..059ff8bcdbbc 100644
> > --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> > @@ -7476,8 +7476,8 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >   	netif_napi_add(netdev, &adapter->napi, e1000e_poll);
> >   	strscpy(netdev->name, pci_name(pdev), sizeof(netdev->name));
> >   
> > -	netdev->mem_start = mmio_start;
> > -	netdev->mem_end = mmio_start + mmio_len;
> > +	netdev->dev_mapping.mem_start = mmio_start;
> > +	netdev->dev_mapping.mem_end = mmio_start + mmio_len;
> >   
> >   	adapter->bd_number = cards_found++;
> >   
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 6a14b7b11766..c5987e90a078 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -2031,13 +2031,7 @@ struct net_device {
> >   	char			name[IFNAMSIZ];
> >   	struct netdev_name_node	*name_node;
> >   	struct dev_ifalias	__rcu *ifalias;
> > -	/*
> > -	 *	I/O specific fields
> > -	 *	FIXME: Merge these and struct ifmap into one
> > -	 */
> > -	unsigned long		mem_end;
> > -	unsigned long		mem_start;
> > -	unsigned long		base_addr;
> > +	struct ifmap dev_mapping;
> >   
> >   	/*
> >   	 *	Some hardware also needs these fields (state,dev_list,
> > diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
> > index 5cdbfbf9a7dc..89469cb97e35 100644
> > --- a/net/core/dev_ioctl.c
> > +++ b/net/core/dev_ioctl.c
> > @@ -88,9 +88,9 @@ static int dev_getifmap(struct net_device *dev, struct ifreq *ifr)
> >   	if (in_compat_syscall()) {
> >   		struct compat_ifmap *cifmap = (struct compat_ifmap *)ifmap;
> >   
> > -		cifmap->mem_start = dev->mem_start;
> > -		cifmap->mem_end   = dev->mem_end;
> > -		cifmap->base_addr = dev->base_addr;
> > +		cifmap->mem_start = dev->dev_mapping.mem_start;
> > +		cifmap->mem_end   = dev->dev_mapping.mem_end;
> > +		cifmap->base_addr = dev->dev_mapping.base_addr;
> >   		cifmap->irq       = dev->irq;
> >   		cifmap->dma       = dev->dma;
> >   		cifmap->port      = dev->if_port;
> > @@ -98,9 +98,9 @@ static int dev_getifmap(struct net_device *dev, struct ifreq *ifr)
> >   		return 0;
> >   	}
> >   
> > -	ifmap->mem_start  = dev->mem_start;
> > -	ifmap->mem_end    = dev->mem_end;
> > -	ifmap->base_addr  = dev->base_addr;
> > +	ifmap->mem_start  = dev->dev_mapping.mem_start;
> > +	ifmap->mem_end    = dev->dev_mapping.mem_end;
> > +	ifmap->base_addr  = dev->dev_mapping.base_addr;
> >   	ifmap->irq        = dev->irq;
> >   	ifmap->dma        = dev->dma;
> >   	ifmap->port       = dev->if_port;
> > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > index 5d8eb57867a9..ff8fc1bbda31 100644
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> > @@ -1445,9 +1445,9 @@ static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
> >   	struct rtnl_link_ifmap map;
> >   
> >   	memset(&map, 0, sizeof(map));
> > -	map.mem_start   = dev->mem_start;
> > -	map.mem_end     = dev->mem_end;
> > -	map.base_addr   = dev->base_addr;
> > +	map.mem_start   = dev->dev_mapping.mem_start;
> > +	map.mem_end     = dev->dev_mapping.mem_end;
> > +	map.base_addr   = dev->dev_mapping.base_addr;
> >   	map.irq         = dev->irq;
> >   	map.dma         = dev->dma;
> >   	map.port        = dev->if_port;