Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RPM #528

Open
wants to merge 66 commits into
base: master
Choose a base branch
from
Open

RPM #528

wants to merge 66 commits into from

Conversation

DirkHoffmann
Copy link
Contributor

I am not sure if this is mature enough for merging, but I would like to get into contact with the dev team for discussing.
In our project we need to deploy software as RPM packages. I started to provide the necessary .spec file, and the deployment works well. The necessary files with a README are available in the directory RPM/.
I would certainly like to polish the code a bit more before integrating it into your main repository. And maybe you have some ideas how I should organise my contribution for that. Please advise.
Are you happy with a pull request with all commits, or do I need to squash them into the latest commit first?

DirkHoffmann and others added 30 commits March 14, 2017 19:04
With this feature a user application may allocate its own memory
(e.g., via mmap()) and pass it to netmap while registering (NIOCREGIF)
an interface.  If the interface was not already registered, it will pin
the underlying pages in memory and use them for its own netmap_if's,
netmap_ring's and buffers.

Example usage:

  void *mem = mmap(...);
  /* the user must fill a netmap_pools_info header before passing
   * the region to netmap;
   * the header is used to split the region into netmap_if's,
   * netmap_ring's and buffers
   */
  struct netmap_pools_info *pi = mem;
  pi->ring_pool_objtotal = 100;   /* how many rings */
  pi->buf_pool_objtotal = 100000; /* how many buffers */
  /* etc. */
  struct nmreq req;
  /* other nmreq initializations... */
  nmreq_pointer_put(&req, mem);
  req.nr_cmd = NETMAP_POOLS_CREATE;
  fd = open("/dev/netmap");
  ioctl(fd, NIOCREGIF, &req);
  /* pi is now filled with the actual values used */

Possible uses:
- support of hugepages
- persistent memory (e.g., PASTE)

Bugs:
- while a port is registered in this way, no other application can
  open it, not even on non intersecting sets of rings (this may actually
  be a feature);
- each netmap_if, netmap_ring and buffer needs to be contiguous in
  physical memory; netmap_ring's, in particular, are usually bigger
  than 4KiB and netmap may fail to allocate them if the user pages
  are scattered around; note, however, that this is not a problem
  for the above suggested use-cases;
- nmreq_pointer_put() overwrites nr_arg1, nr_arg2 and nr_arg3; this means
  that extra buffers cannot be asked for in the same NIOCREGIF (one may
  ask for extra buffers in another NIOCREGIF, possibily on a dummy port)
- Linux only (for now)
Pre-allocated objects that cross non-contiguous pages cannot be used.
The previous code skipped these objects may setting their vaddr to NULL
in the lut, but this created bugs in the mmap code.
@vmaffione
Copy link
Collaborator

Thanks.
You need to rebase your changes on the current master, and squash everything in a single commit.

@DirkHoffmann
Copy link
Contributor Author

Hmm. I lack experience with squashing and rebase. Maybe you can help me?

$ git rebase master 
First, rewinding head to replay your work on top of it...
Applying: Just recovering old files from 2014. This does *not* work yet.
Applying: Adapted spec file to generate an RPM for the first time (on CentOS 7.4)
Applying: extmem: user-supplied netmap memory region
Using index info to reconstruct a base tree...
M       LINUX/bsd_glue.h
M       LINUX/configure
M       LINUX/netmap_linux.c
M       sys/dev/netmap/netmap.c
M       sys/dev/netmap/netmap_kern.h
M       sys/dev/netmap/netmap_mem2.c
M       sys/dev/netmap/netmap_mem2.h
M       sys/net/netmap.h
M       utils/testmmap.c
<stdin>:57: space before tab in indent.
        #include <linux/mm.h>
<stdin>:67: space before tab in indent.
        #include <linux/mm.h>
<stdin>:77: space before tab in indent.
        #include <linux/mm.h>
<stdin>:90: space before tab in indent.
        #include <linux/mm.h>
<stdin>:226: trailing whitespace.
_netmap_mem_private_new(size_t size, struct netmap_obj_params *p, 
warning: squelched 3 whitespace errors
warning: 8 lines add whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging utils/testmmap.c
CONFLICT (content): Merge conflict in utils/testmmap.c
Auto-merging sys/net/netmap.h
CONFLICT (content): Merge conflict in sys/net/netmap.h
Auto-merging sys/dev/netmap/netmap_mem2.h
CONFLICT (content): Merge conflict in sys/dev/netmap/netmap_mem2.h
Auto-merging sys/dev/netmap/netmap_mem2.c
CONFLICT (content): Merge conflict in sys/dev/netmap/netmap_mem2.c
Auto-merging sys/dev/netmap/netmap.c
CONFLICT (content): Merge conflict in sys/dev/netmap/netmap.c
Auto-merging LINUX/configure
CONFLICT (content): Merge conflict in LINUX/configure
Failed to merge in the changes.
Patch failed at 0003 extmem: user-supplied netmap memory region
The copy of the patch that failed is found in:
   /home/hoffmann/netmap/.git/rebase-apply/patch

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".

There are conflicts. Doh.

Is this because I merged already tag v11.4 into rpm, and your master is still based on v11.3?

How can we sort this out?

@vmaffione
Copy link
Collaborator

This branch looks impossible to recover. I would just create a clean branch starting from the master branch:

$ git checkout -b mybranch origin/master

And the manually copy and git-add the files that you added (maybe taken from your previous branch)

$ cp -r /path/to/other/repo/LINUX/RPM LINUX/RPM
$ git add LINUX/RPM
$ git commit

@DirkHoffmann
Copy link
Contributor Author

DirkHoffmann commented Aug 17, 2018 via email

@vmaffione
Copy link
Collaborator

Pull requests must be based on master. The 11.4 tag was created only for legacy ptnetmap usage. It's not a stable release.
Thanks

@DirkHoffmann
Copy link
Contributor Author

Pull requests must be based on master. The 11.4 tag was created only for legacy ptnetmap usage. It's not a stable release.

Thank you, understood. Is there a forum, newsgroup or mailing list, where I can get the information directly which tags are important or official and which ones are supposed to be "only something"? I will rebase, but would like to avoid the double work in future, of course.

I have to finalise a formal request to contribute on your github, with respect to my employer. Then I come back to this. In the meantime, I moved the RPM directory to LINUX (as it is specific to that and it made no sense for me on the same level as WINDOWS). So there will be quite some changes. If you want to keep your tracker clean and close this PR, it is fine for me, and I will open a clean one later this month.

@vmaffione
Copy link
Collaborator

Please ignore all the tags. Some are for specific use (e.g. 11.4 for ptnetmap), but definitely none is for contributing code.
Please just rebase your work on the master branch. Btw, your contribution should be some files in a new directory, so there should not be any merge conflicts or double work.

Feel free to close yourself this PR when you are ready and open another one.

Thanks

@DirkHoffmann
Copy link
Contributor Author

DirkHoffmann commented Sep 11, 2018 via email

@vmaffione
Copy link
Collaborator

This software is rolling release, so there are no releases.
If you want to use a fixed version, what I would recommend is just to pick current master (f97b2b2), and stick to that.
(Unless you find some bug in the current master, obviously. In that case please report the bug so that we can try to fix it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants