Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Determine if ethtool package can be dropped in favour of an upstream vendor #71

Closed
jodh-intel opened this issue Mar 19, 2018 · 7 comments
Assignees

Comments

@jodh-intel
Copy link
Contributor

The ethtool package:

... originally created under:

Comes from an upstream project:

We should determine if we can drop this package an instead vendor-in the official package above.

The upstream appears to have more functionality now than when we imported it.

@jodh-intel
Copy link
Contributor Author

/cc @mcastelino.

@sboeuf
Copy link

sboeuf commented Mar 19, 2018

We needed only a few things from this package and that's why we copy/pasted what was needed instead of importing the whole thing. If we think we might need those other functionalities from the upstream package, then yes I agree we might reconsider it, and import it directly.

@mcastelino
Copy link
Contributor

@jodh-intel @egernst @sboeuf I would keep the upstream package as we need other ethtool functionality for new use cases that we plan to add to Kata like RDMA support.

@sboeuf
Copy link

sboeuf commented Mar 19, 2018

@mcastelino we're not relying on the upstream package, so you mean we would move to the upstream one, right ?

@jodh-intel
Copy link
Contributor Author

I'd vote for that if at all possible since, stepping back, we're using a copy of an upstream package. We don't even have dep to tell us if upstream changed (think security fixes).

@amshinde
Copy link
Member

Agree, I moved from the vendoring the ethtool package to using a copy of the code to keep the code-base smaller. If we are going to need this for other functionality as well, and as @jodh-intel mentioned we need to consider security fixes, I would vendor in the upstream package.
If no one objects, I'll vendor the upstream.

@sboeuf
Copy link

sboeuf commented Mar 19, 2018

@amshinde I think everybody agreed, you can go ahead and vendor the upstream package !

amshinde added a commit to amshinde/kata-runtime that referenced this issue Mar 19, 2018
We were using code copied from github.com/safchain/ethtool.
Vendor in upstream package instead to use additional
functionality added in.

Fixes kata-containers#71

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
@egernst egernst removed the review label Mar 19, 2018
zklei pushed a commit to zklei/runtime that referenced this issue Jun 13, 2019
oci: Convert the OCI spec into libcontainer config
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants