[dpdk-dev] GitHub sandbox for the DPDK community
Neil On 5/3/15, 2:00 PM, "Neil Horman" wrote: >On Fri, May 01, 2015 at 07:10:11PM +, Wiles, Keith wrote: >> >> >> On 5/1/15, 1:48 PM, "Neil Horman" wrote: >> >> >On Fri, May 01, 2015 at 10:31:08AM -0700, Matthew Hall wrote: >> >> On Fri, May 01, 2015 at 12:45:12PM -0400, Neil Horman wrote: >> >> > Yes, but as you said above, using a web browser doesn't make >> >>reviewing patches >> >> > faster. In fact, I would assert that it slows the process down, as >> >>it prevents >> >> > quick, easy command line access to patch review (as you have with a >> >>properly >> >> > configured MUA). That seems like we're going in the opposite >> >>direction of at >> >> > least one problem we would like to solve. >> >> >> >> Normally I'm a big command-line supporter. However I have found >> >>reviewing >> >> patches by email for me is about the most painful workflow. >> >> >> >> The emails are pages and pages. >> >> >> >So collapse the quoted text (see below) >> > >> >> The replies from commenters are buried in the walls of text. >> >> >> >Again, collapse the text, many MUA's let you do that, its not a feature >> >unique >> >to github. >> > >> >> Replies to replies keep shifting farther off the edge of the screen. >> >>The code >> >> gets weirder and weirder to try to read. >> >> >> >Text Collapse will reformat that for you. >> > >> >> Quickly reading over the patchset by scrolling through to get the >> >>flavor of >> >> it, to see if I'm qualified to review it, and look at the parts I >> >>actually >> >> know about is much harder. >> >> >> >Thats what the origional post is for, no? Look at that to determine if >> >you are >> >qualified to read it. >> > >> >> I can go to one place to see every candidate patchset out there, the >>GH >> >>Pull >> >> Request page. Then I can just sync up the branch and test it on my >>own >> >>systems >> >> to see if it works, not just try to read it. >> >> >> >how is that different from a mailing list? both let you search for >> >posts, and >> >both allow you to sync git branches (github via git remote/pull, >>mailing >> >list >> >via git am) >> > >> >> Github automatically minimizes old comments that are already fixed, >>so >> >>they >> >> don't keep consuming space and mental bandwidth from the review. >> >An MUA can do that too. IIRC evolution and thunderbird both have >>collapse >> >features. I'm sure others do too. >> >> Not all email clients allow for collapsing threads, I am using outlook >>for >> Mac and I do not think the windows version has that feature. I am not >>sure >> Apple mail client can handle collapsing or not as I am stuck with >>outlook >> as my email virus (I mean client) :-) >> >Ok, but this isn't about ensuring we can do everything with all email >clients. >You asserted that you wanted a feature whereby conversations can be >collapsed I never stated I needed conversation collapse support. I pointing out that not everyone has that feature and besides that is not the point, in the gh style the comments are different and some like that style just like you prefer your style. Not everyone is comfortable with the style we have today. The community needs to grow and will IMO because we will be getting people from VNF/NFV world. I am sorry some will need to change a bit, but the change will be for the community and those to come. >for easier parsing of the most recent post. I'm fine without that, but >if you >want it, thats fine too. My point was that you can have that feature >without >needing to move our development environment. If your current client >doesn't >support doing that, you're free to choose another MUA, since its clear >that many >MUA's do support what you want. > >Before you assert that your employer is >going to mandate that you use a specific MUA, and that you can't possibly >change, I'll indicate that theres nothing wrong with using a different >email >address/service to do your upstream work. I use my tuxdriver address >rather >than my redhat address simply because (among other reasons), I don't want >to >have to log into my corporate vpn every time I send upstream email. > >If you then plan on asserting that your employer mandate that you use your >corporate email address to do upstream work, I'll indicate that you have a >problem with your employer. They require that you do work using a >process and >tool suite that is non-condusive/incompatible with your preferred >workflow. > >My overall point is, while this is a fine feature I suppose, I don't need >it, >and while I don't want to prevent others from using it, I don't consider >it a >reason to undertake the effort of moving hosting to a different location, >because the feature can be had without doing so. > >> The point here is all emails clients have different ways of displaying >>the >> information some good some bad. I see the GitHub method to be different, >> but for me I am able to understand the way it handles comments and >>patches. >> >Thats great, but
[dpdk-dev] GitHub sandbox for the DPDK community
On 5/1/15, 10:56 AM, "Wiles, Keith" wrote: >Hi Everyone, > >I believe the DPDK community would benefit from moving to GitHub as the >primary DPDK site. http://github.com Here is a good page showing the GitHub features: https://github.com/features/ > >I believe the DPDK community can benefit from being at a very well know >world wide site. GitHub seems to have the most eyes of any of the open >source Git repos today and it appears they have more then twice as many >developers. GitHub has a number of features I see as some good additions >to >our community using the GitHub organization account type. > >The cost for an organization account is $0 as long as we do not need more >then 5 private repos. 10 private repos is $25/month and had other plans >for more. I do not see us needing more then 5 private repos today and the >only reason I can see having a private repo is to do some prep work on the >repo before making public. Every contributor would need to create a GitHub >personal account, which is at no cost unless you need more then 5 private >repos. In both accounts you can have unlimited public repos. > >https://help.github.com/articles/where-can-i-find-open-source-projects-to- >w >ork-on/ > >http://www.sitepoint.com/using-git-open-source-projects/ > >- Adding more committers can lead to a security problems for 6Wind (I >assume). >- 6Wind appearing to own DPDK.org is not a good message to the community. > - Not assuming 6Wind?s dpdk.org site will disappear only where the >community stores the master repos and how the community interacts with the >master. >- Permission and access levels in dpdk.org is only one level and we can >benefit from having 4 levels and teams as well. >- The patch process today suffers from timely reviews, which will not be >fixed by moving. > - GitHub has a per pull request discussions area, which gives a clean >way to review all discussions on a specific change. >- The current patch model is clone/modify/commit/send patch set >- The model with GitHub is fork on GitHub/modify/commit/send pull >request >- The patchwork web site is reasonable, but has some draw backs in >maintaining the site. > - GitHub manages the patches via pull requests and can be easily seen >via a web browser. > - The down side is you do have to use a web browser to do some work, but >the bulk of the everyday work would be done as it is today. >- I think we all have a web browser now :-) >- GitHub has team support and gives a group better control plus >collaboration is much easier as we have a external location to work. > - Most companies have some pretty high security level and being to >collaborate between two or more companies is very difficult if one company >is hosting the repo behind a firewall. > - Using GitHub and teams would make collaboration a lot easier or >collaboration between two or more user accounts as well. >- GitHub has a Web Page system, which can be customized for the community >needs via a public or private repo. >- We still need a dpdk.org email list I believe as I did not find one at >GitHub. > - We can also forward GitHub emails to the list. > - I believe you can reply to an email from GitHub and the email will get >appended to the discussion thread. > >As most do not like to read long emails :-) I will stop here and add one >more thing. > >I believe moving to GitHub for the DPDK community has a lot of advantages, >but I also understand it will be different process and will cause a bit of >issues as we convert. Having more eyes plus in a well know public location >plus utilizing the extra features on Github plus a better public >modifiable web pages is a few big advantages for the DPDK community. > >I have create a sandbox on GitHub for anyone to play with using GitHub. >You will need to create a GitHub account and an email me your account name >to add you to the organization site as a contributor. > >The GitHub site is not a fork of dpdk.org only a sandbox to play with how >GitHub can help the community to gain more developers in a clean manner. > >https://github.com/dpdk-org > > >Regards >++Keith > > > > > >
[dpdk-dev] [PATCH v5 0/5] Refactor module `eventfd_link'
Thomas: Would review this patch this week. On 4/28/2015 10:36 PM, Thomas Monjalon wrote: > Huawei, Changchun, > Any opinion about these patches? > > 2015-04-16 14:48, Pavel Boldin: >> This patchset contains refactoring steps for the `eventfd_link' module >> of the DPDK's `librte_vhost' part. >> >> The commit messages are updated to include `Signed-off-by'. >> >> Pavel Boldin (5): >> vhost: eventfd_link: moving ioctl to a function >> vhost: eventfd_link: add function fget_from_files >> vhost: eventfd_link: fix ioctl return values >> vhost: eventfd_link: replace copy-pasted sys_close >> vhost: eventfd_link: removing extra #includes >> >> lib/librte_vhost/eventfd_link/eventfd_link.c | 181 >> +-- >> 1 file changed, 87 insertions(+), 94 deletions(-) > >
[dpdk-dev] [PATCH] virtio: Fix enqueue/dequeue can't handle chained vring descriptors.
Vring enqueue need consider the 2 cases:
1. Vring descriptors chained together, the first one is for virtio header, the
rest are for real data;
2. Only one descriptor, virtio header and real data share one single
descriptor;
So does vring dequeue.
Signed-off-by: Changchun Ouyang
---
lib/librte_vhost/vhost_rxtx.c | 60 +++
1 file changed, 44 insertions(+), 16 deletions(-)
diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 510ffe8..3135883 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -59,7 +59,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
struct virtio_net_hdr_mrg_rxbuf virtio_hdr = {{0, 0, 0, 0, 0, 0}, 0};
uint64_t buff_addr = 0;
uint64_t buff_hdr_addr = 0;
- uint32_t head[MAX_PKT_BURST], packet_len = 0;
+ uint32_t head[MAX_PKT_BURST];
uint32_t head_idx, packet_success = 0;
uint16_t avail_idx, res_cur_idx;
uint16_t res_base_idx, res_end_idx;
@@ -113,6 +113,10 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
rte_prefetch0(&vq->desc[head[packet_success]]);
while (res_cur_idx != res_end_idx) {
+ uint32_t offset = 0;
+ uint32_t data_len, len_to_cpy;
+ uint8_t plus_hdr = 0;
+
/* Get descriptor from available ring */
desc = &vq->desc[head[packet_success]];
@@ -125,7 +129,6 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
/* Copy virtio_hdr to packet and increment buffer address */
buff_hdr_addr = buff_addr;
- packet_len = rte_pktmbuf_data_len(buff) + vq->vhost_hlen;
/*
* If the descriptors are chained the header and data are
@@ -136,24 +139,44 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
desc = &vq->desc[desc->next];
/* Buffer address translation. */
buff_addr = gpa_to_vva(dev, desc->addr);
- desc->len = rte_pktmbuf_data_len(buff);
} else {
buff_addr += vq->vhost_hlen;
- desc->len = packet_len;
+ plus_hdr = 1;
}
+ data_len = rte_pktmbuf_data_len(buff);
+ len_to_cpy = RTE_MIN(data_len, desc->len);
+ do {
+ if (len_to_cpy > 0) {
+ /* Copy mbuf data to buffer */
+ rte_memcpy((void *)(uintptr_t)buff_addr,
+ (const void *)(rte_pktmbuf_mtod(buff,
const char *) + offset),
+ len_to_cpy);
+ PRINT_PACKET(dev, (uintptr_t)buff_addr,
+ len_to_cpy, 0);
+
+ desc->len = len_to_cpy + (plus_hdr ?
vq->vhost_hlen : 0);
+ offset += len_to_cpy;
+ if (desc->flags & VRING_DESC_F_NEXT) {
+ desc = &vq->desc[desc->next];
+ buff_addr = gpa_to_vva(dev, desc->addr);
+ len_to_cpy = RTE_MIN(data_len - offset,
desc->len);
+ } else
+ break;
+ } else {
+ desc->len = 0;
+ if (desc->flags & VRING_DESC_F_NEXT)
+desc = &vq->desc[desc->next];
+ else
+ break;
+ }
+ } while (1);
+
/* Update used ring with desc information */
vq->used->ring[res_cur_idx & (vq->size - 1)].id =
head[packet_success];
- vq->used->ring[res_cur_idx & (vq->size - 1)].len = packet_len;
-
- /* Copy mbuf data to buffer */
- /* FIXME for sg mbuf and the case that desc couldn't hold the
mbuf data */
- rte_memcpy((void *)(uintptr_t)buff_addr,
- rte_pktmbuf_mtod(buff, const void *),
- rte_pktmbuf_data_len(buff));
- PRINT_PACKET(dev, (uintptr_t)buff_addr,
- rte_pktmbuf_data_len(buff), 0);
+ vq->used->ring[res_cur_idx & (vq->size - 1)].len =
+ offset + vq->vhost_hlen;
res_cur_idx++;
packet_success++;
@@ -583,7 +606,14 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t
queue_id,
desc = &vq->desc[head[entry_success]];
/* Discard first buffer as it is the virtio header */
- de
[dpdk-dev] GitHub sandbox for the DPDK community
On 2015-05-01 17:56, Wiles, Keith wrote: > I believe the DPDK community would benefit from moving to GitHub as the > primary DPDK site. http://github.com > [...] While I'm really mostly a DPDK outsider, I'd like to express my support for this suggestion. For my part, I'm mostly interested in the general development discussion on dpdk-dev, not details about changes in e.g., the i40 PMD code. Architecture discussions, usage questions etc tend to get drowned in an endless flow of patches. > - GitHub has a per pull request discussions area, which gives a clean > way to review all discussions on a specific change. ... and I think this is one of the great features of github. For an example on how discussions on merge requests can look, take a look at e.g., one of the rust-lang merge requests: https://github.com/rust-lang/rust/pull/25058#discussion_r29548050 which shows nice git commit links, discussions about relevant parts of the commits, automatic test results etc. Bug reports also get very nicely integrated into the workflow. I used to be more of a command-line guy, but I'm starting to see the benefits of the github way of doing things, and I think it would be a nice improvement for DPDK to start using it as well. // Simon
[dpdk-dev] [PATCH] vhost: Fix compilation issue
Minor fix for the referring of a pointer when debug and dump is enabled. The original commit is: 72ec8d77 Signed-off-by: Changchun Ouyang --- examples/vhost/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/vhost/main.c b/examples/vhost/main.c index 195d82f..8cec7c2 100644 --- a/examples/vhost/main.c +++ b/examples/vhost/main.c @@ -1105,7 +1105,7 @@ find_local_dest(struct virtio_net *dev, struct rte_mbuf *m, "(%"PRIu64") TX: pkt to local VM device id:" "(%"PRIu64") vlan tag: %d.\n", dev->device_fh, dev_ll->vdev->dev->device_fh, - vlan_tag); + (int)*vlan_tag); break; } -- 1.8.4.2
[dpdk-dev] [PATCH 1/3] lib: set LDLIBS for each library
Hi Sergio, On 04/15/2015 11:30 AM, Sergio Gonzalez Monroy wrote: > This patch introduces a new LDLIBS variable to be set per library. > Its purpose is to especify the library's dependent libraries to > be explicitly linked against. > > Given the circular dependencies between eal, malloc, mempool and ring, > we work around it by not linking eal against its dependent DPDK libraries. > Therefore, eal will not have proper DT_NEEDED entries (ie. no DT_NEEDED > entries for librte_malloc and librte_mempool). > > This means that any application that links against eal, needs to be > certain of linking against malloc, mempool and ring too, to prevent > a case where the application does not directly use mempool (therefore > no DT_NEEDED entry). In such case, the application will fail to start as > eal does not have a DT_NEEDED entry for mempool either. > > Signed-off-by: Sergio Gonzalez Monroy > --- > [...] > --- a/lib/librte_ip_frag/Makefile > +++ b/lib/librte_ip_frag/Makefile > @@ -41,6 +41,8 @@ EXPORT_MAP := rte_ipfrag_version.map > > LIBABIVER := 1 > > +LDLIBS += -lrte_eal -lrte_malloc -lethdev > + > #source files > SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv4_fragmentation.c > SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv6_fragmentation.c > @@ -55,5 +57,6 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_IP_FRAG)-include += > rte_ip_frag.h > > # this library depends on rte_ether > DEPDIRS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += lib/librte_mempool lib/librte_ether > +DEPDIRS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += lib/librte_malloc lib/librte_mbuf > > include $(RTE_SDK)/mk/rte.lib.mk It seems that in the rest of the patch LDLIBS and and DEPDIRS-y are often similar, but that's not the case here. Can you confirm it's because librte_ip_frag only uses inlined functions from librte_mempool and librte_mbuf? Did you use a specific scripting method to find the good value for LDLIBS or did you do it manually? Is there a way to check that the values are correct? Thanks, Olivier > diff --git a/lib/librte_ivshmem/Makefile b/lib/librte_ivshmem/Makefile > index 16defdb..fab6f5f 100644 > --- a/lib/librte_ivshmem/Makefile > +++ b/lib/librte_ivshmem/Makefile > @@ -40,6 +40,8 @@ EXPORT_MAP := rte_ivshmem_version.map > > LIBABIVER := 1 > > +LDLIBS += -lrte_mempool > + > # all source are stored in SRCS-y > SRCS-$(CONFIG_RTE_LIBRTE_IVSHMEM) := rte_ivshmem.c > > diff --git a/lib/librte_jobstats/Makefile b/lib/librte_jobstats/Makefile > index 136a448..04589d4 100644 > --- a/lib/librte_jobstats/Makefile > +++ b/lib/librte_jobstats/Makefile > @@ -41,6 +41,8 @@ EXPORT_MAP := rte_jobstats_version.map > > LIBABIVER := 1 > > +LDLIBS += -lrte_eal > + > # all source are stored in SRCS-y > SRCS-$(CONFIG_RTE_LIBRTE_JOBSTATS) := rte_jobstats.c > > diff --git a/lib/librte_kni/Makefile b/lib/librte_kni/Makefile > index 7107832..504ecf7 100644 > --- a/lib/librte_kni/Makefile > +++ b/lib/librte_kni/Makefile > @@ -40,6 +40,8 @@ EXPORT_MAP := rte_kni_version.map > > LIBABIVER := 1 > > +LDLIBS += -lrte_eal -lrte_malloc -lethdev > + > # all source are stored in SRCS-y > SRCS-$(CONFIG_RTE_LIBRTE_KNI) := rte_kni.c > > diff --git a/lib/librte_kvargs/Makefile b/lib/librte_kvargs/Makefile > index 87b09f2..173e1ac 100644 > --- a/lib/librte_kvargs/Makefile > +++ b/lib/librte_kvargs/Makefile > @@ -42,6 +42,8 @@ EXPORT_MAP := rte_kvargs_version.map > > LIBABIVER := 1 > > +LDLIBS += -lrte_eal > + > # all source are stored in SRCS-y > SRCS-$(CONFIG_RTE_LIBRTE_KVARGS) := rte_kvargs.c > > diff --git a/lib/librte_lpm/Makefile b/lib/librte_lpm/Makefile > index 35e6389..125d52e 100644 > --- a/lib/librte_lpm/Makefile > +++ b/lib/librte_lpm/Makefile > @@ -41,6 +41,8 @@ EXPORT_MAP := rte_lpm_version.map > > LIBABIVER := 1 > > +LDLIBS += -lrte_eal -lrte_malloc > + > # all source are stored in SRCS-y > SRCS-$(CONFIG_RTE_LIBRTE_LPM) := rte_lpm.c rte_lpm6.c > > diff --git a/lib/librte_malloc/Makefile b/lib/librte_malloc/Makefile > index 947e41c..3e7348f 100644 > --- a/lib/librte_malloc/Makefile > +++ b/lib/librte_malloc/Makefile > @@ -40,6 +40,8 @@ CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 > > EXPORT_MAP := rte_malloc_version.map > > +LDLIBS += -lrte_eal > + > # all source are stored in SRCS-y > SRCS-$(CONFIG_RTE_LIBRTE_MALLOC) := rte_malloc.c malloc_elem.c malloc_heap.c > > diff --git a/lib/librte_mbuf/Makefile b/lib/librte_mbuf/Makefile > index 080f3cf..d819891 100644 > --- a/lib/librte_mbuf/Makefile > +++ b/lib/librte_mbuf/Makefile > @@ -40,6 +40,8 @@ EXPORT_MAP := rte_mbuf_version.map > > LIBABIVER := 1 > > +LDLIBS += -lrte_eal > + > # all source are stored in SRCS-y > SRCS-$(CONFIG_RTE_LIBRTE_MBUF) := rte_mbuf.c > > diff --git a/lib/librte_mempool/Makefile b/lib/librte_mempool/Makefile > index 940d1f7..8ebebb6 100644 > --- a/lib/librte_mempool/Makefile > +++ b/lib/librte_mempool/Makefile > @@ -40,6 +40,8 @@ EXPORT_MAP := rte_mempool_version.map > > LIBABIVER := 1 > > +LDLIBS += -lrte_eal -lrte_malloc -lrte_rin
[dpdk-dev] [RFC PATCH 0/8] reduce header dependency on rte_mbuf.h
Hi Bruce, On 04/23/2015 03:03 PM, Bruce Richardson wrote: > A large number of our header files and libraries are dependent on one > another, > which can lead to problems with circular dependencies if trying to tie some of > those libraries together, e.g. when prototyping with pktdev, or other schemes > to get a common API for ethdev/rings/KNI. :-) > > One small way to reduce issues when doing this is to eliminate #includes when > they are not needed. While most includes in our headers are necessary, one > common pattern seen is where a library just takes mbufs as part of it's API, > but does not de-reference those in the header file. In cases like this, it's > not necessary to include the whole mbuf header file just to allow pointers to > mbuf structures - a forward declaration of "struct rte_mbuf" will do. > Including the mbuf header file, also triggers inclusion of the mempool headers > which causes the inclusion of the ring headers amongst others. > > Therefore, I propose changing the header files for our libraries to just use > the forward declaration instead of the full header inclusion where possible. Series Acked-by: Olivier Matz
[dpdk-dev] Compiling files with .S with GCC
Hello, On 04/26/2015 06:55 PM, Wiles, Keith wrote: > > > On 4/26/15, 11:53 AM, "Wiles, Keith" wrote: > >> Hi All, >> >> I noticed in my builds with foo.S file I would get a warning from the >> compiler the foo_s.o.tmp linker file will not be used as the code is not >> linked. A strange error and the foo_s.o file would not be created. In the >> mk/internal/rte.compile-pre.mk we have the following >> >> # command to assemble a .S file to generate an object >> ifeq ($(USE_HOST),1) >> S_TO_O = $(CPP) $(HOST_CPPFLAGS) $($(@)_CPPFLAGS) $(HOST_EXTRA_CPPFLAGS) >> $< $(@).tmp && \ >>$(HOSTAS) $(HOST_ASFLAGS) $($(@)_ASFLAGS) $(HOST_EXTRA_ASFLAGS) -o $@ >> $(@).tmp >> S_TO_O_STR = $(subst ','\'',$(S_TO_O)) #'# fix syntax highlight >> S_TO_O_DISP = $(if $(V),"$(S_TO_O_STR)"," HOSTAS $(@)") >> else >> S_TO_O = $(CPP) $(CPPFLAGS) $($(@)_CPPFLAGS) $(EXTRA_CPPFLAGS) $< -o >> $(@).tmp && \ >>$(AS) $(ASFLAGS) $($(@)_ASFLAGS) $(EXTRA_ASFLAGS) -o $@ $(@).tmp >> S_TO_O_STR = $(subst ','\'',$(S_TO_O)) #'# fix syntax highlight >> S_TO_O_DISP = $(if $(V),"$(S_TO_O_STR)"," AS $(@)") >> endif >> >> I had to change the above ?.tmp? strings to ?.s? to remove this warning >> and get the foo_s.o file created. Using the .s the file name becomes >> foo_s.o.s > > Compiler used on Ubuntu 14.04 'gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2' > >> >> Did not try this with clang or any other compiler, but I expect we can >> safely change the ?.tmp? to ?.s? IMO >> >> Anyone else notice this problem? I tested a similar use-case and I don't reproduce the issue. I don't don't understand why replacing $(@).tmp by $(@).s would solve it. The file $(@).tmp is used as a temporary file to store the preprocessed version of the $(@).s file. I agree that using the .s extension is not a bad choice (see below), but to me it is not the proper solution to your problem. >From https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html file.s Assembler code. file.S file.sx Assembler code that must be preprocessed. Regards, Olivier
[dpdk-dev] [PATCH] kni: fix igb and ixgbe kni ethtool get_link op
Hi Chia
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Shelton Chia
> Sent: Friday, April 3, 2015 11:19 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] kni: fix igb and ixgbe kni ethtool get_link op
>
> igb and ixgbe's link detected always return yes, fix get_link func refer to
> get_settings, it works correctly for my i350 and 82599es nic.
Could you help to add more detailed description of why we need these code
changes? Thanks!
>
> Signed-off-by: Shelton Chia
> ---
> .../linuxapp/kni/ethtool/igb/igb_ethtool.c | 18 ++-
> .../linuxapp/kni/ethtool/ixgbe/ixgbe_ethtool.c | 26
> +-
> 2 files changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ethtool.c
> b/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ethtool.c
> index f3c48b2..5457f48 100644
> --- a/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ethtool.c
> +++ b/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ethtool.c
> @@ -383,19 +383,15 @@ static int igb_set_settings(struct net_device
> *netdev, struct ethtool_cmd *ecmd) static u32 igb_get_link(struct
> net_device *netdev) {
> struct igb_adapter *adapter = netdev_priv(netdev);
> - struct e1000_mac_info *mac = &adapter->hw.mac;
> + struct e1000_hw *hw = &adapter->hw;
> + u32 status;
>
> - /*
> - * If the link is not reported up to netdev, interrupts are disabled,
> - * and so the physical link state may have changed since we last
> - * looked. Set get_link_status to make sure that the true link
> - * state is interrogated, rather than pulling a cached and possibly
> - * stale link state from the driver.
> - */
> - if (!netif_carrier_ok(netdev))
> - mac->get_link_status = 1;
> + status = E1000_READ_REG(hw, E1000_STATUS);
Can ' check_for_link ' be used for checking the link here? It needs to support
all possible link types.
>
> - return igb_has_link(adapter);
> + if (status & E1000_STATUS_LU)
> + return 1;
> + else
> + return 0;
> }
>
> static void igb_get_pauseparam(struct net_device *netdev, diff --git
> a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_ethtool.c
> b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_ethtool.c
> index 11472bd..184b14f 100644
> --- a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_ethtool.c
> +++ b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_ethtool.c
> @@ -801,6 +801,30 @@ static void ixgbe_get_regs(struct net_device *netdev,
> struct ethtool_regs *regs,
> regs_buff[1128] = IXGBE_READ_REG(hw, IXGBE_MFLCN); }
>
> +static u32 ixgbe_get_link(struct net_device *netdev) {
> + struct ixgbe_adapter *adapter = netdev_priv(netdev);
> + struct ixgbe_hw *hw = &adapter->hw;
> + u32 link_speed = 0;
> + bool link_up;
> +
> + if (!in_interrupt()) {
> + hw->mac.ops.check_link(hw, &link_speed, &link_up, false);
As done in kernel driver function ' ixgbe_watchdog_update_link ()', more checks
may be needed.
Regards,
Helin
> + } else {
> + /*
> + * this case is a special workaround for RHEL5 bonding
> + * that calls this routine from interrupt context
> + */
> + link_speed = adapter->link_speed;
> + link_up = adapter->link_up;
> + }
> +
> + if (link_up)
> + return 1;
> + else
> + return 0;
> +}
> +
> static int ixgbe_get_eeprom_len(struct net_device *netdev) {
> struct ixgbe_adapter *adapter = netdev_priv(netdev); @@ -2838,7
> +2862,7 @@ struct ethtool_ops ixgbe_ethtool_ops = {
> .get_wol= ixgbe_get_wol,
> .set_wol= ixgbe_set_wol,
> .nway_reset = ixgbe_nway_reset,
> - .get_link = ethtool_op_get_link,
> + .get_link = ixgbe_get_link,
> .get_eeprom_len = ixgbe_get_eeprom_len,
> .get_eeprom = ixgbe_get_eeprom,
> .set_eeprom = ixgbe_set_eeprom,
> --
> 2.3.5
[dpdk-dev] GitHub sandbox for the DPDK community
On 04/05/15 08:52, Simon Ka*gstro"m wrote: > On 2015-05-01 17:56, Wiles, Keith wrote: >> I believe the DPDK community would benefit from moving to GitHub as the >> primary DPDK site. http://github.com >> [...] > While I'm really mostly a DPDK outsider, I'd like to express my support > for this suggestion. For my part, I'm mostly interested in the general > development discussion on dpdk-dev, not details about changes in e.g., > the i40 PMD code. Architecture discussions, usage questions etc tend to > get drowned in an endless flow of patches. Indeed. I was proposing in this mail: http://dpdk.org/ml/archives/dev/2015-April/016909.html to have dpdk-users and dpdk-dev. >> - GitHub has a per pull request discussions area, which gives a clean >> way to review all discussions on a specific change. > ... and I think this is one of the great features of github. For an > example on how discussions on merge requests can look, take a look at > e.g., one of the rust-lang merge requests: > > https://github.com/rust-lang/rust/pull/25058#discussion_r29548050 > > which shows nice git commit links, discussions about relevant parts of > the commits, automatic test results etc. Bug reports also get very > nicely integrated into the workflow. > > > I used to be more of a command-line guy, but I'm starting to see the > benefits of the github way of doing things, and I think it would be a > nice improvement for DPDK to start using it as well. I agree that patch/bug discussion gets slightly improved, and scoped (which is good and bad). I think we need to find a good integration with the mailing list in order to both coexist. Discussions cannot be broken into ML and GH. marc > > // Simon
[dpdk-dev] [PATCH v6 1/2] Simplify the ifdefs in rte.app.mk.
Hi Keith, On 05/01/2015 04:22 PM, Keith Wiles wrote: > Trying to simplify the ifdefs in rte.app.mk to make the code > more readable and maintainable by moving LDLIBS variable to use > the same style as LDLIBS-y being used in the rest of the code. > > Added a new variable called EXTRA_LDLIBS to be used by example apps > instead of using LDLIBS directly. The new internal variable _LDLIBS > should not be used outside of the rte.app.mk file. The makefiles > can still use LDLIBS, but I would suggest using EXTRA_LDLIBS instead. Why are you suggesting to change LIBS to EXTRA_LIBS? We discussed in a previous thread that EXTRA_* variables should (as much as possible) be kept empty in Makefiles as it allows a user to append things in them. By the way, it would be easier to follow the different versions of your patches if you add "--in-reply-to " in your git-send-email command, as described in http://dpdk.org/dev Regards, Olivier > > Signed-off-by: Keith Wiles > --- > examples/dpdk_qat/Makefile | 4 +- > examples/vm_power_manager/Makefile | 2 +- > mk/rte.app.mk | 242 > + > 3 files changed, 63 insertions(+), 185 deletions(-) > > diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile > index f1e06a1..90ca1d3 100644 > --- a/examples/dpdk_qat/Makefile > +++ b/examples/dpdk_qat/Makefile > @@ -77,8 +77,8 @@ else > ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a > endif > > -LDLIBS += -L$(ICP_ROOT)/build > -LDLIBS += $(ICP_LIBRARY_PATH) \ > +EXTRA_LDLIBS += -L$(ICP_ROOT)/build > +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \ > -lz \ > -losal \ > -ladf_proxy \ > diff --git a/examples/vm_power_manager/Makefile > b/examples/vm_power_manager/Makefile > index 113dbc4..8fb78d4 100644 > --- a/examples/vm_power_manager/Makefile > +++ b/examples/vm_power_manager/Makefile > @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c > CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/ > CFLAGS += $(WERROR_FLAGS) > > -LDLIBS += -lvirt > +EXTRA_LDLIBS += -lvirt > > # workaround for a gcc bug with noreturn attribute > # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 > diff --git a/mk/rte.app.mk b/mk/rte.app.mk > index 62a76ae..b8030d2 100644 > --- a/mk/rte.app.mk > +++ b/mk/rte.app.mk > @@ -1,7 +1,7 @@ > # BSD LICENSE > # > -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. > -# Copyright(c) 2014 6WIND S.A. > +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. > +# Copyright(c) 2014-2015 6WIND S.A. > # All rights reserved. > # > # Redistribution and use in source and binary forms, with or without > @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT) > endif > > # default path for libs > -LDLIBS += -L$(RTE_SDK_BIN)/lib > +_LDLIBS-y += -L$(RTE_SDK_BIN)/lib > > # > # Include libraries depending on config if NO_AUTOLIBS is not set > @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib > # > ifeq ($(NO_AUTOLIBS),) > > -LDLIBS += --whole-archive > +_LDLIBS-y += --whole-archive > > -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y) > -LDLIBS += -l$(RTE_LIBNAME) > -endif > +_LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS)+= -l$(RTE_LIBNAME) > > ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) > > -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) > -LDLIBS += -lrte_distributor > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y) > -LDLIBS += -lrte_reorder > -endif > +_LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)+= -lrte_distributor > +_LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)+= -lrte_reorder > > -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) > ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) > -LDLIBS += -lrte_kni > -endif > +_LDLIBS-$(CONFIG_RTE_LIBRTE_KNI)+= -lrte_kni > +_LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM)+= -lrte_ivshmem > endif > > -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y) > -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) > -LDLIBS += -lrte_ivshmem > -endif > -endif > +_LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE) += -lrte_pipeline > +_LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE) += -lrte_table > +_LDLIBS-$(CONFIG_RTE_LIBRTE_PORT) += -lrte_port > +_LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer > +_LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash > +_LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats > +_LDLIBS-$(CONFIG_RTE_LIBRTE_LPM)+= -lrte_lpm > +_LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power > +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)+= -lrte_acl > +_LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter > > -ifeq ($(CONFIG_RTE_LIBRTE_PIPELINE),y) > -LDLIBS += -lrte_pipeline > -endif > +_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched > +_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lm > +_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrt > > -ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y) > -LDLIBS += -lrte_table > -endif > - > -ifeq ($(CONFIG_RTE_LIBRTE_PORT),y) > -LDLIBS += -lrte_port > -endi
[dpdk-dev] [PATCH 00/10] Improve cast alignment for strict aligned machines
Hi Cyril, On 04/29/2015 06:15 PM, Cyril Chemparathy wrote: > This series contains a few improvements that allow the DPDK code base to build > properly on machines that enforce strict pointer cast alignment constraints. > > When dealing with packet data which could be arbitrarily aligned, we get the > compiler to do the right thing by (a) making sure that header types are > packed, and (b) introducing and using unaligned_uint(16|32|64)_t types when > upcasting from byte pointers. > > In a few other instances, we know apriori that the pointer cast cannot > possibly break alignment. This applies to the changes in mempool, hash, mbuf, > and the ethdev stats code. Here, we simply silence the compiler by casting > through (void *) using the RTE_PTR_(ADD|SUB) macros. > > Finally, we introduce a new rte_pktmbuf_mtod_offset() helper to return a type > casted pointer to an offset within the packet data. This replaces the > following commonly used pattern: > (struct foo *)(rte_pktmbuf_mtod(m, char *) + offset) > with: > rte_pktmbuf_mtod_offset(m, struct foo *, offset) > To ensure consistency, the above transform was applied throughout the code > base using the coccinelle semantic patching tool. > Before diving into the patches, I'm wondering if adding aligned(1) or (packed) attribute at some places would have a performance impact on supported architectures (Intel or IBM Power). Did you manage to test it? Regards, Olivier
[dpdk-dev] GitHub sandbox for the DPDK community
On 5/2/2015 1:19 AM, Aaro Koskinen wrote: > Hi, > > On Fri, May 01, 2015 at 12:45:12PM -0400, Neil Horman wrote: >>> - GitHub manages the patches via pull requests and can be easily seen >>> via a web browser. >>> - The down side is you do have to use a web browser to do some work, but >>> the bulk of the everyday work would be done as it is today. >>> - I think we all have a web browser now :-) >> Yes, but as you said above, using a web browser doesn't make reviewing >> patches >> faster. In fact, I would assert that it slows the process down, as it >> prevents quick, easy command line access to patch review (as you have with a >> properly configured MUA). That seems like we're going in the opposite >> direction of at least one problem we would like to solve. > I agree. Having a web browser interface for reviews etc. is acceptable > only if people can still continue to use e-mail if they prefer. > I don't know how github works in this regard but patches should still > appear on the mailing list and it should be still possible to comment > them via mailing list. How it comes? As I know if use github, the maillist will be meaningless. And do you know some area on the earth maybe have issue with github? Anyway, Github is OK to me, but I prefer maillist :). Thanks, Michael > A. >
[dpdk-dev] build issue with out-of-tree builds and multiple mounts
Hi! I'm trying to do a out-of-tree build of DPDK 2.0.0 (with make -C and O=), but failing with errors such as In file included from [...]/lib/librte_eal/common/include/rte_eal_memconfig.h:40:0, [...] rte_malloc_heap.h:39:26: fatal error: rte_spinlock.h: No such file or directory Looking in the include/ directory in my build directory, I see a lot of invalid symlinks to things like rte_spinlock.h: lrwxrwxrwx 1 ska users 99 May 4 14:33 rte_spinlock.h -> ../[...]/dpdk/lib/librte_eal/common/include/arch/x86/rte_spinlock.h and I believe that this is caused by my use of multiple mounts: I have my home directory and the source code on NFS, while the build directory is on local disk (under a build/-symlink from my home directory). So using ../ relative to the build directory will actually end up on the local disk, and not work. I guess it should be possible to workaround this by building somewhere else, but perhaps there is some patch to be made in mk/? Any pointer to where to look? // Simon
[dpdk-dev] GitHub sandbox for the DPDK community
On 5/2/2015 1:33 AM, Matthew Hall wrote: > On Fri, May 01, 2015 at 12:45:12PM -0400, Neil Horman wrote: >> Yes, but as you said above, using a web browser doesn't make reviewing >> patches >> faster. In fact, I would assert that it slows the process down, as it >> prevents >> quick, easy command line access to patch review (as you have with a properly >> configured MUA). That seems like we're going in the opposite direction of at >> least one problem we would like to solve. > Normally I'm a big command-line supporter. However I have found reviewing > patches by email for me is about the most painful workflow. What mail client do you use? I think mail client supporting thread mode is important for patch review. Thanks, Michael > > The emails are pages and pages. > > The replies from commenters are buried in the walls of text. > > Replies to replies keep shifting farther off the edge of the screen. The code > gets weirder and weirder to try to read. > > Quickly reading over the patchset by scrolling through to get the flavor of > it, to see if I'm qualified to review it, and look at the parts I actually > know about is much harder. > > I can go to one place to see every candidate patchset out there, the GH Pull > Request page. Then I can just sync up the branch and test it on my own > systems > to see if it works, not just try to read it. > > Github automatically minimizes old comments that are already fixed, so they > don't keep consuming space and mental bandwidth from the review. > > All in all, I'd be able to review more DPDK patches faster with the GH > interface than having them in the mailing list. > > Matthew. >
[dpdk-dev] build issue with out-of-tree builds and multiple mounts
Hi Simon, On 05/04/2015 02:42 PM, Simon K?gstr?m wrote: > Hi! > > I'm trying to do a out-of-tree build of DPDK 2.0.0 (with make -C and > O=), but failing with errors such as > > In file included from > [...]/lib/librte_eal/common/include/rte_eal_memconfig.h:40:0, > [...] > rte_malloc_heap.h:39:26: fatal error: rte_spinlock.h: No such file or > directory > > Looking in the include/ directory in my build directory, I see a lot of > invalid symlinks to things like rte_spinlock.h: > > lrwxrwxrwx 1 ska users 99 May 4 14:33 rte_spinlock.h -> > ../[...]/dpdk/lib/librte_eal/common/include/arch/x86/rte_spinlock.h Can you please send the full make command line that produces this issue? Thanks, Olivier > > and I believe that this is caused by my use of multiple mounts: I have > my home directory and the source code on NFS, while the build directory > is on local disk (under a build/-symlink from my home directory). So > using ../ relative to the build directory will actually end up on the > local disk, and not work. > > > I guess it should be possible to workaround this by building somewhere > else, but perhaps there is some patch to be made in mk/? Any pointer to > where to look? > > // Simon >
[dpdk-dev] build issue with out-of-tree builds and multiple mounts
On 2015-05-04 14:48, Olivier MATZ wrote: > Hi Simon, > > On 05/04/2015 02:42 PM, Simon K?gstr?m wrote: >> Hi! >> >> I'm trying to do a out-of-tree build of DPDK 2.0.0 (with make -C and >> O=), but failing with errors such as >> >> In file included from >> [...]/lib/librte_eal/common/include/rte_eal_memconfig.h:40:0, >> [...] >> rte_malloc_heap.h:39:26: fatal error: rte_spinlock.h: No such file or >> directory >> >> Looking in the include/ directory in my build directory, I see a lot of >> invalid symlinks to things like rte_spinlock.h: >> >> lrwxrwxrwx 1 ska users 99 May 4 14:33 rte_spinlock.h -> >> ../[...]/dpdk/lib/librte_eal/common/include/arch/x86/rte_spinlock.h > > Can you please send the full make command line that produces this > issue? make -C /home/ska/devel/dpdk EXTRA_CFLAGS="-g -O0" RTE_KERNELDIR=/tmp/ubuntu-14.04/usr/src/linux-headers-3.13.0-51-generic O=/home/ska/build/dpdk T=x86_64-default-linuxapp-gcc V=1 The config step works fine, but it fails on this step. /home/ska is mounted on NFS, and /home/ska/build is a symlink to /lhome/ska/build It appears to work if I build outside the symlink, i.e.., build with O=/lhome/ska/build/... instead. // Simon
[dpdk-dev] [RFC PATCH 0/4 v2] Extending DPDK with multiple device support
On 13/04/15 21:44, Keith Wiles wrote: > Hi All, > > Here is a set of RFC patches to update DPDK to support a generic set of > devices. The patches that follow create two files eal_common_device.h > and rte_common_device.c and then a few items in rte_ethdev.[ch] were moved > to cleanup those files by moving the device generic values and functions > into a common set of device generic files. (Old thread. But in the context of some private pktdev discussions, I was asked to give some feedback on this specific version) > > In the rte_common_device.h file are a couple of macros to abstract a few > common items from rte_ethdev.h which are not ethernet specific. These > generic device specific structure members are place into macros to be > placed at the top of each new device type crypto, hardware offload, dpi, > compression and possible others to be defined later. In version 2 I used > nested structures a bit cleaner from the macro design, but modified a lot > more files. I don't see easy and that really pays off to have a common rte_dev_data structure. So far the common data, as far as I can see, is name and _maybe_ MTU. The queues and number of queues is something difficult to abstract (e.g. KNI or rte_ring, 1 TX and 1 RX queue?), although it can be done. > > Most of the changes are to update the code to locate the new location of > the members in the nested structures. To not try and rewrite code I > used macros to help hide the changes, but these constructs are now used > in a lot of files withint DPDK. > > I did not pull the Rx/Tx Routines into a common function, but it could be > done if everyone agrees. It does not mean every device will use a common > Rx/Tx routine. Without pulling Rx/Tx routines into a common file allows > the device writer to have similar or different set of routines. Actually, to me this is the part that makes more sense to abstract, because all the DPDK applications that have to deal with RX/TX over heterogeneous devices have to construct the same de-multiplexing logic (create an abstraction of a generic port on top of rte libraries) that we could provide. I think this patch set addresses more of a fundamental re-organization of data structures to maximize reusage than this "higher level" abstraction. I am commenting some stuff inline, to the point that I could. > > These patches do not contain any new devices as these are being work on > today. I could have included the DPI (dpidev) routines we did in the PoC, > but the DPI is not open source. > > More cleanup of ethdev could be done to remove NIC specific features not > supported in all devices and someone needs to do that cleanup IMO. > > The code is untested and I wanted to get something our for others to poke > at today and more could be pulled out of ethdev as well. I/We will be > looking at testing the code as we get more completed. > > I have not finished up the crypto APIs yet, but I am planning to work on > those additions today. The crypto code we are using is the Quick Assist > code found on 01.org, but we need to update the code to be move DPDK > friendly. > > The QAT code does have a modified like API similar to Linux Kernel crypto > API and I want to review that code first. Seeing the balance of additions/deletions doesn't look like much of a gain in terms of number of lines too. Marc > > Regards, > ++Keith > > > Keith Wiles (4): >Adding the common device files for multiple device support >Add the ethdev changes for multiple device support >Add the test file changes for common device support >Update PMD files for new common device support > > app/test-pmd/config.c | 6 +- > app/test-pmd/testpmd.h| 4 +- > app/test/test_kni.c | 12 +- > app/test/test_link_bonding.c | 24 +- > app/test/virtual_pmd.c| 106 +-- > examples/link_status_interrupt/main.c | 6 +- > lib/librte_eal/bsdapp/eal/Makefile| 1 + > lib/librte_eal/common/Makefile| 1 + > lib/librte_eal/common/eal_common_device.c | 185 + > lib/librte_eal/common/include/rte_common_device.h | 674 +++ > lib/librte_eal/common/include/rte_log.h | 1 + > lib/librte_eal/linuxapp/eal/Makefile | 1 + > lib/librte_ether/rte_ethdev.c | 944 > +- > lib/librte_ether/rte_ethdev.h | 340 ++-- > lib/librte_kni/rte_kni.c | 4 +- > lib/librte_pmd_af_packet/rte_eth_af_packet.c | 38 +- > lib/librte_pmd_bond/rte_eth_bond_8023ad.c | 18 +- > lib/librte_pmd_bond/rte_eth_bond_alb.c| 10 +- > lib/librte_pmd_bond/rte_eth_bond_api.c| 142 ++-- > lib/librte_pmd_bond/rte_eth_bond_args.c | 2 +- > lib/librte_pmd_bond/rte_eth_bond_pmd.c| 164
[dpdk-dev] [RFC PATCH 1/4 v2] Adding the common device files for multiple device support
On 13/04/15 21:44, Keith Wiles wrote:
> Add the eal_common_device.c and rte_common_device.h and include the
> build support changes.
>
> Signed-off-by: Keith Wiles
> ---
> lib/librte_eal/bsdapp/eal/Makefile| 1 +
> lib/librte_eal/common/Makefile| 1 +
> lib/librte_eal/common/eal_common_device.c | 185 ++
> lib/librte_eal/common/include/rte_common_device.h | 674
> ++
> lib/librte_eal/common/include/rte_log.h | 1 +
> lib/librte_eal/linuxapp/eal/Makefile | 1 +
> lib/librte_kni/rte_kni.c | 4 +-
> 7 files changed, 865 insertions(+), 2 deletions(-)
> create mode 100644 lib/librte_eal/common/eal_common_device.c
> create mode 100644 lib/librte_eal/common/include/rte_common_device.h
>
> diff --git a/lib/librte_eal/bsdapp/eal/Makefile
> b/lib/librte_eal/bsdapp/eal/Makefile
> index 2357cfa..7bb2689 100644
> --- a/lib/librte_eal/bsdapp/eal/Makefile
> +++ b/lib/librte_eal/bsdapp/eal/Makefile
> @@ -78,6 +78,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_devargs.c
> SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_dev.c
> SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_options.c
> SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_thread.c
> +SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_device.c
>
> CFLAGS_eal.o := -D_GNU_SOURCE
> #CFLAGS_eal_thread.o := -D_GNU_SOURCE
> diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
> index 3ea3bbf..c4bf805 100644
> --- a/lib/librte_eal/common/Makefile
> +++ b/lib/librte_eal/common/Makefile
> @@ -40,6 +40,7 @@ INC += rte_string_fns.h rte_version.h
> INC += rte_eal_memconfig.h rte_malloc_heap.h
> INC += rte_hexdump.h rte_devargs.h rte_dev.h
> INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
> +INC += rte_common_device.h
>
> ifeq ($(CONFIG_RTE_INSECURE_FUNCTION_WARNING),y)
> INC += rte_warnings.h
> diff --git a/lib/librte_eal/common/eal_common_device.c
> b/lib/librte_eal/common/eal_common_device.c
> new file mode 100644
> index 000..a9ef925
> --- /dev/null
> +++ b/lib/librte_eal/common/eal_common_device.c
> @@ -0,0 +1,185 @@
> +/*-
> + * BSD LICENSE
> + *
> + * Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + * Copyright(c) 2014 6WIND S.A.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in
> + * the documentation and/or other materials provided with the
> + * distribution.
> + * * Neither the name of Intel Corporation nor the names of its
> + * contributors may be used to endorse or promote products derived
> + * from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "rte_common_device.h"
> +
> +void *
> +rte_dev_add_callback(struct rte_dev_rxtx_callback ** cbp,
> + void * fn, void *user_param)
> +{
> + struct rte_dev_rxtx_callback *cb;
> +
> + cb = rte_zmalloc(NULL, sizeof(*cb), 0);
> +
> + if (cb == NULL) {
> + rte_errno = ENOMEM;
> + return NULL;
> + }
> +
> + cb->fn.vp = fn;
> + cb->param = user_param;
> + cb->next = *cbp;
> + *cbp = cb;
> + return cb;
> +}
> +
> +int
> +rte_dev_remove_callback(struct rte_dev_rxtx_callback ** cbp,
> + struct rte_dev_rxtx_callback *user_cb)
> +{
> + struct rte_dev_rxtx_callback *cb = *cbp;
> + struct rte_dev_rxtx_callback *prev_cb;
> +
> + /* Reset head pointer and remove user cb if first in the list. */
> + if
[dpdk-dev] [PATCH v6 1/2] Simplify the ifdefs in rte.app.mk.
On 5/4/15, 2:18 AM, "Olivier MATZ" wrote: >Hi Keith, > >On 05/01/2015 04:22 PM, Keith Wiles wrote: >> Trying to simplify the ifdefs in rte.app.mk to make the code >> more readable and maintainable by moving LDLIBS variable to use >> the same style as LDLIBS-y being used in the rest of the code. >> >> Added a new variable called EXTRA_LDLIBS to be used by example apps >> instead of using LDLIBS directly. The new internal variable _LDLIBS >> should not be used outside of the rte.app.mk file. The makefiles >> can still use LDLIBS, but I would suggest using EXTRA_LDLIBS instead. > >Why are you suggesting to change LIBS to EXTRA_LIBS? Hi Olivier, I do not change LIBS to EXTRA_LIBS as I did not touch those variables. I did add EXTRA_LDLIBS and let LDLIBS as it was in the patch. I also created LDLIBS-y as an internal variable. Did I miss your point here? ++Keith >We discussed in a previous thread that EXTRA_* variables should >(as much as possible) be kept empty in Makefiles as it allows a >user to append things in them. > >By the way, it would be easier to follow the different versions >of your patches if you add "--in-reply-to " in your >git-send-email command, as described in http://dpdk.org/dev > >Regards, >Olivier > > >> >> Signed-off-by: Keith Wiles >> --- >> examples/dpdk_qat/Makefile | 4 +- >> examples/vm_power_manager/Makefile | 2 +- >> mk/rte.app.mk | 242 >>+ >> 3 files changed, 63 insertions(+), 185 deletions(-) >> >> diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile >> index f1e06a1..90ca1d3 100644 >> --- a/examples/dpdk_qat/Makefile >> +++ b/examples/dpdk_qat/Makefile >> @@ -77,8 +77,8 @@ else >> ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a >> endif >> >> -LDLIBS += -L$(ICP_ROOT)/build >> -LDLIBS += $(ICP_LIBRARY_PATH) \ >> +EXTRA_LDLIBS += -L$(ICP_ROOT)/build >> +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \ >> -lz \ >> -losal \ >> -ladf_proxy \ >> diff --git a/examples/vm_power_manager/Makefile >>b/examples/vm_power_manager/Makefile >> index 113dbc4..8fb78d4 100644 >> --- a/examples/vm_power_manager/Makefile >> +++ b/examples/vm_power_manager/Makefile >> @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c >> CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/ >> CFLAGS += $(WERROR_FLAGS) >> >> -LDLIBS += -lvirt >> +EXTRA_LDLIBS += -lvirt >> >> # workaround for a gcc bug with noreturn attribute >> # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 >> diff --git a/mk/rte.app.mk b/mk/rte.app.mk >> index 62a76ae..b8030d2 100644 >> --- a/mk/rte.app.mk >> +++ b/mk/rte.app.mk >> @@ -1,7 +1,7 @@ >> # BSD LICENSE >> # >> -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. >> -# Copyright(c) 2014 6WIND S.A. >> +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. >> +# Copyright(c) 2014-2015 6WIND S.A. >> # All rights reserved. >> # >> # Redistribution and use in source and binary forms, with or without >> @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT) >> endif >> >> # default path for libs >> -LDLIBS += -L$(RTE_SDK_BIN)/lib >> +_LDLIBS-y += -L$(RTE_SDK_BIN)/lib >> >> # >> # Include libraries depending on config if NO_AUTOLIBS is not set >> @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib >> # >> ifeq ($(NO_AUTOLIBS),) >> >> -LDLIBS += --whole-archive >> +_LDLIBS-y += --whole-archive >> >> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y) >> -LDLIBS += -l$(RTE_LIBNAME) >> -endif >> +_LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS)+= -l$(RTE_LIBNAME) >> >> ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) >> >> -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) >> -LDLIBS += -lrte_distributor >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y) >> -LDLIBS += -lrte_reorder >> -endif >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)+= -lrte_distributor >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)+= -lrte_reorder >> >> -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) >> ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >> -LDLIBS += -lrte_kni >> -endif >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_KNI)+= -lrte_kni >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM)+= -lrte_ivshmem >> endif >> >> -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y) >> -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >> -LDLIBS += -lrte_ivshmem >> -endif >> -endif >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE) += -lrte_pipeline >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE) += -lrte_table >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PORT) += -lrte_port >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_LPM)+= -lrte_lpm >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)+= -lrte_acl >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_
[dpdk-dev] [RFC PATCH 1/4 v2] Adding the common device files for multiple device support
Hi Marc
On 5/4/15, 6:13 AM, "Marc Sune" wrote:
>
>
>On 13/04/15 21:44, Keith Wiles wrote:
>> Add the eal_common_device.c and rte_common_device.h and include the
>> build support changes.
>>
>> Signed-off-by: Keith Wiles
>> ---
>> lib/librte_eal/bsdapp/eal/Makefile| 1 +
>> lib/librte_eal/common/Makefile| 1 +
>> lib/librte_eal/common/eal_common_device.c | 185 ++
>> lib/librte_eal/common/include/rte_common_device.h | 674
>>++
>> lib/librte_eal/common/include/rte_log.h | 1 +
>> lib/librte_eal/linuxapp/eal/Makefile | 1 +
>> lib/librte_kni/rte_kni.c | 4 +-
>> 7 files changed, 865 insertions(+), 2 deletions(-)
>> create mode 100644 lib/librte_eal/common/eal_common_device.c
>> create mode 100644 lib/librte_eal/common/include/rte_common_device.h
>>
>> diff --git a/lib/librte_eal/bsdapp/eal/Makefile
>>b/lib/librte_eal/bsdapp/eal/Makefile
>> index 2357cfa..7bb2689 100644
>> --- a/lib/librte_eal/bsdapp/eal/Makefile
>> +++ b/lib/librte_eal/bsdapp/eal/Makefile
>> @@ -78,6 +78,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) +=
>>eal_common_devargs.c
>> SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_dev.c
>> SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_options.c
>> SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_thread.c
>> +SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_device.c
>>
>> CFLAGS_eal.o := -D_GNU_SOURCE
>> #CFLAGS_eal_thread.o := -D_GNU_SOURCE
>> diff --git a/lib/librte_eal/common/Makefile
>>b/lib/librte_eal/common/Makefile
>> index 3ea3bbf..c4bf805 100644
>> --- a/lib/librte_eal/common/Makefile
>> +++ b/lib/librte_eal/common/Makefile
>> @@ -40,6 +40,7 @@ INC += rte_string_fns.h rte_version.h
>> INC += rte_eal_memconfig.h rte_malloc_heap.h
>> INC += rte_hexdump.h rte_devargs.h rte_dev.h
>> INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
>> +INC += rte_common_device.h
>>
>> ifeq ($(CONFIG_RTE_INSECURE_FUNCTION_WARNING),y)
>> INC += rte_warnings.h
>> diff --git a/lib/librte_eal/common/eal_common_device.c
>>b/lib/librte_eal/common/eal_common_device.c
>> new file mode 100644
>> index 000..a9ef925
>> --- /dev/null
>> +++ b/lib/librte_eal/common/eal_common_device.c
>> @@ -0,0 +1,185 @@
>> +/*-
>> + * BSD LICENSE
>> + *
>> + * Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
>> + * Copyright(c) 2014 6WIND S.A.
>> + * All rights reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + *
>> + * * Redistributions of source code must retain the above copyright
>> + * notice, this list of conditions and the following disclaimer.
>> + * * Redistributions in binary form must reproduce the above
>>copyright
>> + * notice, this list of conditions and the following disclaimer
>>in
>> + * the documentation and/or other materials provided with the
>> + * distribution.
>> + * * Neither the name of Intel Corporation nor the names of its
>> + * contributors may be used to endorse or promote products
>>derived
>> + * from this software without specific prior written permission.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
>>CONTRIBUTORS
>> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
>>FOR
>> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
>>COPYRIGHT
>> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
>>INCIDENTAL,
>> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
>>USE,
>> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
>>ANY
>> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
>>TORT
>> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
>>USE
>> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
>>DAMAGE.
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include "rte_common_device.h"
>> +
>> +void *
>> +rte_dev_add_callback(struct rte_dev_rxtx_callback ** cbp,
>> +void * fn, void *user_param)
>> +{
>> +struct rte_dev_rxtx_callback *cb;
>> +
>> +cb = rte_zmalloc(NULL, sizeof(*cb), 0);
>> +
>> +if (cb == NULL) {
>> +rte_errno = ENOMEM;
>> +return NULL;
>> +}
>> +
>> +cb->fn.vp = fn;
>> +cb->param = user_param;
>> +cb->next = *cbp;
>> +*cbp = cb;
>> +return cb;
>> +}
>> +
>> +int
>> +rte_dev_remove_callback(struct rte_dev_rxtx_callback ** cbp,
>> +struct rte_dev_rxtx_callback *user_cb)
>> +{
[dpdk-dev] Compiling files with .S with GCC
On 5/4/15, 1:37 AM, "Olivier MATZ" wrote: >Hello, > >On 04/26/2015 06:55 PM, Wiles, Keith wrote: >> >> >> On 4/26/15, 11:53 AM, "Wiles, Keith" wrote: >> >>> Hi All, >>> >>> I noticed in my builds with foo.S file I would get a warning from the >>> compiler the foo_s.o.tmp linker file will not be used as the code is >>>not >>> linked. A strange error and the foo_s.o file would not be created. In >>>the >>> mk/internal/rte.compile-pre.mk we have the following >>> >>> # command to assemble a .S file to generate an object >>> ifeq ($(USE_HOST),1) >>> S_TO_O = $(CPP) $(HOST_CPPFLAGS) $($(@)_CPPFLAGS) >>>$(HOST_EXTRA_CPPFLAGS) >>> $< $(@).tmp && \ >>>$(HOSTAS) $(HOST_ASFLAGS) $($(@)_ASFLAGS) $(HOST_EXTRA_ASFLAGS) -o >>>$@ >>> $(@).tmp >>> S_TO_O_STR = $(subst ','\'',$(S_TO_O)) #'# fix syntax highlight >>> S_TO_O_DISP = $(if $(V),"$(S_TO_O_STR)"," HOSTAS $(@)") >>> else >>> S_TO_O = $(CPP) $(CPPFLAGS) $($(@)_CPPFLAGS) $(EXTRA_CPPFLAGS) $< -o >>> $(@).tmp && \ >>>$(AS) $(ASFLAGS) $($(@)_ASFLAGS) $(EXTRA_ASFLAGS) -o $@ $(@).tmp >>> S_TO_O_STR = $(subst ','\'',$(S_TO_O)) #'# fix syntax highlight >>> S_TO_O_DISP = $(if $(V),"$(S_TO_O_STR)"," AS $(@)") >>> endif >>> >>> I had to change the above ?.tmp? strings to ?.s? to remove this warning >>> and get the foo_s.o file created. Using the .s the file name becomes >>> foo_s.o.s >> >> Compiler used on Ubuntu 14.04 'gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2' >> >>> >>> Did not try this with clang or any other compiler, but I expect we can >>> safely change the ?.tmp? to ?.s? IMO >>> >>> Anyone else notice this problem? > >I tested a similar use-case and I don't reproduce the issue. >I don't don't understand why replacing $(@).tmp by $(@).s would >solve it. In the case I saw the compiler seemed to want the .s to under stand it was not some other type of file. Not sure why the error occurred in my external build. We can leave it using .tmp if you like, but I feel .s would have been the correct name for the file. If you think it needs to be changed to .s then you can submit a patch or I can or just leave it along your option. Regards, ++Keith > >The file $(@).tmp is used as a temporary file to store the preprocessed >version of the $(@).s file. I agree that using the .s extension is not >a bad choice (see below), but to me it is not the proper solution to >your problem. > >From https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html > > file.s > Assembler code. > file.S > file.sx > Assembler code that must be preprocessed. > >Regards, >Olivier
[dpdk-dev] [PATCH v6 1/2] Simplify the ifdefs in rte.app.mk.
Sent from my iPhone > On May 4, 2015, at 2:19 AM, Olivier MATZ wrote: > > Hi Keith, > >> On 05/01/2015 04:22 PM, Keith Wiles wrote: >> Trying to simplify the ifdefs in rte.app.mk to make the code >> more readable and maintainable by moving LDLIBS variable to use >> the same style as LDLIBS-y being used in the rest of the code. >> >> Added a new variable called EXTRA_LDLIBS to be used by example apps >> instead of using LDLIBS directly. The new internal variable _LDLIBS >> should not be used outside of the rte.app.mk file. The makefiles >> can still use LDLIBS, but I would suggest using EXTRA_LDLIBS instead. > > Why are you suggesting to change LIBS to EXTRA_LIBS? > We discussed in a previous thread that EXTRA_* variables should > (as much as possible) be kept empty in Makefiles as it allows a > user to append things in them. > > By the way, it would be easier to follow the different versions > of your patches if you add "--in-reply-to " in your > git-send-email command, as described in http://dpdk.org/dev One more reason email handling is not working too many details and on github this would not have happened. Regards ++Keith > > Regards, > Olivier > > >> >> Signed-off-by: Keith Wiles >> --- >> examples/dpdk_qat/Makefile | 4 +- >> examples/vm_power_manager/Makefile | 2 +- >> mk/rte.app.mk | 242 >> + >> 3 files changed, 63 insertions(+), 185 deletions(-) >> >> diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile >> index f1e06a1..90ca1d3 100644 >> --- a/examples/dpdk_qat/Makefile >> +++ b/examples/dpdk_qat/Makefile >> @@ -77,8 +77,8 @@ else >> ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a >> endif >> >> -LDLIBS += -L$(ICP_ROOT)/build >> -LDLIBS += $(ICP_LIBRARY_PATH) \ >> +EXTRA_LDLIBS += -L$(ICP_ROOT)/build >> +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \ >> -lz \ >> -losal \ >> -ladf_proxy \ >> diff --git a/examples/vm_power_manager/Makefile >> b/examples/vm_power_manager/Makefile >> index 113dbc4..8fb78d4 100644 >> --- a/examples/vm_power_manager/Makefile >> +++ b/examples/vm_power_manager/Makefile >> @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c >> CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/ >> CFLAGS += $(WERROR_FLAGS) >> >> -LDLIBS += -lvirt >> +EXTRA_LDLIBS += -lvirt >> >> # workaround for a gcc bug with noreturn attribute >> # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 >> diff --git a/mk/rte.app.mk b/mk/rte.app.mk >> index 62a76ae..b8030d2 100644 >> --- a/mk/rte.app.mk >> +++ b/mk/rte.app.mk >> @@ -1,7 +1,7 @@ >> # BSD LICENSE >> # >> -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. >> -# Copyright(c) 2014 6WIND S.A. >> +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. >> +# Copyright(c) 2014-2015 6WIND S.A. >> # All rights reserved. >> # >> # Redistribution and use in source and binary forms, with or without >> @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT) >> endif >> >> # default path for libs >> -LDLIBS += -L$(RTE_SDK_BIN)/lib >> +_LDLIBS-y += -L$(RTE_SDK_BIN)/lib >> >> # >> # Include libraries depending on config if NO_AUTOLIBS is not set >> @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib >> # >> ifeq ($(NO_AUTOLIBS),) >> >> -LDLIBS += --whole-archive >> +_LDLIBS-y += --whole-archive >> >> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y) >> -LDLIBS += -l$(RTE_LIBNAME) >> -endif >> +_LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS)+= -l$(RTE_LIBNAME) >> >> ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) >> >> -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) >> -LDLIBS += -lrte_distributor >> -endif >> - >> -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y) >> -LDLIBS += -lrte_reorder >> -endif >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)+= -lrte_distributor >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)+= -lrte_reorder >> >> -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) >> ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >> -LDLIBS += -lrte_kni >> -endif >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_KNI)+= -lrte_kni >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM)+= -lrte_ivshmem >> endif >> >> -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y) >> -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >> -LDLIBS += -lrte_ivshmem >> -endif >> -endif >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE) += -lrte_pipeline >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE) += -lrte_table >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PORT) += -lrte_port >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_LPM)+= -lrte_lpm >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)+= -lrte_acl >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter >> >> -ifeq ($(CONFIG_RTE_LIBRTE_PIPELINE),y) >> -LDLIBS += -lrte_pipeline >> -endif
[dpdk-dev] [PATCH v6 1/2] Simplify the ifdefs in rte.app.mk.
On 05/04/2015 04:36 PM, Wiles, Keith wrote: > > > On 5/4/15, 2:18 AM, "Olivier MATZ" wrote: > >> Hi Keith, >> >> On 05/01/2015 04:22 PM, Keith Wiles wrote: >>> Trying to simplify the ifdefs in rte.app.mk to make the code >>> more readable and maintainable by moving LDLIBS variable to use >>> the same style as LDLIBS-y being used in the rest of the code. >>> >>> Added a new variable called EXTRA_LDLIBS to be used by example apps >>> instead of using LDLIBS directly. The new internal variable _LDLIBS >>> should not be used outside of the rte.app.mk file. The makefiles >>> can still use LDLIBS, but I would suggest using EXTRA_LDLIBS instead. >> >> Why are you suggesting to change LIBS to EXTRA_LIBS? > > Hi Olivier, > > I do not change LIBS to EXTRA_LIBS as I did not touch those variables. > > I did add EXTRA_LDLIBS and let LDLIBS as it was in the patch. I also > created LDLIBS-y as an internal variable. Did I miss your point here? In your previous mail, you say "The makefiles can still use LDLIBS, but I would suggest using EXTRA_LDLIBS instead." The question is: why are you suggesting that? And in the patch you are submitting, you are replacing LDLIBS by EXTRA_LDLIBS in examples/dpdk_qat/Makefile and examples/vm_power_manager/Makefile. Regards, Olivier > > ++Keith > >> We discussed in a previous thread that EXTRA_* variables should >> (as much as possible) be kept empty in Makefiles as it allows a >> user to append things in them. >> >> By the way, it would be easier to follow the different versions >> of your patches if you add "--in-reply-to " in your >> git-send-email command, as described in http://dpdk.org/dev >> >> Regards, >> Olivier >> >> >>> >>> Signed-off-by: Keith Wiles >>> --- >>> examples/dpdk_qat/Makefile | 4 +- >>> examples/vm_power_manager/Makefile | 2 +- >>> mk/rte.app.mk | 242 >>> + >>> 3 files changed, 63 insertions(+), 185 deletions(-) >>> >>> diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile >>> index f1e06a1..90ca1d3 100644 >>> --- a/examples/dpdk_qat/Makefile >>> +++ b/examples/dpdk_qat/Makefile >>> @@ -77,8 +77,8 @@ else >>> ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a >>> endif >>> >>> -LDLIBS += -L$(ICP_ROOT)/build >>> -LDLIBS += $(ICP_LIBRARY_PATH) \ >>> +EXTRA_LDLIBS += -L$(ICP_ROOT)/build >>> +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \ >>> -lz \ >>> -losal \ >>> -ladf_proxy \ >>> diff --git a/examples/vm_power_manager/Makefile >>> b/examples/vm_power_manager/Makefile >>> index 113dbc4..8fb78d4 100644 >>> --- a/examples/vm_power_manager/Makefile >>> +++ b/examples/vm_power_manager/Makefile >>> @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c >>> CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/ >>> CFLAGS += $(WERROR_FLAGS) >>> >>> -LDLIBS += -lvirt >>> +EXTRA_LDLIBS += -lvirt >>> >>> # workaround for a gcc bug with noreturn attribute >>> # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 >>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk >>> index 62a76ae..b8030d2 100644 >>> --- a/mk/rte.app.mk >>> +++ b/mk/rte.app.mk >>> @@ -1,7 +1,7 @@ >>> # BSD LICENSE >>> # >>> -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. >>> -# Copyright(c) 2014 6WIND S.A. >>> +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. >>> +# Copyright(c) 2014-2015 6WIND S.A. >>> # All rights reserved. >>> # >>> # Redistribution and use in source and binary forms, with or without >>> @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT) >>> endif >>> >>> # default path for libs >>> -LDLIBS += -L$(RTE_SDK_BIN)/lib >>> +_LDLIBS-y += -L$(RTE_SDK_BIN)/lib >>> >>> # >>> # Include libraries depending on config if NO_AUTOLIBS is not set >>> @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib >>> # >>> ifeq ($(NO_AUTOLIBS),) >>> >>> -LDLIBS += --whole-archive >>> +_LDLIBS-y += --whole-archive >>> >>> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y) >>> -LDLIBS += -l$(RTE_LIBNAME) >>> -endif >>> +_LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS)+= -l$(RTE_LIBNAME) >>> >>> ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) >>> -LDLIBS += -lrte_distributor >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y) >>> -LDLIBS += -lrte_reorder >>> -endif >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)+= -lrte_distributor >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)+= -lrte_reorder >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) >>> ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >>> -LDLIBS += -lrte_kni >>> -endif >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_KNI)+= -lrte_kni >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM)+= -lrte_ivshmem >>> endif >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y) >>> -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >>> -LDLIBS += -lrte_ivshmem >>> -endif >>> -endif >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE) += -lrte_pipeline >>> +_LDLIBS-$(
[dpdk-dev] [PATCH] e1000: add missing 82583v pci id
From: James Davidson Add support for 82583V (E1000) PCI device id. Signed-off-by: James Davidson Signed-off-by: Stephen Hemminger --- lib/librte_eal/common/include/rte_pci_dev_ids.h | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/librte_eal/common/include/rte_pci_dev_ids.h b/lib/librte_eal/common/include/rte_pci_dev_ids.h index 21d2eed..b9b5d76 100644 --- a/lib/librte_eal/common/include/rte_pci_dev_ids.h +++ b/lib/librte_eal/common/include/rte_pci_dev_ids.h @@ -282,6 +282,7 @@ RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL, E1000_DEV_ID_82572EI) RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL, E1000_DEV_ID_82573L) RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL, E1000_DEV_ID_82574L) RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL, E1000_DEV_ID_82574LA) +RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL, E1000_DEV_ID_82583V) / Physical IGB devices from e1000_hw.h / -- 2.1.4
[dpdk-dev] [PATCH v6 1/2] Simplify the ifdefs in rte.app.mk.
On 05/04/2015 05:09 PM, Wiles, Keith wrote: > > > Sent from my iPhone > >> On May 4, 2015, at 2:19 AM, Olivier MATZ wrote: >> >> Hi Keith, >> >>> On 05/01/2015 04:22 PM, Keith Wiles wrote: >>> Trying to simplify the ifdefs in rte.app.mk to make the code >>> more readable and maintainable by moving LDLIBS variable to use >>> the same style as LDLIBS-y being used in the rest of the code. >>> >>> Added a new variable called EXTRA_LDLIBS to be used by example apps >>> instead of using LDLIBS directly. The new internal variable _LDLIBS >>> should not be used outside of the rte.app.mk file. The makefiles >>> can still use LDLIBS, but I would suggest using EXTRA_LDLIBS instead. >> >> Why are you suggesting to change LIBS to EXTRA_LIBS? >> We discussed in a previous thread that EXTRA_* variables should >> (as much as possible) be kept empty in Makefiles as it allows a >> user to append things in them. >> >> By the way, it would be easier to follow the different versions >> of your patches if you add "--in-reply-to " in your >> git-send-email command, as described in http://dpdk.org/dev > > One more reason email handling is not working too many details and on github > this would not have happened. Sorry but that's not the debate here. I'm just suggesting that you use --in-reply-to to help other people to review your patches. Thanks, Olivier > > Regards > ++Keith >> >> Regards, >> Olivier >> >> >>> >>> Signed-off-by: Keith Wiles >>> --- >>> examples/dpdk_qat/Makefile | 4 +- >>> examples/vm_power_manager/Makefile | 2 +- >>> mk/rte.app.mk | 242 >>> + >>> 3 files changed, 63 insertions(+), 185 deletions(-) >>> >>> diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile >>> index f1e06a1..90ca1d3 100644 >>> --- a/examples/dpdk_qat/Makefile >>> +++ b/examples/dpdk_qat/Makefile >>> @@ -77,8 +77,8 @@ else >>> ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a >>> endif >>> >>> -LDLIBS += -L$(ICP_ROOT)/build >>> -LDLIBS += $(ICP_LIBRARY_PATH) \ >>> +EXTRA_LDLIBS += -L$(ICP_ROOT)/build >>> +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \ >>> -lz \ >>> -losal \ >>> -ladf_proxy \ >>> diff --git a/examples/vm_power_manager/Makefile >>> b/examples/vm_power_manager/Makefile >>> index 113dbc4..8fb78d4 100644 >>> --- a/examples/vm_power_manager/Makefile >>> +++ b/examples/vm_power_manager/Makefile >>> @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c >>> CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/ >>> CFLAGS += $(WERROR_FLAGS) >>> >>> -LDLIBS += -lvirt >>> +EXTRA_LDLIBS += -lvirt >>> >>> # workaround for a gcc bug with noreturn attribute >>> # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 >>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk >>> index 62a76ae..b8030d2 100644 >>> --- a/mk/rte.app.mk >>> +++ b/mk/rte.app.mk >>> @@ -1,7 +1,7 @@ >>> # BSD LICENSE >>> # >>> -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. >>> -# Copyright(c) 2014 6WIND S.A. >>> +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. >>> +# Copyright(c) 2014-2015 6WIND S.A. >>> # All rights reserved. >>> # >>> # Redistribution and use in source and binary forms, with or without >>> @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT) >>> endif >>> >>> # default path for libs >>> -LDLIBS += -L$(RTE_SDK_BIN)/lib >>> +_LDLIBS-y += -L$(RTE_SDK_BIN)/lib >>> >>> # >>> # Include libraries depending on config if NO_AUTOLIBS is not set >>> @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib >>> # >>> ifeq ($(NO_AUTOLIBS),) >>> >>> -LDLIBS += --whole-archive >>> +_LDLIBS-y += --whole-archive >>> >>> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y) >>> -LDLIBS += -l$(RTE_LIBNAME) >>> -endif >>> +_LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS)+= -l$(RTE_LIBNAME) >>> >>> ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) >>> -LDLIBS += -lrte_distributor >>> -endif >>> - >>> -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y) >>> -LDLIBS += -lrte_reorder >>> -endif >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)+= -lrte_distributor >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)+= -lrte_reorder >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) >>> ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >>> -LDLIBS += -lrte_kni >>> -endif >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_KNI)+= -lrte_kni >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM)+= -lrte_ivshmem >>> endif >>> >>> -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y) >>> -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) >>> -LDLIBS += -lrte_ivshmem >>> -endif >>> -endif >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE) += -lrte_pipeline >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE) += -lrte_table >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PORT) += -lrte_port >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats >>> +_L
[dpdk-dev] Compiling files with .S with GCC
On 05/04/2015 04:49 PM, Wiles, Keith wrote: > > > On 5/4/15, 1:37 AM, "Olivier MATZ" wrote: > >> Hello, >> >> On 04/26/2015 06:55 PM, Wiles, Keith wrote: >>> >>> >>> On 4/26/15, 11:53 AM, "Wiles, Keith" wrote: >>> Hi All, I noticed in my builds with foo.S file I would get a warning from the compiler the foo_s.o.tmp linker file will not be used as the code is not linked. A strange error and the foo_s.o file would not be created. In the mk/internal/rte.compile-pre.mk we have the following # command to assemble a .S file to generate an object ifeq ($(USE_HOST),1) S_TO_O = $(CPP) $(HOST_CPPFLAGS) $($(@)_CPPFLAGS) $(HOST_EXTRA_CPPFLAGS) $< $(@).tmp && \ $(HOSTAS) $(HOST_ASFLAGS) $($(@)_ASFLAGS) $(HOST_EXTRA_ASFLAGS) -o $@ $(@).tmp S_TO_O_STR = $(subst ','\'',$(S_TO_O)) #'# fix syntax highlight S_TO_O_DISP = $(if $(V),"$(S_TO_O_STR)"," HOSTAS $(@)") else S_TO_O = $(CPP) $(CPPFLAGS) $($(@)_CPPFLAGS) $(EXTRA_CPPFLAGS) $< -o $(@).tmp && \ $(AS) $(ASFLAGS) $($(@)_ASFLAGS) $(EXTRA_ASFLAGS) -o $@ $(@).tmp S_TO_O_STR = $(subst ','\'',$(S_TO_O)) #'# fix syntax highlight S_TO_O_DISP = $(if $(V),"$(S_TO_O_STR)"," AS $(@)") endif I had to change the above ?.tmp? strings to ?.s? to remove this warning and get the foo_s.o file created. Using the .s the file name becomes foo_s.o.s >>> >>> Compiler used on Ubuntu 14.04 'gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2' >>> Did not try this with clang or any other compiler, but I expect we can safely change the ?.tmp? to ?.s? IMO Anyone else notice this problem? >> >> I tested a similar use-case and I don't reproduce the issue. >> I don't don't understand why replacing $(@).tmp by $(@).s would >> solve it. > > In the case I saw the compiler seemed to want the .s to under stand it was > not some other type of file. Not sure why the error occurred in my > external build. We can leave it using .tmp if you like, but I feel .s > would have been the correct name for the file. Can you send a log of the error you've seen? Can you reproduce it outside the dpdk framework by trying to assemble a file called foo.tmp? This would show that the problem really comes from the extension of the file. > > If you think it needs to be changed to .s then you can submit a patch or I > can or just leave it along your option. Until we really understand the cause of the issue, I don't think this should be changed. But I'm not opposed to change it if we have a good justification. Regards, Olivier > > Regards, > ++Keith >> >> The file $(@).tmp is used as a temporary file to store the preprocessed >> version of the $(@).s file. I agree that using the .s extension is not >> a bad choice (see below), but to me it is not the proper solution to >> your problem. >> >>From https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html >> >> file.s >> Assembler code. >> file.S >> file.sx >> Assembler code that must be preprocessed. >> >> Regards, >> Olivier >
[dpdk-dev] Compiling files with .S with GCC
On 5/4/15, 8:42 AM, "Olivier MATZ" wrote: > > >On 05/04/2015 04:49 PM, Wiles, Keith wrote: >> >> >> On 5/4/15, 1:37 AM, "Olivier MATZ" wrote: >> >>> Hello, >>> >>> On 04/26/2015 06:55 PM, Wiles, Keith wrote: On 4/26/15, 11:53 AM, "Wiles, Keith" wrote: > Hi All, > > I noticed in my builds with foo.S file I would get a warning from the > compiler the foo_s.o.tmp linker file will not be used as the code is > not > linked. A strange error and the foo_s.o file would not be created. In > the > mk/internal/rte.compile-pre.mk we have the following > > # command to assemble a .S file to generate an object > ifeq ($(USE_HOST),1) > S_TO_O = $(CPP) $(HOST_CPPFLAGS) $($(@)_CPPFLAGS) > $(HOST_EXTRA_CPPFLAGS) > $< $(@).tmp && \ >$(HOSTAS) $(HOST_ASFLAGS) $($(@)_ASFLAGS) $(HOST_EXTRA_ASFLAGS) -o > $@ > $(@).tmp > S_TO_O_STR = $(subst ','\'',$(S_TO_O)) #'# fix syntax highlight > S_TO_O_DISP = $(if $(V),"$(S_TO_O_STR)"," HOSTAS $(@)") > else > S_TO_O = $(CPP) $(CPPFLAGS) $($(@)_CPPFLAGS) $(EXTRA_CPPFLAGS) $< -o > $(@).tmp && \ >$(AS) $(ASFLAGS) $($(@)_ASFLAGS) $(EXTRA_ASFLAGS) -o $@ $(@).tmp > S_TO_O_STR = $(subst ','\'',$(S_TO_O)) #'# fix syntax highlight > S_TO_O_DISP = $(if $(V),"$(S_TO_O_STR)"," AS $(@)") > endif > > I had to change the above ?.tmp? strings to ?.s? to remove this >warning > and get the foo_s.o file created. Using the .s the file name becomes > foo_s.o.s Compiler used on Ubuntu 14.04 'gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2' > > Did not try this with clang or any other compiler, but I expect we >can > safely change the ?.tmp? to ?.s? IMO > > Anyone else notice this problem? >>> >>> I tested a similar use-case and I don't reproduce the issue. >>> I don't don't understand why replacing $(@).tmp by $(@).s would >>> solve it. >> >> In the case I saw the compiler seemed to want the .s to under stand it >>was >> not some other type of file. Not sure why the error occurred in my >> external build. We can leave it using .tmp if you like, but I feel .s >> would have been the correct name for the file. > >Can you send a log of the error you've seen? >Can you reproduce it outside the dpdk framework by trying to assemble >a file called foo.tmp? >This would show that the problem really comes from the extension of >the file. OK will try, but out of town this week. > > >> >> If you think it needs to be changed to .s then you can submit a patch >>or I >> can or just leave it along your option. > >Until we really understand the cause of the issue, I don't think this >should be changed. But I'm not opposed to change it if we have a good >justification. > > >Regards, >Olivier > > > >> >> Regards, >> ++Keith >>> >>> The file $(@).tmp is used as a temporary file to store the preprocessed >>> version of the $(@).s file. I agree that using the .s extension is not >>> a bad choice (see below), but to me it is not the proper solution to >>> your problem. >>> >>>From https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html >>> >>> file.s >>> Assembler code. >>> file.S >>> file.sx >>> Assembler code that must be preprocessed. >>> >>> Regards, >>> Olivier >>
[dpdk-dev] [PATCH v6 1/2] Simplify the ifdefs in rte.app.mk.
Sent from my iPhone > On May 4, 2015, at 8:27 AM, Olivier MATZ wrote: > > > >> On 05/04/2015 04:36 PM, Wiles, Keith wrote: >> >> >>> On 5/4/15, 2:18 AM, "Olivier MATZ" wrote: >>> >>> Hi Keith, >>> On 05/01/2015 04:22 PM, Keith Wiles wrote: Trying to simplify the ifdefs in rte.app.mk to make the code more readable and maintainable by moving LDLIBS variable to use the same style as LDLIBS-y being used in the rest of the code. Added a new variable called EXTRA_LDLIBS to be used by example apps instead of using LDLIBS directly. The new internal variable _LDLIBS should not be used outside of the rte.app.mk file. The makefiles can still use LDLIBS, but I would suggest using EXTRA_LDLIBS instead. >>> >>> Why are you suggesting to change LIBS to EXTRA_LIBS? >> >> Hi Olivier, >> >> I do not change LIBS to EXTRA_LIBS as I did not touch those variables. >> >> I did add EXTRA_LDLIBS and let LDLIBS as it was in the patch. I also >> created LDLIBS-y as an internal variable. Did I miss your point here? > > In your previous mail, you say "The makefiles can still use LDLIBS, > but I would suggest using EXTRA_LDLIBS instead." > > The question is: why are you suggesting that? > > And in the patch you are submitting, you are replacing LDLIBS > by EXTRA_LDLIBS in examples/dpdk_qat/Makefile and > examples/vm_power_manager/Makefile. > I thought use the extra variable was the right way in those make files. Could have left them using LDLIBS but does it make any difference? > Regards, > Olivier > > > >> >> ++Keith >> >>> We discussed in a previous thread that EXTRA_* variables should >>> (as much as possible) be kept empty in Makefiles as it allows a >>> user to append things in them. >>> >>> By the way, it would be easier to follow the different versions >>> of your patches if you add "--in-reply-to " in your >>> git-send-email command, as described in http://dpdk.org/dev >>> >>> Regards, >>> Olivier >>> >>> Signed-off-by: Keith Wiles --- examples/dpdk_qat/Makefile | 4 +- examples/vm_power_manager/Makefile | 2 +- mk/rte.app.mk | 242 + 3 files changed, 63 insertions(+), 185 deletions(-) diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile index f1e06a1..90ca1d3 100644 --- a/examples/dpdk_qat/Makefile +++ b/examples/dpdk_qat/Makefile @@ -77,8 +77,8 @@ else ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a endif -LDLIBS += -L$(ICP_ROOT)/build -LDLIBS += $(ICP_LIBRARY_PATH) \ +EXTRA_LDLIBS += -L$(ICP_ROOT)/build +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \ -lz \ -losal \ -ladf_proxy \ diff --git a/examples/vm_power_manager/Makefile b/examples/vm_power_manager/Makefile index 113dbc4..8fb78d4 100644 --- a/examples/vm_power_manager/Makefile +++ b/examples/vm_power_manager/Makefile @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/ CFLAGS += $(WERROR_FLAGS) -LDLIBS += -lvirt +EXTRA_LDLIBS += -lvirt # workaround for a gcc bug with noreturn attribute # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 diff --git a/mk/rte.app.mk b/mk/rte.app.mk index 62a76ae..b8030d2 100644 --- a/mk/rte.app.mk +++ b/mk/rte.app.mk @@ -1,7 +1,7 @@ # BSD LICENSE # -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. -# Copyright(c) 2014 6WIND S.A. +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. +# Copyright(c) 2014-2015 6WIND S.A. # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT) endif # default path for libs -LDLIBS += -L$(RTE_SDK_BIN)/lib +_LDLIBS-y += -L$(RTE_SDK_BIN)/lib # # Include libraries depending on config if NO_AUTOLIBS is not set @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib # ifeq ($(NO_AUTOLIBS),) -LDLIBS += --whole-archive +_LDLIBS-y += --whole-archive -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y) -LDLIBS += -l$(RTE_LIBNAME) -endif +_LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS)+= -l$(RTE_LIBNAME) ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) -LDLIBS += -lrte_distributor -endif - -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y) -LDLIBS += -lrte_reorder -endif +_LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)+= -lrte_distributor +_LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)+= -lrte_reorder -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) -LDLIBS += -lrte_kni -endif +_
[dpdk-dev] [PATCH v6 1/2] Simplify the ifdefs in rte.app.mk.
Sent from my iPhone > On May 4, 2015, at 8:34 AM, Olivier MATZ wrote: > > > >> On 05/04/2015 05:09 PM, Wiles, Keith wrote: >> >> >> Sent from my iPhone >> >>> On May 4, 2015, at 2:19 AM, Olivier MATZ wrote: >>> >>> Hi Keith, >>> On 05/01/2015 04:22 PM, Keith Wiles wrote: Trying to simplify the ifdefs in rte.app.mk to make the code more readable and maintainable by moving LDLIBS variable to use the same style as LDLIBS-y being used in the rest of the code. Added a new variable called EXTRA_LDLIBS to be used by example apps instead of using LDLIBS directly. The new internal variable _LDLIBS should not be used outside of the rte.app.mk file. The makefiles can still use LDLIBS, but I would suggest using EXTRA_LDLIBS instead. >>> >>> Why are you suggesting to change LIBS to EXTRA_LIBS? >>> We discussed in a previous thread that EXTRA_* variables should >>> (as much as possible) be kept empty in Makefiles as it allows a >>> user to append things in them. >>> >>> By the way, it would be easier to follow the different versions >>> of your patches if you add "--in-reply-to " in your >>> git-send-email command, as described in http://dpdk.org/dev >> >> One more reason email handling is not working too many details and on github >> this would not have happened. > > Sorry but that's not the debate here. I'm just suggesting that you > use --in-reply-to to help other people to review your patches. That is why I did not put it in the previous email. Sorry if I offend you in some way. > > Thanks, > Olivier > > > >> >> Regards >> ++Keith >>> >>> Regards, >>> Olivier >>> >>> Signed-off-by: Keith Wiles --- examples/dpdk_qat/Makefile | 4 +- examples/vm_power_manager/Makefile | 2 +- mk/rte.app.mk | 242 + 3 files changed, 63 insertions(+), 185 deletions(-) diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile index f1e06a1..90ca1d3 100644 --- a/examples/dpdk_qat/Makefile +++ b/examples/dpdk_qat/Makefile @@ -77,8 +77,8 @@ else ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a endif -LDLIBS += -L$(ICP_ROOT)/build -LDLIBS += $(ICP_LIBRARY_PATH) \ +EXTRA_LDLIBS += -L$(ICP_ROOT)/build +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \ -lz \ -losal \ -ladf_proxy \ diff --git a/examples/vm_power_manager/Makefile b/examples/vm_power_manager/Makefile index 113dbc4..8fb78d4 100644 --- a/examples/vm_power_manager/Makefile +++ b/examples/vm_power_manager/Makefile @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/ CFLAGS += $(WERROR_FLAGS) -LDLIBS += -lvirt +EXTRA_LDLIBS += -lvirt # workaround for a gcc bug with noreturn attribute # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603 diff --git a/mk/rte.app.mk b/mk/rte.app.mk index 62a76ae..b8030d2 100644 --- a/mk/rte.app.mk +++ b/mk/rte.app.mk @@ -1,7 +1,7 @@ # BSD LICENSE # -# Copyright(c) 2010-2014 Intel Corporation. All rights reserved. -# Copyright(c) 2014 6WIND S.A. +# Copyright(c) 2010-2015 Intel Corporation. All rights reserved. +# Copyright(c) 2014-2015 6WIND S.A. # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT) endif # default path for libs -LDLIBS += -L$(RTE_SDK_BIN)/lib +_LDLIBS-y += -L$(RTE_SDK_BIN)/lib # # Include libraries depending on config if NO_AUTOLIBS is not set @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib # ifeq ($(NO_AUTOLIBS),) -LDLIBS += --whole-archive +_LDLIBS-y += --whole-archive -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y) -LDLIBS += -l$(RTE_LIBNAME) -endif +_LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS)+= -l$(RTE_LIBNAME) ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n) -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y) -LDLIBS += -lrte_distributor -endif - -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y) -LDLIBS += -lrte_reorder -endif +_LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)+= -lrte_distributor +_LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)+= -lrte_reorder -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) -LDLIBS += -lrte_kni -endif +_LDLIBS-$(CONFIG_RTE_LIBRTE_KNI)+= -lrte_kni +_LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM)+= -lrte_ivshmem endif -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y) -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) -LDLIBS += -lrte_ivshmem -endif -endif +_LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE) +
[dpdk-dev] [PATCH 00/10] Improve cast alignment for strict aligned machines
On Mon, 4 May 2015 11:50:09 +0200 Olivier MATZ wrote: > Hi Cyril, > > On 04/29/2015 06:15 PM, Cyril Chemparathy wrote: > > This series contains a few improvements that allow the DPDK code > > base to build properly on machines that enforce strict pointer cast > > alignment constraints. > > > > When dealing with packet data which could be arbitrarily aligned, > > we get the compiler to do the right thing by (a) making sure that > > header types are packed, and (b) introducing and using > > unaligned_uint(16|32|64)_t types when upcasting from byte pointers. > > > > In a few other instances, we know apriori that the pointer cast > > cannot possibly break alignment. This applies to the changes in > > mempool, hash, mbuf, and the ethdev stats code. Here, we simply > > silence the compiler by casting through (void *) using the > > RTE_PTR_(ADD|SUB) macros. > > > > Finally, we introduce a new rte_pktmbuf_mtod_offset() helper to > > return a type casted pointer to an offset within the packet data. > > This replaces the following commonly used pattern: > > (struct foo *)(rte_pktmbuf_mtod(m, char *) + offset) > > with: > > rte_pktmbuf_mtod_offset(m, struct foo *, offset) > > To ensure consistency, the above transform was applied throughout > > the code base using the coccinelle semantic patching tool. > > > > Before diving into the patches, I'm wondering if adding aligned(1) or > (packed) attribute at some places would have a performance impact on > supported architectures (Intel or IBM Power). Did you manage to test > it? > I don't expect to see a performance impact on x86. I don't really have a way of trying this out on PPC, and I could use some help here. Overall, I think that the few places where we've injected aligned(1) or packed really warrant the insertion for correctness. These are places where packet data of unknown alignment is being typecast and dereferenced. In such cases, _not_ representing the unaligned nature of these accesses breaks machines with strict alignment. Thanks -- Cyril.
[dpdk-dev] GitHub sandbox for the DPDK community
On Mon, May 04, 2015 at 12:43:48PM +, Qiu, Michael wrote: > What mail client do you use? I think mail client supporting thread mode > is important for patch review. Like many UNIX people, I use mutt. My concern is that, if we're making the widespread adoption, usage, and contributions for DPDK dependent on selection or debate of the features of various MUAs, I'm not sure that we're looking at this from the right angle. I'm just trying to figure out how to get DPDK in the place where the most eyeballs are, rather than trying to drag the eyeballs to the place where DPDK is. Matthew.
[dpdk-dev] GitHub sandbox for the DPDK community
On Mon, May 04, 2015 at 10:48:57AM -0700, Matthew Hall wrote: > On Mon, May 04, 2015 at 12:43:48PM +, Qiu, Michael wrote: > > What mail client do you use? I think mail client supporting thread mode > > is important for patch review. > > Like many UNIX people, I use mutt. > > My concern is that, if we're making the widespread adoption, usage, and > contributions for DPDK dependent on selection or debate of the features of > various MUAs, I'm not sure that we're looking at this from the right angle. > I think the exact opposite is being asserted here. To use the example being discussed, if you feel that quote collapsing is an important feature to your workflow, then pick an MUA that gives you that feature. Theres no need to mandate a workflow that is implemented by a service like github, just so that some set of features is available to those that like it. Neil
[dpdk-dev] GitHub sandbox for the DPDK community
On 01/05/15 20:17, Wiles, Keith wrote: > > On 5/1/15, 1:09 PM, "Stephen Hemminger" wrote: > >> On Fri, 1 May 2015 15:56:32 + >> "Wiles, Keith" wrote: >> >>> Hi Everyone, >>> >>> I believe the DPDK community would benefit from moving to GitHub as the >>> primary DPDK site. http://github.com >>> >>> I believe the DPDK community can benefit from being at a very well know >>> world wide site. GitHub seems to have the most eyes of any of the open >>> source Git repos today and it appears they have more then twice as many >>> developers. GitHub has a number of features I see as some good >>> additions to >>> our community using the GitHub organization account type. >>> >>> The cost for an organization account is $0 as long as we do not need >>> more >>> then 5 private repos. Minor issue: https://github.com/pricing Private repos for both users and organizations are not for free in github (they've never been afaik). They are in bitbucket, up to 5 contributors. But I don't get how private repositories have any influence in this discussion. Private repositories will be owned by companies and not DPDK as a community anyway. marc >>> 10 private repos is $25/month and had other plans >>> for more. I do not see us needing more then 5 private repos today and >>> the >>> only reason I can see having a private repo is to do some prep work on >>> the >>> repo before making public. Every contributor would need to create a >>> GitHub >>> personal account, which is at no cost unless you need more then 5 >>> private >>> repos. In both accounts you can have unlimited public repos. >>> >>> >>> https://help.github.com/articles/where-can-i-find-open-source-projects-to >>> -w >>> ork-on/ >>> >>> http://www.sitepoint.com/using-git-open-source-projects/ >>> >>> - Adding more committers can lead to a security problems for 6Wind (I >>> assume). >>> - 6Wind appearing to own DPDK.org is not a good message to the >>> community. >>> - Not assuming 6Wind?s dpdk.org site will disappear only where the >>> community stores the master repos and how the community interacts with >>> the >>> master. >>> - Permission and access levels in dpdk.org is only one level and we can >>> benefit from having 4 levels and teams as well. >>> - The patch process today suffers from timely reviews, which will not be >>> fixed by moving. >>> - GitHub has a per pull request discussions area, which gives a clean >>> way to review all discussions on a specific change. >>> - The current patch model is clone/modify/commit/send patch set >>> - The model with GitHub is fork on GitHub/modify/commit/send pull >>> request >>> - The patchwork web site is reasonable, but has some draw backs in >>> maintaining the site. >>> - GitHub manages the patches via pull requests and can be easily seen >>> via a web browser. >>> - The down side is you do have to use a web browser to do some work, >>> but >>> the bulk of the everyday work would be done as it is today. >>> - I think we all have a web browser now :-) >>> - GitHub has team support and gives a group better control plus >>> collaboration is much easier as we have a external location to work. >>> - Most companies have some pretty high security level and being to >>> collaborate between two or more companies is very difficult if one >>> company >>> is hosting the repo behind a firewall. >>> - Using GitHub and teams would make collaboration a lot easier or >>> collaboration between two or more user accounts as well. >>> - GitHub has a Web Page system, which can be customized for the >>> community >>> needs via a public or private repo. >>> - We still need a dpdk.org email list I believe as I did not find one at >>> GitHub. >>> - We can also forward GitHub emails to the list. >>> - I believe you can reply to an email from GitHub and the email will >>> get >>> appended to the discussion thread. >>> >> In my experience the github pull model causes less review, not more. >> It only works if maintainers are motivated to do this as their full time >> job. >> >> With email, the patches are right in front of developers and easier to >> quote >> for review comments. > We are not getting the eyes on the review today, which means to me it will > not matter if we move to GitHub method in the future. > > Personally I am able to see the differences with the GitHub display and > helps me see what is really happening. The emails are too flat and then > they can indent forever or someones email client (like mine) screws up the > format. With the GitHub method is will be exactly the same for everyone. >
[dpdk-dev] GitHub sandbox for the DPDK community
On 02/05/15 15:59, Wiles, Keith wrote: > > On 5/2/15, 6:40 AM, "Neil Horman" wrote: > >> On Fri, May 01, 2015 at 01:36:58PM -0700, Matthew Hall wrote: >>> On Fri, May 01, 2015 at 10:59:32PM +0300, Aaro Koskinen wrote: Projects like GCC, GLIBC, binutils, busybox, etc or what? A. >>> You'll notice all of these are low-level UNIX hacker sorts of tools >>> mostly, >>> with the partial exception of busybox. But even that is mainly for >>> embedded >>> use. It doesn't mean I don't think they're good and useful, but it does >>> limit >>> the possible size of the community in my view. >>> >>> Since we are talking about how to get the largest widest community >>> possible >>> for DPDK, it could require doing things a bit differently from how many >>> low-level tools have historically done things. >>> >> Why? >> >> Contributors to GCC: ~600 (based on svn) review >> Contrubutors to glibc : ~300 (based on git) review >> Contributors to binutils: ~600 >> Contributors to busybox: ~300 >> >> Contributors to DPDK: ~125 > I think the DPDK community can grow the number above and as we move toward > VNF/NFV I think it will grow to a much wider group of developers and not a > niche project as you stated. We can be much more then some of the above > IMHO. Keith, Since I didn't really know where to post this, I do it here. Like you, I think hosting the repository in github is a good idea to increase visibility to more developers. I am not so sure the development workflow can be shifted completely to github pull requests; there is a lot of controversy on this. So I would propose a middle-ground, *if* we think we can make it work: 1) The mailing-list, or mailing-lists, and the github pull requests should be synchronized. For this we could set a small cron job or BOT that inspects via the github API [*] the existing pull requests and emails the new ones to the DPDK mailing list. All pull requests can be downloaded as diffs and patches: https://github.com///pull/.diff https://github.com///pull/.patch [*] https://developer.github.com/v3/ The BOT could even do very basic checkings, such as the discussed "dpdk checkpatch" over the PR, and publish automatically comments on the PR based on conformance/no conformance of the patch style. 2) Discussion in the PR could be "echoed" by the bot in the mailing list, respecting the subject and threading, also via github's API. Automatic e-mails by github doesn't seem adequate to be echoed rawly in the list. 3) The synchronization needs to happen the other way around too. I am not completely sure which is the best way: a) Open an issue and reference the mailing list (DPDK mailman) for the patch and nothing more. b) More work but probably better; in a fork for the BOT of the official DPDK repository: i) Make the bot get the patch from the mailing list, create a branch, apply on top of current HEAD. If fails, notify the user to rebase its patched, informing on top of which version could not be applied ii) Issue a pull request "github.com/dpdk_bot/dpdk branch " -> "github.com/dpdk-conmmunity/dpdk branch master" 4) Discussions in the mailing list about a PULL request or a patch sent in the mailing list should be recovered by the BOT and echoed in the pull request 5) Normal issues: since the current DPDK doesn't have an issue tracker (afaik) it is easy. We could simply use that one and echo a _digested_ version of the comments into the mailing list. With this approach both "mailing list users" and "github users" should be able to work in parallel. Keith; what do you think? It really needs work, but I guess it could do the job. If you like it we could set up a small (parallel) mailing and work with your repository to try this "combined" workflow. Marc p.s. if by chance someone from github is listening reading, a functionality similar to this one would be welcome. >> Now I grant you that dpdk is a newer, much more niche project, but its >> disingenuous to state that we _have_ to do things differently to reach a >> wider >> audience. We can, but its by no means a prerequisite to gainining a wider >> audience. >>
[dpdk-dev] [PATCH] kni: fix compilation issue on kernel 3.19
Due to commit c0371da6 in kernel 3.19, which removed msg_iov
and msg_iovlen from struct msghdr, DPDK would not build.
This patch makes use of struct iov_iter, which has references
to those two variables.
Reported-by: Thomas Monjalon
Signed-off-by: Pablo de Lara
---
lib/librte_eal/linuxapp/kni/compat.h|4
lib/librte_eal/linuxapp/kni/kni_vhost.c | 13 ++---
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/lib/librte_eal/linuxapp/kni/compat.h
b/lib/librte_eal/linuxapp/kni/compat.h
index 1313523..1ad22ba 100644
--- a/lib/librte_eal/linuxapp/kni/compat.h
+++ b/lib/librte_eal/linuxapp/kni/compat.h
@@ -19,3 +19,7 @@
#define sk_sleep(s) (s)->sk_sleep
#endif /* < 2.6.35 */
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,19,0)
+#define HAVE_IOV_ITER_MSGHDR
+#endif
diff --git a/lib/librte_eal/linuxapp/kni/kni_vhost.c
b/lib/librte_eal/linuxapp/kni/kni_vhost.c
index 7141f83..a2fd97f 100644
--- a/lib/librte_eal/linuxapp/kni/kni_vhost.c
+++ b/lib/librte_eal/linuxapp/kni/kni_vhost.c
@@ -76,7 +76,7 @@ static struct proto kni_raw_proto = {
};
static inline int
-kni_vhost_net_tx(struct kni_dev *kni, struct iovec *iov,
+kni_vhost_net_tx(struct kni_dev *kni, const struct iovec *iov,
unsigned offset, unsigned len)
{
struct rte_kni_mbuf *pkt_kva = NULL;
@@ -143,7 +143,7 @@ drop:
}
static inline int
-kni_vhost_net_rx(struct kni_dev *kni, struct iovec *iov,
+kni_vhost_net_rx(struct kni_dev *kni, const struct iovec *iov,
unsigned offset, unsigned len)
{
uint32_t pkt_len;
@@ -361,8 +361,11 @@ kni_sock_sndmsg(struct kiocb *iocb, struct socket *sock,
if (unlikely(len < ETH_HLEN + q->vnet_hdr_sz))
return -EINVAL;
-
+#ifdef HAVE_IOV_ITER_MSGHDR
+ return kni_vhost_net_tx(q->kni, m->msg_iter.iov, vnet_hdr_len, len);
+#else
return kni_vhost_net_tx(q->kni, m->msg_iov, vnet_hdr_len, len);
+#endif
}
static int
@@ -391,7 +394,11 @@ kni_sock_rcvmsg(struct kiocb *iocb, struct socket *sock,
#endif
if (unlikely(0 == (pkt_len = kni_vhost_net_rx(q->kni,
+#ifdef HAVE_IOV_ITER_MSGHDR
+ m->msg_iter.iov, vnet_hdr_len, len
+#else
m->msg_iov, vnet_hdr_len, len
+#endif
return 0;
#ifdef RTE_KNI_VHOST_VNET_HDR_EN
--
1.7.4.1
[dpdk-dev] GitHub sandbox for the DPDK community
> On May 4, 2015, at 10:12 PM, Wiles, Keith wrote: > > > > On 5/4/15, 10:48 AM, "Matthew Hall" wrote: > >> On Mon, May 04, 2015 at 12:43:48PM +, Qiu, Michael wrote: >>> What mail client do you use? I think mail client supporting thread mode >>> is important for patch review. >> >> Like many UNIX people, I use mutt. >> >> My concern is that, if we're making the widespread adoption, usage, and >> contributions for DPDK dependent on selection or debate of the features >> of >> various MUAs, I'm not sure that we're looking at this from the right >> angle. >> >> I'm just trying to figure out how to get DPDK in the place where the most >> eyeballs are, rather than trying to drag the eyeballs to the place where >> DPDK >> is. > > +1, I agree with this statement completely and I feel discussions about an > MUA is non-productive and out of scope. +1. I?ve avoided the whole discussion, because ? ok, ?non-productive and out of scope? is a polite way of saying it. jim
