Prev: [PATCH -v3 2/2] irq_work, MCE: use irq_work in MCE
Next: Update the [register,unregister]_memory routines
From: Stephen Hemminger on 13 Jul 2010 01:20
On Mon, 12 Jul 2010 20:06:28 -0700
Shreyas Bhatewara <sbhatewara(a)vmware.com> wrote:

>
> On Thu, 2010-05-06 at 13:21 -0700, Christoph Hellwig wrote:
> > On Wed, May 05, 2010 at 10:52:53AM -0700, Stephen Hemminger wrote:
> > > Let me put it bluntly. Any design that allows external code to run
> > > in the kernel is not going to be accepted. Out of tree kernel modules are enough
> > > of a pain already, why do you expect the developers to add another
> > > interface.
> >
> > Exactly. Until our friends at VMware get this basic fact it's useless
> > to continue arguing.
> >
> > Pankaj and Dmitry: you're fine to waste your time on this, but it's not
> > going to go anywhere until you address that fundamental problem. The
> > first thing you need to fix in your archicture is to integrate the VF
> > function code into the kernel tree, and we can work from there.
> >
> > Please post patches doing this if you want to resume the discussion.
> >
> > _______________________________________________
> > Pv-drivers mailing list
> > Pv-drivers(a)vmware.com
> > http://mailman2.vmware.com/mailman/listinfo/pv-drivers
>
>
> As discussed, following is the patch to give you an idea
> about implementation of NPA for vmxnet3 driver. Although the
> patch is big, I have verified it with checkpatch.pl. It gave
> 0 errors / warnings.
>
> Signed-off-by: Matthieu Bucchaineri <matthieu(a)vmware.com>
> Signed-off-by: Shreyas Bhatewara <sbhatewara(a)vmware.com>

I think the concept won't fly.

But you should really at least try running checkpatch to make sure
the style conforms.


--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Stephen Hemminger on 13 Jul 2010 20:40
On Mon, 12 Jul 2010 20:06:28 -0700
Shreyas Bhatewara <sbhatewara(a)vmware.com> wrote:

>
> On Thu, 2010-05-06 at 13:21 -0700, Christoph Hellwig wrote:
> > On Wed, May 05, 2010 at 10:52:53AM -0700, Stephen Hemminger wrote:
> > > Let me put it bluntly. Any design that allows external code to run
> > > in the kernel is not going to be accepted. Out of tree kernel modules are enough
> > > of a pain already, why do you expect the developers to add another
> > > interface.
> >
> > Exactly. Until our friends at VMware get this basic fact it's useless
> > to continue arguing.
> >
> > Pankaj and Dmitry: you're fine to waste your time on this, but it's not
> > going to go anywhere until you address that fundamental problem. The
> > first thing you need to fix in your archicture is to integrate the VF
> > function code into the kernel tree, and we can work from there.
> >
> > Please post patches doing this if you want to resume the discussion.
> >
> > _______________________________________________
> > Pv-drivers mailing list
> > Pv-drivers(a)vmware.com
> > http://mailman2.vmware.com/mailman/listinfo/pv-drivers
>
>
> As discussed, following is the patch to give you an idea
> about implementation of NPA for vmxnet3 driver. Although the
> patch is big, I have verified it with checkpatch.pl. It gave
> 0 errors / warnings.
>
> Signed-off-by: Matthieu Bucchaineri <matthieu(a)vmware.com>
> Signed-off-by: Shreyas Bhatewara <sbhatewara(a)vmware.com>
> ---

I am surprised, the code seems to use lots of mixed case in places
that don't really follow current kernel practice.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 14 Jul 2010 05:50
On Mon, Jul 12, 2010 at 08:06:28PM -0700, Shreyas Bhatewara wrote:
> drivers/net/vmxnet3/vmxnet3_drv.c | 1845
> +++++++++++++++++++--------------

Your patch is line-wrapped and can not be applied :(

Care to fix your email client?

One thing just jumped out at me when glancing at this:

> +static INLINE void
> +MoveMemory(void *dst,
> + void *src,
> + size_t length)
> +{
> + size_t i;
> + for (i = 0; i < length; ++i)
> + ((u8 *)dst)[i] = ((u8 *)src)[i];
> +}
> +
> +static INLINE void
> +ZeroMemory(void *memory,
> + size_t length)
> +{
> + size_t i;
> + for (i = 0; i < length; ++i)
> + ((u8 *)memory)[i] = 0;
> +}

Is there some reason that our in-kernel functions that do this type of
logic are not working for you to require you to reimplement this?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Pankaj Thakkar on 14 Jul 2010 13:30
The plugin is guest agnostic and hence we did not want to rely on any kernel provided functions. The plugin uses only the interface provided by the shell. The assumption is that since the plugin is really simple and straight forward (all the control/init complexity lies in the PF driver in the hypervisor) we should be able to get by for most of the things and for things like memcpy/memset the plugin can write simple functions like this.


-p


________________________________________
From: Greg KH [greg(a)kroah.com]
Sent: Wednesday, July 14, 2010 2:49 AM
To: Shreyas Bhatewara
Cc: Christoph Hellwig; Stephen Hemminger; Pankaj Thakkar; pv-drivers(a)vmware.com; netdev(a)vger.kernel.org; linux-kernel(a)vger.kernel.org; virtualization(a)lists.linux-foundation.org
Subject: Re: [Pv-drivers] RFC: Network Plugin Architecture (NPA) for vmxnet3


Is there some reason that our in-kernel functions that do this type of
logic are not working for you to require you to reimplement this?

thanks,

greg k-h--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Miller on 14 Jul 2010 14:00
From: Pankaj Thakkar <pthakkar(a)vmware.com>
Date: Wed, 14 Jul 2010 10:18:22 -0700

> The plugin is guest agnostic and hence we did not want to rely on
> any kernel provided functions.

While I disagree entirely with this kind of approach, even that
doesn't justify what you're doing here.

memcpy() and memset() are on a much more fundamental ground than
"kernel provided functions". They had better be available no matter
where you build this thing.

And doing what you're doing is foolish on so many levels. One more
duplication of code, one more place for unnecessary bugs to live, one
more place that might need optimizations and thus require duplication
of even more work people have done over the years.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 |  Next  |  Last
Pages: 1 2
Prev: [PATCH -v3 2/2] irq_work, MCE: use irq_work in MCE
Next: Update the [register,unregister]_memory routines