netdev - Re: [Xen-devel] [PATCH] grant-table, xen-netback: Introduce helper functions for grant copy operations

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] [day] [month] [year] [list]

Date:	Fri, 4 Apr 2014 15:50:59 +0100
From:	Ian Campbell <Ian.Campbell@...rix.com>
To:	David Vrabel <david.vrabel@...rix.com>
CC:	Paul Durrant <Paul.Durrant@...rix.com>,
	Zoltan Kiss <zoltan.kiss@...rix.com>,
	Wei Liu <wei.liu2@...rix.com>,
	"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
	"konrad.wilk@...cle.com" <konrad.wilk@...cle.com>,
	"boris.ostrovsky@...cle.com" <boris.ostrovsky@...cle.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Stefano Stabellini <Stefano.Stabellini@...rix.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Jonathan Davies <Jonathan.Davies@...rix.com>
Subject: Re: [Xen-devel] [PATCH] grant-table, xen-netback: Introduce helper
 functions for grant copy operations

On Thu, 2014-04-03 at 10:48 +0100, David Vrabel wrote:
> On 03/04/14 09:12, Paul Durrant wrote:
> > Zoltan Kiss wrote:
> >>
> >> Create helper functions for grant copy operations and use them in netback.
> >>
> [...]
> >> --- a/drivers/net/xen-netback/netback.c
> >> +++ b/drivers/net/xen-netback/netback.c
> >> @@ -275,23 +275,29 @@ static void xenvif_gop_frag_copy(struct xenvif *vif,
> >> struct sk_buff *skb,
> >>  			bytes = MAX_BUFFER_OFFSET - npo->copy_off;
> >>
> >>  		copy_gop = npo->copy + npo->copy_prod++;
> >> -		copy_gop->flags = GNTCOPY_dest_gref;
> >> -		copy_gop->len = bytes;
> >>
> >>  		if (foreign_vif) {
> >> -			copy_gop->source.domid = foreign_vif->domid;
> >> -			copy_gop->source.u.ref = foreign_gref;
> >> -			copy_gop->flags |= GNTCOPY_source_gref;
> >> +			gnttab_set_copy_op_ref_to_ref(copy_gop,
> >> +						      foreign_gref,
> >> +						      npo->copy_gref,
> >> +						      foreign_vif->domid,
> >> +						      offset,
> >> +						      vif->domid,
> >> +						      npo->copy_off,
> >> +						      bytes,
> >> +						      GNTCOPY_dest_gref |
> >> +						      GNTCOPY_source_gref);
> > 
> > Can't you lose the flags here (and in the other variants)? Since
> > they are implied by the variant of the function they could just be hardcoded
> > there, couldn't they?
> 
> Even with that change these are still functions with 7 or 8 parameters
> with no obvious order.  That's just awful.  The open-coded struct
> initialization is better.

Yes, I think this was a failed experiment.

The original code would likely still be improvable by using a temporary
variable to hold "vif->tx_copy_ops[*copy_ops]", basically the way the
mapping ops are via mop->.

Ian.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html