<div dir="ltr"><div>That sounds good to me. I will send it as a separate patch then. And I can <br></div><div>maintain it. No issues.<br></div><div><br></div><div>Thanks and Regards,<br></div><div>Kotresh H R<br></div><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 28, 2017 at 1:02 PM, Niels de Vos <span dir="ltr"><<a href="mailto:ndevos@redhat.com" target="_blank">ndevos@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, Jun 28, 2017 at 12:51:07PM +0530, Amar Tumballi wrote:<br>
> On Tue, Jun 27, 2017 at 8:46 PM, Niels de Vos <<a href="mailto:ndevos@redhat.com">ndevos@redhat.com</a>> wrote:<br>
><br>
> > On Tue, Jun 27, 2017 at 08:09:25AM -0400, Kaleb S. KEITHLEY wrote:<br>
> > ><br>
> > > xxhash doesn't seem to change much. Last update to the non-test code was<br>
> > six<br>
> > > months ago.<br>
> > ><br>
> > > bundling giant (for some definition of giant) packages/projects would be<br>
> > > bad. bundling two (three if you count the test) C files doesn't seem too<br>
> > bad<br>
> > > when you consider that there are already three or four packages in fedora<br>
> > > (perl, python, R-digest, ghc (gnu haskell) that have implementations of<br>
> > > xxhash or murmur but didn't bother to package a C implementation and use<br>
> > it.<br>
> ><br>
> > I prefer to have as little maintenance components in the Gluster sources<br>
> > as we can. The maintenance burdon is already very high. The number of<br>
> > changes to xxhash seem limited, but we still need someone to track and<br>
> > pay attention to them.<br>
> ><br>
><br>
> I agree that someone should maintain it, and we should add it to<br>
> MAINTAINERS file<br>
> (or some other place, where we are tracking the dependencies).<br>
><br>
> For now, Kotresh will be looking into keeping these changes up-to-date with<br>
> upstream xxhash project, along with me.<br>
<br>
</span>Kotresh as maintainer/owner, and Aravinda as peer?<br>
<span class=""><br>
> > > I'd be for packaging it in Fedora rather than bundling it in gluster. But<br>
> > > then we get to "carry" it in rhgs as we do with userspace-rcu.<br>
> ><br>
> > We should descide what the most maintainable solution is. Having package<br>
> > maintainers with the explicit task to keep xxhash updated and current is<br>
> > apealing to me. Merging (even small) projects into the Gluster codebase<br>
> > will add more maintenance need to the project members. Therefor I have a<br>
> > strong preference to use xxhash (or an other library) that is provided<br>
> > by distributions. The more common the library is, the better it will be<br>
> > maintained without our (Gluster Community's) help.<br>
> ><br>
> ><br>
> While this is desirable, we didn't see any library available for xxhash (<br>
> <a href="http://cyan4973.github.io/xxHash/" rel="noreferrer" target="_blank">http://cyan4973.github.io/<wbr>xxHash/</a>) in our distro.<br>
><br>
> I would recommend taking these patches with TODO to use library in future<br>
> when its available, and continue to have xxhash in 'contrib/'. It is not<br>
> new for us to take code from different libraries and use it for our need<br>
> and maintain only that part (eg. libfuse). Lets treat this as similar setup.<br>
<br>
</span>Yes, if there is no suitable alternative available in the majority of<br>
distributions, this is the only sensible approach. Much of the code in<br>
contrib/ is not maintained at all. We should prevent this from happening<br>
with new code and assigning an owner/maintainer and peer(s) just like<br>
for other components is a must.<br>
<br>
Thanks,<br>
Niels<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
><br>
> Regards,<br>
> Amar<br>
><br>
><br>
><br>
><br>
><br>
> > Niels<br>
> ><br>
> ><br>
> > > On 06/27/2017 04:08 AM, Niels de Vos wrote:<br>
> > > > On Tue, Jun 27, 2017 at 12:25:11PM +0530, Kotresh Hiremath Ravishankar<br>
> > wrote:<br>
> > > > > Hi,<br>
> > > > ><br>
> > > > > We were looking for faster non-cryptographic hash to be used for the<br>
> > > > > gfid2path infra [1]<br>
> > > > > The initial testing was done with md5 128bit checksum which was a<br>
> > slow,<br>
> > > > > cryptographic hash<br>
> > > > > and using it makes software not complaint to FIPS [2]<br>
> > > > ><br>
> > > > > On searching online a bit we found out xxhash [3] seems to be faster<br>
> > from<br>
> > > > > the results of<br>
> > > > > benchmark tests shared and lot of projects use it. So we have<br>
> > decided to us<br>
> > > > > xxHash<br>
> > > > > and added following files to gluster code base with the patch [4]<br>
> > > > ><br>
> > > > > BSD 2-Clause License:<br>
> > > > > contrib/xxhash/xxhash.c<br>
> > > > > contrib/xxhash/xxhash.h<br>
> > > > ><br>
> > > > > GPL v2 License:<br>
> > > > > tests/utils/xxhsum.c<br>
> > > > ><br>
> > > > > NOTE: We have ignored the code guideline check for these files as<br>
> > > > > maintaining it<br>
> > > > > further becomes difficult.<br>
> > > > ><br>
> > > > > Please comment on the same if there are any issues around it.<br>
> > > ><br>
> > > > How performance critical is the hashing for gfid2path?<br>
> > > ><br>
> > > > What is the plan to keep these files maintained? At minimal we need to<br>
> > > > add these files to MAINTAINERS and the maintainers need to cherry-pick<br>
> > > > updates and bugfixes from the original project. The few patches a year<br>
> > > > makes this a recurring task that should not be forgoten. It would be<br>
> > > > much better to use this as an external library that is provided by the<br>
> > > > distributions. We already rely on OpenSSL, does this library not<br>
> > provide<br>
> > > > an alternative 'FIPS approved' hashing that performs reasonably well?<br>
> > > ><br>
> > > > Some distributions are very strict on bundling external projects, and<br>
> > we<br>
> > > > need to inform the packagers about the additions so that they can<br>
> > handle<br>
> > > > it correctly. Adding an external project to contrib/ should be<br>
> > mentioned<br>
> > > > in the release notes at the very least.<br>
> > > ><br>
> > > > Note that none of the symbols of any public functions in Gluster may<br>
> > > > collide with functions in standard distribution libraries. This causes<br>
> > > > for regular problems with gfapi applications. All exposed symbols that<br>
> > > > get imported in contrib/ should have a gf_ prefix.<br>
> > > ><br>
> > > > Thanks,<br>
> > > > Niels<br>
> > > ><br>
> > > ><br>
> > > > ><br>
> > > > > [1] Issue: <a href="https://github.com/gluster/glusterfs/issues/139" rel="noreferrer" target="_blank">https://github.com/gluster/<wbr>glusterfs/issues/139</a><br>
> > > > > [2] <a href="https://en.wikipedia.org/wiki/Federal_Information_" rel="noreferrer" target="_blank">https://en.wikipedia.org/wiki/<wbr>Federal_Information_</a><br>
> > Processing_Standards<br>
> > > > > [3] <a href="http://cyan4973.github.io/xxHash/" rel="noreferrer" target="_blank">http://cyan4973.github.io/<wbr>xxHash/</a><br>
> > > > > [4] <a href="https://review.gluster.org/#/c/17488/10" rel="noreferrer" target="_blank">https://review.gluster.org/#/<wbr>c/17488/10</a><br>
> > > > ><br>
> > > > ><br>
> > > > ><br>
> > > > > --<br>
> > > > > Thanks and Regards,<br>
> > > > > Kotresh H R and Aravinda VK<br>
> > ><br>
> ><br>
><br>
><br>
><br>
> --<br>
> Amar Tumballi (amarts)<br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div>Thanks and Regards,<br></div>Kotresh H R<br></div></div>
</div>