Commit Graph

35 Commits

Author SHA1 Message Date
Midhun Somasundaran
6dca23bac0 Add neighbor entry verification tests
Summary: Adding tests to verify that NDP and ARP entry resolution happens correctly on Aggregate ports. Recently we have seen sevs where ARP resolution happens incorrectly on subports

Reviewed By: jasmeetbagga

Differential Revision: D28156706

fbshipit-source-id: 3066620e9ed0d8805cf0d2733ea6fdece47b4254
2021-05-19 09:45:19 -07:00
Midhun Somasundaran
00bcea3aa4 MultiNode test - Add local and remote link event test cases
Summary: Add test cases to verify LACP behavior with local and remote link down events. For triggering remote link events, a thrift client is used.

Differential Revision: D26329897

fbshipit-source-id: fa1bdfe6df80cab77731ec6e6f21aa826aca3e7f
2021-02-19 08:17:15 -08:00
Srikrishna Gopu
27d32bff2f getting rid of Time.h
Summary: As titled, getting rid of Time.h and use std::chrono to compute the current time.

Differential Revision: D21005713

fbshipit-source-id: 7e9d31af4b8652199bbe36b6ef76968cadcb4f2c
2020-04-14 09:53:10 -07:00
Srikrishna Gopu
a75c4ac770 return time for NowInSecFast
Summary: As titled, fixing NowInSecFast to return current time in OSS.

Reviewed By: boryas

Differential Revision: D20996594

fbshipit-source-id: 69fb37ea3a6a01e6d0f61fb696121705da52f811
2020-04-13 16:10:41 -07:00
Jasmeet Bagga
a7246e9781 Factor our hw_port_stats_fb303 lib
Summary:
Implementation based on what we do in bcm layer
for reporting (non histogram) port stat counters. Abstracting
this lib out so we can reuse this code in both SAI and bcm
layers. Plus pulling this out of BcmPort makes it much
easier to unit test this code.

Reviewed By: boryas

Differential Revision: D19676641

fbshipit-source-id: 88aa20f20339729f5387500bcf6084871f4e7caa
2020-02-03 17:37:05 -08:00
Alex Eckert
c022b28460 deprecate copied stats headers that are now open sourced
Summary:
We are now using the open source fb303 stats libraries so
these locally managed headers are no longer needed.

Reviewed By: chadaustin

Differential Revision: D16969961

fbshipit-source-id: 71df4add75b045967a0087721c0b742edcabdf4d
2019-08-27 17:15:54 -07:00
Alex Eckert
badd2c8f26 break oss build dependency on fboss/common/
Summary:
All of the requisite headers are now open sourced and eden
has been converted to use them. This removes any logic that references
this directory and kills the now unused cmake file.

Reviewed By: chadaustin

Differential Revision: D16994732

fbshipit-source-id: 573b8b50540ee64590682be2a54aa94659f12368
2019-08-27 17:15:54 -07:00
Alex Eckert
850ab27d4d use facebook::fb303 instead of common::stats where possible
Summary:
Most stat code in fbcode was moved in to open source under
fbcode/fb303. The only exception that we use internally is the
MonotonicCounter class. This diff moves all of our stats code to use
the open source headers and updates cmake files to pull in the open
source fb303.

Reviewed By: chadaustin, shri-khare

Differential Revision: D16969960

fbshipit-source-id: 12b7abb54c956c242c8e27eb69fd96925c7e61f7
2019-08-27 17:15:54 -07:00
Adam Simpkins
8dc58acda1 rename the fb303_thrift_cpp2 build rule to fb303_thrift_cpp
Summary:
Drop the `2` from the `fb303_thrift_cpp2` rule name.

The old `cpp` version of fbthrift is gone, `cpp2` is the only supported C++
version, so lets drop the `2` from this build rule name.

I plan to change some of the CMake thrift rule generation code soon, to
support multiple languages (namely Python).  As part of this I plan to
automatically add a language-specific suffix(`_py` or `_cpp`) to the build
rules.  It seems better to use `_cpp` rather than `_cpp2` for C++ libraries.
Changing the name for this fb303 rule will make it fit this planned naming
scheme.

Reviewed By: chadaustin

Differential Revision: D16653264

fbshipit-source-id: e4da70c79ff14e9981717ab909d982770d104512
2019-08-07 11:22:34 -07:00
Adam Simpkins
513dfb17fd add a stub implementation of ServiceData::incrementCounter()
Summary: Add a stub for this so we can use it in some the Eden code.

Reviewed By: chadaustin

Differential Revision: D15831519

fbshipit-source-id: 8f16d8fcb7080d207c748281bd93b9243c104596
2019-06-14 18:14:43 -07:00
Shrikrishna Khare
1163685005 Remove unncessary includes that break OSS
Summary:
D15324752, D15331361 changed includes from common/stats/* to fb303/* stats, but
this breaks the FBOSS OSS build. However, we can do without those includes.

Each of the FBOSS #include callsites either also include
 - common/stats/MonotonicCounter.h (includes common/stats/ServiceData.h includes
   fb303/ServiceData.h includes Exported*) or,
 - common/stats/ThreadCachedServiceData.h includes
   (common/stats/ThreadLocalStatsMap.h includes
   common/stats/ThreadLocalStats.h includes common/stats/ServiceData.h …)

Specifically, BcmPort.h includes MonotonicCounter.h, while SwitchStats.cpp/.h,
BcmStats.cpp/.h includes ThreadCachedServiceData.h

Lastly, fix the fboss OSS stub implementation to have similar includes

Reviewed By: chadaustin

Differential Revision: D15626299

fbshipit-source-id: 540aa40150f801bb29aeadb235cb6bb1bafca6ce
2019-06-05 15:35:52 -07:00
Yedidya Feldblum
0e620ade4d Cut D support from assorted .thrift files
Summary: Cut D support from assorted .thrift files.

Reviewed By: simpkins, markisaa, GirasoleY

Differential Revision: D15492500

fbshipit-source-id: 68c7fb4ff6c8917be0db961c7bdaa62a75f1b300
2019-05-29 02:32:16 -07:00
Adam Simpkins
1cdee770a5 update copyright headers in some of the common/ stub headers
Summary: Update copyright messages to match the patterns expected by our open source linters.

Reviewed By: chadaustin

Differential Revision: D15105843

fbshipit-source-id: 6c66b4e3a552f9d71e6b43f9a5e6fcda48fd00d9
2019-04-26 14:38:28 -07:00
Shrikrishna Khare
55b1190327 Remove stale fb303 counters
Summary:
When counters are renamed (port, trunk or queues), the current FBOSS logic
creates a new monotonic counter with new name and removes (last reference to)
monotonic counter for the old name.

However, the old counter continues to unncessarily get exported. This patch
fixes it by deleting the old counters.

Differential Revision: D14942193

fbshipit-source-id: 34cf7dd5290ae87a2c8002775e9d0940615c47fb
2019-04-16 08:30:40 -07:00
Shrikrishna Khare
ec85a8b308 spawn thread to publish stats
Summary:
wedge_agent uses ThreadLocal stats update API to avoid the overhead of
acquiring lock for each stat update. Prior to this patch, wedge_agent used its
own StatsPublisher class which called publishStats every second. StatsPublisher
also asserted if startPublishThread is not running as StatsPublisher was doing
the stats publish.

However, there is no reason for wedge_agent to publish stats on its own.
Instead, the wedge_agent could use startPublishThread mechanism.

With D14255180, servicerouter/dispatcher now calls startPublishThread, which
started hitting in the aforementioned ASSERT in StatsPublisher.

This patch fixes it by having wedge_agent start the publish thread by calling
startPublishThread. Among other things, StatsPublisher also updated route stats
and port info. That functionality is moved to updateStats.

Differential Revision: D14388119

fbshipit-source-id: 503d08f12131317592be64e46243401d8a2e5bae
2019-03-09 10:14:46 -08:00
Wez Furlong
cd5b4b6d92 watchman: pull in thrift in the oss build for eden support
Summary:
This diff enables building the eden watcher by linking in thrift and
its various dependencies.

To support building in-fbsource and in the github repo, a `maybe_shipit_dir`
function is used to setup a symlink to the `eden` and `fboss/common` dirs (this
mirrors the shipit configuration for this project: we cannot simply run shipit
because we have to build on mac and windows and shipit requires Hack, and that
does not support those platforms).

I tried to persuade cmake to let me build this without the use of a symlink but
found it too difficult to teach everything about the path mapping.  The
symlinks aren't terrible, but are the reason why this diff also updates some
`.gitinore` files that are seemingly unrelated to this diff.

This diff changes a couple of build/link options: without them the end product
fails to link either due to implicit/unilateral enablement of UBSAN in some of
the deps, or because warning->error promotion is turned on.

This diff includes a copy of the `ThriftCppLibrary.cmake` file from the fboss
repo.  This should get centralized and shipit'ed out into the places that
consume it.  That can be done when someone gets around to doing the same for
the `FindGlog.cmake` file and doesn't need to hold up this diff.

Reviewed By: simpkins

Differential Revision: D13486486

fbshipit-source-id: 3bb5b011771b2a87618147ca019b4e50a8e0aaf2
2019-02-04 21:37:47 -08:00
Shrikrishna Khare
0de0ba4f3b enable BST stats by default
Summary:
This patch enables BST stats by default, and works as follows:
  - it uses quantile stats to maintain per queue per port stats.
  - these stats are exposed to ODS.
  - experiments showed that a significant CPU cycles were spent in doing
    bcm_cosq_bst_stat_sync which syncs the hw stat value to sw copy.
    For example, calling bcm_cosq_bst_stat_sync while retrieving BST stats
    for for every port/every queue/every second resulted into CPU utilization
    going up from ~ 50% for Wedge 100 to ~100-120%. Thus, this patch calls
    bcm_cosq_bst_stat_sync only once per minute and then retrieves all BST stats.
  - The above optimization also helps when bufferstatlogging is enabled.
  - when bufferstatlogging is enabled, the BST stats will be retrieved every
    second for scuba logging, and quantile stats will also be exported every
    second.
  - The bufferstatlogging is enabled using fboss CLI (over thrift).
    We no longer need enable_fine_grained_buffer_stats option. But, a new option
    viz. update_bststats_interval_s is introduced that keeps the frequency of BST
    stats updates configurable.

The virtual memory overhead of always maintaining BST stats is < 5%, and
physical memory overhead is < 0.1%. Thanks to the sync optimization above,
the CPU overhead is negligible. Refer Test Plan for raw data and math.

An alternative is to explore using TimeSeriesMinMax (to maintain max over a
time window) and TLCounter (to expose stats to ODS) instead of quantile stats.
One benefit of using quantile stats is that we don't need to maintain our own
TimeSeriesMinMax but can rely on common facebook wide infra. Moreoever,
quantile stats interface is (arguably) simpler than using combination of
TimeSeriesMinMax + TlCounter'.

A separate diff against configerator is sent out to expose the new bst_stats: D8902636

Differential Revision: D8764329

fbshipit-source-id: 78db3de5340497f2f5780863035dd9ff28dcd79b
2019-01-25 10:37:57 -08:00
Wez Furlong
c219906817 add getPid to fb303
Summary: Eden uses getPid() heavily for health/status validation.

Reviewed By: chadaustin

Differential Revision: D13478522

fbshipit-source-id: c1ce95671f4039cdabdd6015fdffdec738306fed
2018-12-17 15:53:15 -08:00
Adam Simpkins
a5ede43ea8 add FacebookBase2::exportThriftFuncHist() to the fb303 stubs
Summary:
We call this function in Eden, so make it available in the fb303 stubs that
we include in the github repository.

Differential Revision: D12813738

fbshipit-source-id: 1432e751ce676410e9d2e589274c7c1dc2c45321
2018-10-30 13:06:14 -07:00
Jun Wu
0b10854ee8 add more APIs to common/stats stubs
Summary:
Solves an issue building eden GitHub version:

  eden/fs/service/EdenServer.cpp:1044:34: error: ‘class facebook::stats::ServiceData’ has no member named ‘addStatValue’
       stats::ServiceData::get()->addStatValue(
                                  ^~~~~~~~~~~~

Reviewed By: bolinfest, simpkins

Differential Revision: D9029274

fbshipit-source-id: c0e5ea492c1c38f16bfae4b08d41538a2443450f
2018-07-27 14:36:42 -07:00
Adam Simpkins
dea514bbef add CMake build files
Summary: Add CMakeFiles to build Eden.

Reviewed By: wez

Differential Revision: D7479587

fbshipit-source-id: 7e0b4a756005dadc3af5c13c36ce22d1dcc15071
2018-04-30 14:37:46 -07:00
Adam Simpkins
2aad795256 add additional APIs to common/stats stubs
Summary: Update the common/stats stubs with additional APIs needed by eden.

Reviewed By: capveg

Differential Revision: D7479588

fbshipit-source-id: 963a6ff03dbd53d101c05b651ab291745a42e20a
2018-04-04 15:22:57 -07:00
Adam Simpkins
21d2b6c46d Remove TARGETS files
Summary:
This removes the TARGETS files from the eden github repository.  The
open source buck build has been failing for several months, since buck
removed support for the thrift_library() rule.

I will potentially take a stab at adding CMake build support for Eden
at some point in the future.

Reviewed By: chadaustin

Differential Revision: D6893233

fbshipit-source-id: e6023094a807cf481ac49998c6f21b213be6c288
2018-02-20 19:57:45 -08:00
Rob Sherwood
ed47573b67 Fix open source aliveSince() implementation
Summary:
aliveSince() has a misleading name and I misunderstood.
The correct implementation just returns the timestamp of when
the process started as opposed to my understanding (the number
of seconds since the process started).

Reviewed By: linhbui

Differential Revision: D6789987

fbshipit-source-id: db60bf00f6b07e31d91dfc8ed06d1c1cbe172a64
2018-01-25 10:22:07 -08:00
Rob Sherwood
0eb330520f Added aliveSince() implementation to OSS fb303 skeleton
Summary:
fb303 is not open source, but fboss depends on it.  So, the
open source version of fboss uses its own private, mostly
stubbed out version of fb303 (in fboss/common/...).

With the changes to QsfpCache.cpp, we now depend on aliveSince()
fb303 primitive and the open source started spewing errors
about it not being implemented.

This patch implements aliveSince() for the skeleton open source
fb303 but we'll have to figure out a better, longer term fix
(e.g., open source all of fb303).

Reviewed By: aeckert

Differential Revision: D6731321

fbshipit-source-id: 6acdcd68053e7bfcada2cfd62aaa3aecf3dec3e0
2018-01-17 13:02:09 -08:00
Alex Eckert
c684fed90a fix oss sandcastle job
Summary:
My recent stack of diffs introduced a handful of oss bugs :(.

1. VLOG_EVERY_MS is an internal function.
fix: added an oss stub version

2. Forgot to add QsfpCache.cpp to CMakeLists.txt
fix: add it

3. vanilla gcc5.4 has a bug where it can't handle lambdas like:
  [this]() {
    memberFn();
  }
and instead you need to write like:
  [this]() {
    this->memberFn();
  }

fix: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67274 says it should
be fixed in gcc7, but I added 'this->' in the meantime.

4. The base fb303.thrift file we have in open source does not declare
aliveSince, which we now call on qsfp_service.

fix: add it to fb303.thrift

Reviewed By: ninas

Differential Revision: D6627705

fbshipit-source-id: 2100783df1ea0e9af0fed66e4e24ef85b71fc7e9
2017-12-22 09:21:44 -08:00
Philip Jameson
f18128537f Migrate TARGETS files from @/ to //
Summary:
This is a codemod to change from using @/ to // in basic cases.
- TARGETS files with lines starting with @/ (but excluding @/third-party:
- autodeps lines in source and TARGETS files ( (dep|manual)=@/ ), excluding @/third-party
- Targets in string macros

The only thing left of the old format should be @/third-party:foo:bar

drop-conflicts

Reviewed By: ttsugriy

Differential Revision: D6605560

fbshipit-source-id: 17d3a196b91045f0db5ee2a5afad467b6344be0b
2017-12-20 18:39:22 -08:00
Rob Sherwood
4c3ea3f14d Completely redo CMakeFiles.txt and getdeps.sh for new thrift
Summary:
Should make OSS build again :-(
Closes https://github.com/facebook/fboss/pull/57

Reviewed By: eduardo-elizondo

Differential Revision: D6048266

Pulled By: capveg

fbshipit-source-id: 005a9f5b2c0aa21abfcd4932b16e8ef062bb7eb0
2017-10-17 00:35:03 -07:00
Joseph Wu
6169d8c059 rename link_state_down to link_state_flap
Summary:
In this diff, the main goal is to change the old `link_state.down` to `link_state.flap`. However, the tricky part is how can we use the normalized
portName(ethX/Y/Z) instead of portX as the key name. Since, the initialization of wedge_agent will go through two stage genrally:
In such case, we need to support changing the portName for PortStats after it's created.

My last diff(D5695076) is trying to delete all the TLTimeseries of all the threads in PortUpdateHandler observer, while ThreadLocalStats will also
try to unregister(delete) the TLTimeseries before we shut down wedge_agent. Therefore, we saw this:
```
Sep 16 19:09:52 rsw1fs.17.prn2.facebook.com wedge_agent[1884398]: F0916 19:09:52.716562 1884650 ThreadLocalStats-defs.h:495] Check failed: numErased == 1 attempted to unregister a stat that was not registered
```

So I guess probably it's not a good idea to delete a TLTimeseries of some thread in a different thread, unless you can get the global lock like ThreadLocalStats
does. Thus, in this new diff, I kinda walk around it:

Differential Revision: D5914457

fbshipit-source-id: f7d47d2ececfabbab8fdc222a0f82db4184230f9
2017-10-09 10:36:47 -07:00
Jasmeet Bagga
9bbee241e6 Change API between SwSwitch and HwSwitch to indicate HwSwitch might fail some
Summary:
Depend on D4492997
Currently, we assume that changes applied to HwSwitch always succeed (or the process exits). There is a case where we want to keep moving despite failures: when we get excessive number of route (probably) because of some misconfiguration. This diff is a step towards taking care of such situation.

We change the interface between SwSwitch and HwSwitch so that HwSwitch, when asked to apply delta of old and new state, returns either the new state if it succeeds, or returns a "pruned version of new state" if some elements of new state it was not able to apply.

SwSwitch now keeps track of two states, a state that has been applied in the hardware, and another one that is desired in hardware. SwSwitch strives to catch applied state to the desired state (while desired state is also changes as move state updates are arriving).

One issue with this diff: Warmboot is not preserving "unapplied" routes, i.e., unapplied routes get lost across warmboots. This will be fixed in a seperate diff.

One naming issue: I thought about keeping SwSwitch::getState() name intact, but thought that it is not a good idea to keep distinction of states opaque to callers. So, decided to change the name.

Reviewed By: ninas

Differential Revision: D4405751

fbshipit-source-id: c933b4418445ae49d5b8e21dea95e437a8e1593d
2017-06-02 13:38:22 -07:00
Andrew Gallagher
6df267fe5c codemod: format TARGETS with buildifier [3/5] (D5092623)
Reviewed By: igorsugak

fbshipit-source-id: c3036011faa009bb2b051142b1caedfdbe77b3df
2017-06-01 16:51:16 -07:00
Saif Hasan
d792d1607c Do not set default value for ifName
Summary:
Setting default value to empty causes trouble on server if client is old as
server will think that value is set to empty and it tries to parse empty value.

This field is not being used anywhere except in `openr` for now and openr is
fully compatible with this change. Not setting default value will give us as
`__isset.ifName` to `false` instead of being empty string, which is desired
behaviour.

(Note: this ignores all push blocking failures!)

Reviewed By: anantdeepak

Differential Revision: D4934119

fbshipit-source-id: 4dd2e9307c659f1b6ccaf583964efa8df6041d24
2017-04-21 22:49:35 -07:00
Jasmeet Bagga
55812e225c Use normalized names for interface port stats
Summary:
Logical ids are so 2014. We have been thinking
about emitting stats using normalized names for some time.
Approach wise, I picked the following
- If port does not have a name - default to using port ID
- In agent if a port name get set or updated. Re init BCMPort stats. Basically
this means that the moment config gets applied, we would start emitting
normalized interface stats.

Two additional pieces of work here are
i) Update interface based alerts to also look at these new normalized
interface name stats - t17052196
ii) Update cubism to look at these new names - t17052255

I won't land till at least i) is fixed

Differential Revision: D4815561

fbshipit-source-id: c088b5a8981ea40429fedc8570652c376a374be3
2017-04-11 07:08:48 -07:00
Saif Hasan
a03676f7fb Add ifName field to BinaryAddress
Summary:
We are using `BinaryAddress` heavily within fboss codebase to represent a
nexthop as well as an IPAddres in wire format. Nexthops will need to have
a scoping identifier (iface name) for link-local address inorder to resolve
it properly.

We do this in OpenR and well tested on Arista. This seems bit hacky but it
saves a lot of time. In future we would like to have separate network util for
fboss and openr and we can get rid of this hack at that time.

Differential Revision: D4710768

fbshipit-source-id: 72092935f3738b3fb88a422462d1c995cc0acf18
2017-03-17 17:13:20 -07:00
Adam Simpkins
7f58caffe4 Add fb303 stubs in common/ to support building in the opensource repo
Summary:
This adds a common/ directory with stub files required for building
eden.  These stubs are the same ones as used in the fboss repository:
https://github.com/facebook/fboss/

Our goal is to eventually create an opensource fb303 repository with
complete implementations of these files, but for now these basic stubs
allow building eden.  (Most of the underlying stats code that supports
fb303 is already available in folly/stats/ in the folly repository.)

Test Plan:
Tested building eden on Ubuntu 16.04
2017-03-17 17:08:05 -07:00